[BUILD][MISC] Merge pull request #269 from pioneers/makefile_tests_new

[BUILD][MISC] Rewritten all Makefiles for efficiency and correctness

.github/workflows/pull_request_ci.yml

@@ -89,6 +89,11 @@ jobs:
             tar -xzf protobuf-c-1.4.1.tar.gz
             cd protobuf-c-1.4.1 && ./configure && make && sudo make install && sudo ldconfig
 
+      # Before we build Runtime, we check the formatting
+      - name: Format
+        run: |
+           ./runtime format check
+
       # Now that we are done installing Runtime's dependencies, we build Runtime
       - name: Build
         run: |
@@ -97,4 +102,4 @@ jobs:
       # And finally, we test Runtime
       - name: Test
         run: |
-          ./runtime test
+          ./runtime test

.gitignore

@@ -2,9 +2,6 @@
 bin/*
 tests/bin/*
 
-# ignore the tests/devices
-tests/devices
-
 # ignore the entire lowcar/Device folder
 lowcar/Device
 
@@ -24,7 +21,7 @@ network_switch/exit_status.txt
 *.pyc
 *.pyo
 __pycache__/
-build/
+executor/build/
 
 # VS Code
 .vscode
@@ -38,29 +35,11 @@ build/
 *.o
 *.obj
 
-# Precompiled Headers
-*.gch
-*.pch
-
 # Compiled Dynamic libraries
 *.so
 *.dylib
 *.dll
 *.pyd
 
-# Fortran module files
-*.mod
-*.smod
-
-# Compiled Static libraries
-*.lai
-*.la
-*.a
-*.lib
-
-# Executables
-*.exe
-*.out
-*.app
-
+# .DS_Store
 *.DS_Store

Makefile_defs

@@ -0,0 +1,35 @@
+# This Makefile defines a bunch of stuff used by the lower-level Makefiles
+# all file paths are from their perspective!
+
+CC = gcc
+
+BIN = ../bin
+BUILD = ../build
+OBJ = $(BUILD)/obj
+
+# this is the name of the lower level directory (e.g. THIS_DIR in the dev_handler folder is "dev_handler")
+THIS_DIR = $(strip $(shell pwd | xargs basename -z))
+
+# list of runtime component directories
+COMPONENT_DIRS = dev_handler net_handler executor network_switch shm_wrapper runtime_util logger
+ # list of subfolders in $(OBJ) for runtime object files
+OBJ_SUBDIR = $(foreach dir,$(COMPONENT_DIRS),$(OBJ)/$(dir))
+# list of folders to include in compilation commands
+INCLUDE += $(foreach dir,$(COMPONENT_DIRS),-I../$(dir))
+
+# list of all object files this executable is dependent on relative to this Makefile
+OBJS = $(patsubst %.c,$(OBJ)/$(THIS_DIR)/%.o,$(SRCS))
+
+# list of build folders
+BUILD_DIR += $(OBJ) $(BIN) $(BUILD) $(OBJ_SUBDIR)
+
+# append -MMD and -MP to CFLAGS to get the dependency files built for the object files
+CFLAGS += -MMD -MP
+
+# general rule for compiling a list of source files to object files in the $(OBJ) directory
+$(OBJ)/$(THIS_DIR)/%.o: %.c | $(OBJ_SUBDIR)
+	$(CC) $(CFLAGS) $(INCLUDE) -c $< -o $@
+
+# general rule for making a build directory
+$(BUILD_DIR):
+	mkdir -p $@

dev_handler/Makefile

@@ -1,10 +1,30 @@
-LIBFLAGS=-pthread -lrt -Wall
-BIN=../bin
+# list of libraries that dev_handler needs to compile
+LIBS=-pthread -lrt -Wall
 
-dev_handler: dev_handler.c message.c message.h ../logger/logger.c ../runtime_util/runtime_util.c ../shm_wrapper/shm_wrapper.c
-	gcc $^ -o $(BIN)/dev_handler $(LIBFLAGS)
+# list of source files that the target (dev_handler) depends on, relative to this folder
+SRCS = dev_handler.c dev_handler_message.c ../logger/logger.c ../runtime_util/runtime_util.c ../shm_wrapper/shm_wrapper.c
 
+# specify the target (executable we want to make)
+TARGET = dev_handler
+
+.PHONY: all clean $(TARGET)
+
+all: $(TARGET)
+
+# include top-level Makefile for some common variables and rules
+include ../Makefile_defs
+
+# resolve phony target "dev_handler" as ../bin/dev_handler
+$(TARGET): $(BIN)/$(TARGET)
+
+# rule to compile dev_handler
+$(BIN)/$(TARGET): $(OBJS) | $(BIN)
+	$(CC) $(OBJS) -o $@ $(LIBS)
+
+# remove build artifacts
 clean:
-	rm -f $(BIN)/dev_handler
-	rm -f dev_handler
-
+	rm -f $(OBJS)
+	rm -f $(patsubst %.o,%.d,$(OBJS))
+	rm -f $(BIN)/$(TARGET)
+
+-include $(patsubst %.o,%.d,$(OBJS))

dev_handler/dev_handler.c

@@ -5,10 +5,10 @@
 
 #include <termios.h>  // for POSIX terminal control definitions in serialport_open()
 
-#include "../logger/logger.h"
-#include "../runtime_util/runtime_util.h"
-#include "../shm_wrapper/shm_wrapper.h"
-#include "message.h"
+#include <dev_handler_message.h>
+#include <logger.h>
+#include <runtime_util.h>
+#include <shm_wrapper.h>
 
 /**
  * Each device will have a unique port number.
@@ -24,7 +24,7 @@
  * These file paths may also be referred to as "port_prefix" in the code.
  */
 #define LOWCAR_FILE_PATH "/dev/ttyACM"
