Kernel and buildscript improvements (#477)

* **New Features**
  * Centralized module management with global symbol resolution
  * Level-aware logging with colored prefixes and millisecond timestamps

* **Breaking Changes**
  * ModuleParent hierarchy and getModuleParent() removed
  * Logging API and adapter model replaced; LogLevel-driven log_generic signature changed

* **Improvements**
  * Unified, simplified module registration across build targets
  * Tests updated to reflect new module lifecycle and global symbol resolution

TactilityKernel/CMakeLists.txt

@@ -1,20 +1,20 @@
 cmake_minimum_required(VERSION 3.20)
 
-file(GLOB_RECURSE SOURCES "Source/*.c**")
+include("${CMAKE_CURRENT_LIST_DIR}/../Buildscripts/module.cmake")
+
+file(GLOB_RECURSE SOURCE_FILES "Source/*.c**")
+
+list(APPEND REQUIRES_LIST)
 
 if (DEFINED ENV{ESP_IDF_VERSION})
-
-    idf_component_register(
-        SRCS ${SOURCES}
-        INCLUDE_DIRS "Include/"
-        # TODO move the related logic for esp_time in Tactility/time.h into the Platform/ subproject
-        REQUIRES esp_timer
-    )
-
+    # TODO move the related logic for esp_time in Tactility/time.h into the Platform/ subproject
+    list(APPEND REQUIRES_LIST esp_timer)
 else ()
-
-    add_library(TactilityKernel OBJECT ${SOURCES})
-    target_include_directories(TactilityKernel PUBLIC Include/)
-    target_link_libraries(TactilityKernel PUBLIC freertos_kernel)
-
+    list(APPEND REQUIRES_LIST freertos_kernel)
 endif ()
+
+tactility_add_module(TactilityKernel
+    SRCS ${SOURCE_FILES}
+    INCLUDE_DIRS Include/
+    REQUIRES ${REQUIRES_LIST}
+)

TactilityKernel/Include/tactility/log.h

@@ -10,25 +10,32 @@
 extern "C" {
 #endif
 
-#define LOG_TAG(x) "\033[37m"#x"\033[0m"
+/** Used for log output filtering */
+enum LogLevel {
+    LOG_LEVEL_ERROR, /*!< Critical errors, software module can not recover on its own */
+    LOG_LEVEL_WARNING, /*!< Error conditions from which recovery measures have been taken */
+    LOG_LEVEL_INFO, /*!< Information messages which describe normal flow of events */
+    LOG_LEVEL_DEBUG, /*!< Extra information which is not necessary for normal use (values, pointers, sizes, etc). */
+    LOG_LEVEL_VERBOSE /*!< Bigger chunks of debugging information, or frequent messages which can potentially flood the output. */
+};
 
 #ifndef ESP_PLATFORM
 
-void log_generic(const char* tag, const char* format, ...);
+void log_generic(enum LogLevel level, const char* tag, const char* format, ...);
 
-#define LOG_E(x, ...) log_generic(x, ##__VA_ARGS__)
-#define LOG_W(x, ...) log_generic(x, ##__VA_ARGS__)
-#define LOG_I(x, ...) log_generic(x, ##__VA_ARGS__)
-#define LOG_D(x, ...) log_generic(x, ##__VA_ARGS__)
-#define LOG_V(x, ...) log_generic(x, ##__VA_ARGS__)
+#define LOG_E(tag, ...) log_generic(LOG_LEVEL_ERROR, tag, ##__VA_ARGS__)
+#define LOG_W(tag, ...) log_generic(LOG_LEVEL_WARNING, tag, ##__VA_ARGS__)
+#define LOG_I(tag, ...) log_generic(LOG_LEVEL_INFO, tag, ##__VA_ARGS__)
+#define LOG_D(tag, ...) log_generic(LOG_LEVEL_DEBUG, tag, ##__VA_ARGS__)
+#define LOG_V(tag, ...) log_generic(LOG_LEVEL_VERBOSE, tag, ##__VA_ARGS__)
 
 #else
 
-#define LOG_E(x, ...) ESP_LOGE(x, ##__VA_ARGS__)
-#define LOG_W(x, ...) ESP_LOGW(x, ##__VA_ARGS__)
-#define LOG_I(x, ...) ESP_LOGI(x, ##__VA_ARGS__)
-#define LOG_D(x, ...) ESP_LOGD(x, ##__VA_ARGS__)
-#define LOG_V(x, ...) ESP_LOGV(x, ##__VA_ARGS__)
+#define LOG_E(tag, ...) ESP_LOGE(tag, ##__VA_ARGS__)
+#define LOG_W(tag, ...) ESP_LOGW(tag, ##__VA_ARGS__)
+#define LOG_I(tag, ...) ESP_LOGI(tag, ##__VA_ARGS__)
+#define LOG_D(tag, ...) ESP_LOGD(tag, ##__VA_ARGS__)
+#define LOG_V(tag, ...) ESP_LOGV(tag, ##__VA_ARGS__)
 
 #endif
 

TactilityKernel/Include/tactility/module.h

@@ -6,15 +6,17 @@
 #include <stdint.h>
 
 #ifdef __cplusplus
-extern "C" {
+#define MODULE_SYMBOL_TERMINATOR { nullptr, nullptr }
+#else
+#define MODULE_SYMBOL_TERMINATOR { NULL, NULL }
 #endif
 
-struct ModuleParent;
-struct ModuleParentPrivate;
-
-#define MODULE_SYMBOL_TERMINATOR { NULL, NULL }
 #define DEFINE_MODULE_SYMBOL(symbol) { #symbol, (void*)&symbol }
 
+#ifdef __cplusplus
+extern "C" {
+#endif
+
 /** A binary symbol like a function or a variable. */
 struct ModuleSymbol {
     /** The name of the symbol. */
@@ -57,60 +59,45 @@ struct Module {
      */
     const struct ModuleSymbol* symbols;
 
+
     struct {
         bool started;
-        struct ModuleParent* parent;
     } internal;
 };
 
 /**
- * A module parent is a collection of modules that can be loaded and unloaded at runtime.
+ * @brief Construct a module instance.
+ * @param module module instance to construct
+ * @return ERROR_NONE if successful
  */
-struct ModuleParent {
-    /** The name of the parent module, for logging/debugging purposes */
-    const char* name;
-    struct ModuleParentPrivate* module_parent_private;
-};
+error_t module_construct(struct Module* module);
 
 /**
- * @brief Initialize the module parent.
- * @warn This function does no validation on input or state.
- * @param parent parent module
- * @return ERROR_NONE if successful, ERROR_OUT_OF_MEMORY if allocation fails
+ * @brief Destruct a module instance.
+ * @param module module instance to destruct
+ * @return ERROR_NONE if successful
  */
-error_t module_parent_construct(struct ModuleParent* parent);
+error_t module_destruct(struct Module* module);
 
 /**
- * @brief Deinitialize the module parent. Must have no children when calling this.
- * @warn This function does no validation on input.
- * @param parent parent module
- * @return ERROR_NONE if successful or ERROR_INVALID_STATE if the parent has children
+ * @brief Add a module to the system.
+ * @warning Only call this once. This function does not check if it was added before.
+ * @param module module to add
+ * @return ERROR_NONE if successful
  */
-error_t module_parent_destruct(struct ModuleParent* parent);
+error_t module_add(struct Module* module);
 
 /**
- * @brief Resolve a symbol from the module parent.
- * @details This function iterates through all started modules in the parent and attempts to resolve the symbol.
- * @param parent parent module
- * @param symbol_name name of the symbol to resolve
- * @param symbol_address pointer to store the address of the resolved symbol
- * @return true if the symbol was found, false otherwise
+ * @brief Remove a module from the system.
+ * @param module module to remove
+ * @return ERROR_NONE if successful
  */
-bool module_parent_resolve_symbol(struct ModuleParent* parent, const char* symbol_name, uintptr_t* symbol_address);
-
-/**
- * @brief Set the parent of the module.
- * @warning must call before module_start()
- * @param module module
- * @param parent nullable parent module
- * @return ERROR_NONE if successful, ERROR_INVALID_STATE if the module is already started
- */
-error_t module_set_parent(struct Module* module, struct ModuleParent* parent);
+error_t module_remove(struct Module* module);
 
 /**
  * @brief Start the module.
  * @param module module
- * @return ERROR_NONE if already started, ERROR_INVALID_STATE if the module doesn't have a parent, or otherwise it returns the result of the module's start function
+ * @return ERROR_NONE if already started, or otherwise it returns the result of the module's start function
  */
 error_t module_start(struct Module* module);
 
@@ -138,6 +125,15 @@ error_t module_stop(struct Module* module);
  */
 bool module_resolve_symbol(struct Module* module, const char* symbol_name, uintptr_t* symbol_address);
 
+/**
+ * @brief Resolve a symbol from any module
+ * @details This function iterates through all started modules in the parent and attempts to resolve the symbol.
+ * @param symbol_name name of the symbol to resolve
+ * @param symbol_address pointer to store the address of the resolved symbol
+ * @return true if the symbol was found, false otherwise
+ */
+bool module_resolve_symbol_global(const char* symbol_name, uintptr_t* symbol_address);
+
 #ifdef __cplusplus
 }
 #endif

TactilityKernel/Source/concurrent/dispatcher.cpp

@@ -11,7 +11,7 @@
 #include <tactility/log.h>
 #include <atomic>
 
-#define TAG LOG_TAG(Dispatcher)
+#define TAG "Dispatcher"
 
 static constexpr EventBits_t BACKPRESSURE_WARNING_COUNT = 100U;
 static constexpr EventBits_t WAIT_FLAG = 1U;

TactilityKernel/Source/concurrent/thread.cpp

@@ -11,7 +11,7 @@
 #include <string>
 
 static const size_t LOCAL_STORAGE_SELF_POINTER_INDEX = 0;
-static const char* TAG = LOG_TAG(Thread);
+static const char* TAG = "Thread";
 
 struct Thread {
     TaskHandle_t taskHandle = nullptr;

TactilityKernel/Source/crash.cpp

@@ -2,7 +2,7 @@
 #include <tactility/freertos/task.h>
 #include <tactility/log.h>
 
-static const auto* TAG = LOG_TAG(Kernel);
+static const auto* TAG = "Kernel";
 
 static void log_memory_info() {
 #ifdef ESP_PLATFORM

TactilityKernel/Source/device.cpp

@@ -11,7 +11,7 @@
 #include <sys/errno.h>
 #include <vector>
 
-#define TAG LOG_TAG(device)
+#define TAG "device"
 
 struct DevicePrivate {
     std::vector<Device*> children;

TactilityKernel/Source/driver.cpp

@@ -10,7 +10,7 @@
 #include <tactility/error.h>
 #include <tactility/log.h>
 
-#define TAG LOG_TAG(driver)
+#define TAG "driver"
 
 struct DriverPrivate {
     Mutex mutex { 0 };
@@ -30,21 +30,11 @@ struct DriverLedger {
     std::vector<Driver*> drivers;
     Mutex mutex { 0 };
 
-    DriverLedger() {
-        mutex_construct(&mutex);
-    }
+    DriverLedger() { mutex_construct(&mutex); }
+    ~DriverLedger() { mutex_destruct(&mutex); }
 
-    ~DriverLedger() {
-        mutex_destruct(&mutex);
-    }
-
-    void lock() {
-        mutex_lock(&mutex);
-    }
-
-    void unlock() {
-        mutex_unlock(&mutex);
-    }
+    void lock() { mutex_lock(&mutex); }
+    void unlock() { mutex_unlock(&mutex); }
 };
 
 static DriverLedger& get_ledger() {
@@ -99,6 +89,8 @@ error_t driver_add(Driver* driver) {
 error_t driver_remove(Driver* driver) {
     LOG_I(TAG, "remove %s", driver->name);
 
+    if (driver->owner == nullptr) return ERROR_NOT_ALLOWED;
+
     ledger.lock();
     const auto iterator = std::ranges::find(ledger.drivers, driver);
     if (iterator == ledger.drivers.end()) {

TactilityKernel/Source/drivers/root.cpp

@@ -12,8 +12,7 @@ Driver root_driver = {
     .stop_device = nullptr,
     .api = nullptr,
     .device_type = nullptr,
-    .owner = nullptr,
-    .driver_private = nullptr
+    .owner = nullptr
 };
 
 }

TactilityKernel/Source/kernel_init.cpp

@@ -5,12 +5,7 @@
 extern "C" {
 #endif
 
-#define TAG LOG_TAG(kernel)
-
-struct ModuleParent kernel_module_parent = {
-    "kernel",
-    nullptr
-};
+#define TAG "kernel"
 
 static error_t init_kernel_drivers() {
     extern Driver root_driver;
@@ -21,24 +16,37 @@ static error_t init_kernel_drivers() {
 error_t kernel_init(struct Module* platform_module, struct Module* device_module, struct CompatibleDevice devicetree_devices[]) {
     LOG_I(TAG, "init");
 
-    if (module_parent_construct(&kernel_module_parent) != ERROR_NONE) {
-        LOG_E(TAG, "init failed to create kernel module parent");
-        return ERROR_RESOURCE;
-    }
-
     if (init_kernel_drivers() != ERROR_NONE) {
         LOG_E(TAG, "init failed to init kernel drivers");
         return ERROR_RESOURCE;
     }
 
-    module_set_parent(platform_module, &kernel_module_parent);
+    if (module_construct(platform_module) != ERROR_NONE) {
+        LOG_E(TAG, "init failed to construct platform module");
+        return ERROR_RESOURCE;
+    }
+
+    if (module_add(platform_module) != ERROR_NONE) {
+        LOG_E(TAG, "init failed to add platform module");
+        return ERROR_RESOURCE;
+    }
+
     if (module_start(platform_module) != ERROR_NONE) {
         LOG_E(TAG, "init failed to start platform module");
         return ERROR_RESOURCE;
     }
 
     if (device_module != nullptr) {
-        module_set_parent(device_module, &kernel_module_parent);
+        if (module_construct(device_module) != ERROR_NONE) {
+            LOG_E(TAG, "init failed to construct device module");
+            return ERROR_RESOURCE;
+        }
+
+        if (module_add(device_module) != ERROR_NONE) {
+            LOG_E(TAG, "init failed to add device module");
+            return ERROR_RESOURCE;
+        }
+
         if (module_start(device_module) != ERROR_NONE) {
             LOG_E(TAG, "init failed to start device module");
             return ERROR_RESOURCE;
@@ -58,7 +66,7 @@ error_t kernel_init(struct Module* platform_module, struct Module* device_module
 
     LOG_I(TAG, "init done");
     return ERROR_NONE;
-};
+}
 
 #ifdef __cplusplus
 }

TactilityKernel/Source/log.cpp

@@ -4,17 +4,71 @@
 
 #include <tactility/log.h>
 
+#include <mutex>
+#include <inttypes.h>
 #include <stdio.h>
+#include <stdint.h>
 #include <stdarg.h>
+#include <sys/time.h>
+
+static const char* get_log_color(LogLevel level) {
+    using enum LogLevel;
+    switch (level) {
+        case LOG_LEVEL_ERROR:
+            return "\033[1;31m";
+        case LOG_LEVEL_WARNING:
+            return "\033[1;33m";
+        case LOG_LEVEL_INFO:
+            return "\033[32m";
+        case LOG_LEVEL_DEBUG:
+            return "\033[36m";
+        case LOG_LEVEL_VERBOSE:
+            return "\033[37m";
+        default:
+            return "";
+    }
+}
+
+static inline char get_log_prefix(LogLevel level) {
+    using enum LogLevel;
+    switch (level) {
+        case LOG_LEVEL_ERROR:
+            return 'E';
+        case LOG_LEVEL_WARNING:
+            return 'W';
+        case LOG_LEVEL_INFO:
+            return 'I';
+        case LOG_LEVEL_DEBUG:
+            return 'D';
+        case LOG_LEVEL_VERBOSE:
+            return 'V';
+        default:
+            return '?';
+    }
+}
+
+static uint64_t get_log_timestamp() {
+    static uint64_t base = 0U;
+    static std::once_flag init_flag;
+    std::call_once(init_flag, []() {
+      timeval time {};
+      gettimeofday(&time, nullptr);
+      base = ((uint64_t)time.tv_sec * 1000U) + (time.tv_usec / 1000U);
+    });
+    timeval time {};
+    gettimeofday(&time, nullptr);
+    uint64_t now = ((uint64_t)time.tv_sec * 1000U) + (time.tv_usec / 1000U);
+    return now - base;
+}
 
 extern "C" {
 
-void log_generic(const char* tag, const char* format, ...) {
+void log_generic(enum LogLevel level, const char* tag, const char* format, ...) {
     va_list args;
     va_start(args, format);
-    printf("%s ", tag);
+    printf("%s %c (%" PRIu64 ") %s ", get_log_color(level), get_log_prefix(level), get_log_timestamp(), tag);
     vprintf(format, args);
-    printf("\n");
+    printf("\033[0m\n");
     va_end(args);
 }
 

TactilityKernel/Source/module.cpp

@@ -4,88 +4,40 @@
 #include <tactility/concurrent/mutex.h>
 #include <tactility/module.h>
 
-#define TAG LOG_TAG(module)
+#define TAG "module"
 
-struct ModuleParentPrivate {
+struct ModuleLedger {
     std::vector<struct Module*> modules;
     struct Mutex mutex = { 0 };
+
+    ModuleLedger() { mutex_construct(&mutex); }
+    ~ModuleLedger() { mutex_destruct(&mutex); }
 };
 
+static ModuleLedger ledger;
+
 extern "C" {
 
-#pragma region module_parent
-
-error_t module_parent_construct(struct ModuleParent* parent) {
-    parent->module_parent_private = new (std::nothrow) ModuleParentPrivate();
-    if (!parent->module_parent_private) return ERROR_OUT_OF_MEMORY;
-
-    auto* data = static_cast<ModuleParentPrivate*>(parent->module_parent_private);
-    mutex_construct(&data->mutex);
-
+error_t module_construct(struct Module* module) {
+    module->internal.started = false;
     return ERROR_NONE;
 }
 
-error_t module_parent_destruct(struct ModuleParent* parent) {
-    auto* data = static_cast<ModuleParentPrivate*>(parent->module_parent_private);
-    if (data == nullptr) return ERROR_NONE;
-
-    mutex_lock(&data->mutex);
-    if (!data->modules.empty()) {
-        mutex_unlock(&data->mutex);
-        return ERROR_INVALID_STATE;
-    }
-    mutex_unlock(&data->mutex);
-
-    mutex_destruct(&data->mutex);
-    delete data;
-    parent->module_parent_private = nullptr;
+error_t module_destruct(struct Module* module) {
     return ERROR_NONE;
 }
 
-bool module_parent_resolve_symbol(ModuleParent* parent, const char* symbol_name, uintptr_t* symbol_address) {
-    auto* data = static_cast<ModuleParentPrivate*>(parent->module_parent_private);
-    mutex_lock(&data->mutex);
-    for (auto* module : data->modules) {
-        if (!module_is_started(module))
-            continue;
-        if (module_resolve_symbol(module, symbol_name, symbol_address)) {
-            mutex_unlock(&data->mutex);
-            return true;
-        }
-    }
-    mutex_unlock(&data->mutex);
-    return false;
+error_t module_add(struct Module* module) {
+    mutex_lock(&ledger.mutex);
+    ledger.modules.push_back(module);
+    mutex_unlock(&ledger.mutex);
+    return ERROR_NONE;
 }
 
-#pragma endregion
-
-#pragma region module
-
-error_t module_set_parent(struct Module* module, struct ModuleParent* parent) {
-    if (module->internal.started) return ERROR_INVALID_STATE;
-    if (module->internal.parent == parent) return ERROR_NONE;
-
-    // Remove from old parent
-    if (module->internal.parent && module->internal.parent->module_parent_private) {
-        auto* old_data = static_cast<ModuleParentPrivate*>(module->internal.parent->module_parent_private);
-        mutex_lock(&old_data->mutex);
-        auto it = std::find(old_data->modules.begin(), old_data->modules.end(), module);
-        if (it != old_data->modules.end()) {
-            old_data->modules.erase(it);
-        }
-        mutex_unlock(&old_data->mutex);
-    }
-
-    module->internal.parent = parent;
-
-    // Add to new parent
-    if (parent && parent->module_parent_private) {
-        auto* new_data = static_cast<ModuleParentPrivate*>(parent->module_parent_private);
-        mutex_lock(&new_data->mutex);
-        new_data->modules.push_back(module);
-        mutex_unlock(&new_data->mutex);
-    }
-
+error_t module_remove(struct Module* module) {
+    mutex_lock(&ledger.mutex);
+    ledger.modules.erase(std::remove(ledger.modules.begin(), ledger.modules.end(), module), ledger.modules.end());
+    mutex_unlock(&ledger.mutex);
     return ERROR_NONE;
 }
 
@@ -93,7 +45,6 @@ error_t module_start(struct Module* module) {
     LOG_I(TAG, "start %s", module->name);
 
     if (module->internal.started) return ERROR_NONE;
-    if (!module->internal.parent) return ERROR_INVALID_STATE;
 
     error_t error = module->start();
     module->internal.started = (error == ERROR_NONE);
@@ -132,7 +83,19 @@ bool module_resolve_symbol(Module* module, const char* symbol_name, uintptr_t* s
     return false;
 }
 
-#pragma endregion
+bool module_resolve_symbol_global(const char* symbol_name, uintptr_t* symbol_address) {
+    mutex_lock(&ledger.mutex);
+    for (auto* module : ledger.modules) {
+        if (!module_is_started(module))
+            continue;
+        if (module_resolve_symbol(module, symbol_name, symbol_address)) {
+            mutex_unlock(&ledger.mutex);
+            return true;
+        }
+    }
+    mutex_unlock(&ledger.mutex);
+    return false;
+}
 
 }