-#define VIRTUAL_FILE_PATH "ttyACM"			// will be created in the home directory
+#define VIRTUAL_FILE_PATH "ttyACM"  // will be created in the home directory
 #define LOWCAR_USB_FILE_PATH "/dev/ttyUSB"
 
 // **************************** PRIVATE STRUCT ****************************** //
@@ -93,7 +93,7 @@ uint32_t used_lowcar_usb_ports = 0;
 pthread_mutex_t used_ports_lock;  // poll_connected_devices() and relay_clean_up() shouldn't access used_ports at the same time
 
 // String to hold the home directory path (for looking for virtual device sockets)
-const char *home_dir;
+const char* home_dir;
 
 #define MAX_PORT_NAME_SIZE 64
 
@@ -395,7 +395,7 @@ void relay_clean_up(relay_t* relay) {
     pthread_mutex_unlock(&used_ports_lock);
     pthread_mutex_destroy(&relay->relay_lock);
     pthread_cond_destroy(&relay->start_cond);
-    if (relay->dev_id.uid == -1) {
+    if (relay->dev_id.uid == (uint64_t) -1) {
         char port_name[MAX_PORT_NAME_SIZE];
         construct_port_name(port_name, relay->is_virtual, relay->is_usb, relay->port_num);
         log_printf(DEBUG, "Cleaned up bad device %s\n", port_name);
@@ -582,7 +582,7 @@ int receive_message(relay_t* relay, message_t* msg) {
     char port_name[MAX_PORT_NAME_SIZE];
     construct_port_name(port_name, relay->is_virtual, relay->is_usb, relay->port_num);
 
-    if (relay->dev_id.uid == -1) {
+    if (relay->dev_id.uid == (uint64_t) -1) {
         /* Haven't verified device is lowcar yet
          * read() is set to timeout while waiting for an ACK (see serialport_open())*/
         pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL);
@@ -872,7 +872,7 @@ int main(int argc, char* argv[]) {
     // If SIGINT (Ctrl+C) is received, call stop() to clean up
     signal(SIGINT, stop);
     init();
-	home_dir = getenv("HOME"); // set the home directory 
+    home_dir = getenv("HOME");  // set the home directory
     log_printf(INFO, "DEV_HANDLER initialized.");
     poll_connected_devices();
     return 0;

dev_handler/message.c → dev_handler/dev_handler_message.c

@@ -1,10 +1,10 @@
-#include "message.h"
+#include <dev_handler_message.h>
 
 // ******************************** Utility ********************************* //
 
 void print_bytes(uint8_t* data, size_t len) {
     printf("0x");
-    for (int i = 0; i < len; i++) {
+    for (size_t i = 0; i < len; i++) {
         printf("%02X ", data[i]);
     }
     printf("\n");
@@ -71,7 +71,7 @@ static int append_payload(message_t* msg, uint8_t* data, size_t len) {
  */
 static uint8_t checksum(uint8_t* data, size_t len) {
     uint8_t chk = data[0];
-    for (int i = 1; i < len; i++) {
+    for (size_t i = 1; i < len; i++) {
         chk ^= data[i];
     }
     return chk;
@@ -292,7 +292,7 @@ ssize_t message_to_bytes(message_t* msg, uint8_t cobs_encoded[], size_t len) {
     }
     data[0] = msg->message_id;
     data[1] = msg->payload_length;
-    for (int i = 0; i < msg->payload_length; i++) {
+    for (size_t i = 0; i < msg->payload_length; i++) {
         data[i + MESSAGE_ID_SIZE + PAYLOAD_LENGTH_SIZE] = msg->payload[i];
     }
     data[MESSAGE_ID_SIZE + PAYLOAD_LENGTH_SIZE + msg->payload_length] = checksum(data, MESSAGE_ID_SIZE + PAYLOAD_LENGTH_SIZE + msg->payload_length);
@@ -317,7 +317,7 @@ int parse_message(uint8_t data[], message_t* msg_to_fill) {
         // Smaller than valid message
         free(decoded);
         return 3;
-    } else if (ret > (MESSAGE_ID_SIZE + PAYLOAD_LENGTH_SIZE + MAX_PAYLOAD_SIZE + CHECKSUM_SIZE)) {
+    } else if (ret > (int) (MESSAGE_ID_SIZE + PAYLOAD_LENGTH_SIZE + MAX_PAYLOAD_SIZE + CHECKSUM_SIZE)) {
         // Larger than the largest valid message
         free(decoded);
         return 3;

dev_handler/message.h → dev_handler/dev_handler_message.h

@@ -10,8 +10,8 @@
 #ifndef MESSAGE_H
 #define MESSAGE_H
 
-#include "../logger/logger.h"
-#include "../runtime_util/runtime_util.h"
+#include <logger.h>
+#include <runtime_util.h>
 
 /* The maximum number of milliseconds to wait between each DEVICE_PING from a device
  * Waiting for this long will exit all threads for that device (doing cleanup as necessary)

executor/Makefile

@@ -1,26 +1,45 @@
-# Change this to point to your specific Python version. Need >= 3.6
-py=python3.10
+# list of libraries that executor needs to compile
+LIBS=-pthread -lrt -Wall -export-dynamic -fPIC
 
-CFLAGS=$(shell $(py)-config --cflags)
-LDFLAGS=$(shell $(py)-config --ldflags)
-PY_LIB=-l$(py)
-LIBS=-pthread -lrt -export-dynamic -fPIC -lprotobuf-c
-#-rdynamic or -export-dynamic to make it dynamically linked
-BIN=../bin
+# list of source files that the target (executor) depends on, relative to this folder
+SRCS = executor.c gamestate_filter.c ../logger/logger.c ../runtime_util/runtime_util.c ../shm_wrapper/shm_wrapper.c
 
-all: executor studentapi.c
+# Python compilation definitions
+PY_VER = python3.10
+PY_LIB = -l$(PY_VER)
+CFLAGS = $(shell $(PY_VER)-config --cflags)
 
-executor: executor.c gamestate_filter.c ../shm_wrapper/shm_wrapper.c ../logger/logger.c ../runtime_util/runtime_util.c ../net_handler/net_util.c ../net_handler/pbc_gen/text.pb-c.c
-	gcc $(CFLAGS) $^ -o $(BIN)/$@ $(LIBS) $(PY_LIB)
+# specify the target (executable we want to make)
+TARGET = executor
 
-studentapi.c: studentapi.pyx runtime.pxd
-	$(py) setup.py build_ext -i
+.PHONY: all clean $(TARGET)
 
+all: $(TARGET) studentapi.c
+
+# include top-level Makefile for common variables and rules
+include ../Makefile_defs
+
+$(TARGET): $(BIN)/$(TARGET)
+
+# rule to compile executor
+$(BIN)/$(TARGET): $(OBJS) | $(BIN)
+	$(CC) $(OBJS) -o $@ $(LIBS) $(PY_LIB)
+
+# rule to compile studentapi.c
+studentapi.c: studentapi.pyx studentapi.pxd setup.py
+	$(PY_VER) setup.py build_ext -i
+
+# rule to compile testapi
 test_api: studentapi.c test_studentapi.py
-	- $(py) test_studentapi.py
+	- $(PY_VER) test_studentapi.py
 
+# remove build artifacts
 clean:
-	rm -f studentapi.c
-	rm -f $(BIN)/executor
-	rm -f executor
+	rm -f $(OBJS)
+	rm -f $(patsubst %.o,%.d,$(OBJS))
+	rm -rf studentapi.c
 	rm -f *.so
+	rm -rf build
+	rm -f $(BIN)/$(TARGET)
+
+-include $(patsubst %.o,%.d,$(OBJS))