Kernel improvements (#485)

* **New Features**
  * Added public accessors for querying module/device start and ready state.

* **Refactor**
  * Internal state moved to opaque internal objects; module/device/driver initializers now explicitly initialize internal pointers.
  * Lifecycle handling updated to construct/destruct internal state and use accessors.

* **Tests**
  * Tests updated to use public accessors and explicit construct/destruct lifecycle calls.

* **Chores**
  * Test build/include paths and small metadata updated.

TactilityKernel/Include/tactility/concurrent/mutex.h

@@ -18,12 +18,10 @@ struct Mutex {
 };
 
 inline static void mutex_construct(struct Mutex* mutex) {
-    assert(mutex->handle == NULL);
     mutex->handle = xSemaphoreCreateMutex();
 }
 
 inline static void mutex_destruct(struct Mutex* mutex) {
-    assert(mutex->handle != NULL);
     vPortAssertIfInISR();
     vSemaphoreDelete(mutex->handle);
     mutex->handle = NULL;

TactilityKernel/Include/tactility/device.h

@@ -16,39 +16,30 @@ extern "C" {
 #endif
 
 struct Driver;
-struct DevicePrivate;
+struct DeviceInternal;
 
 /** Enables discovering devices of the same type */
 struct DeviceType {
-    /* Placeholder because empty structs have a different size with C vs C++ compilers */
-    uint8_t _;
+    const char* name;
 };
 
 /** Represents a piece of hardware */
 struct Device {
     /** The name of the device. Valid characters: a-z a-Z 0-9 - _ . */
     const char* name;
+
     /** The configuration data for the device's driver */
     const void* config;
+
     /** The parent device that this device belongs to. Can be NULL, but only the root device should have a NULL parent. */
     struct Device* parent;
-    /** Internal data */
-    struct {
-        /** Address of the API exposed by the device instance. */
-        struct Driver* driver;
-        /** The driver data for this device (e.g. a mutex) */
-        void* driver_data;
-        /** The mutex for device operations */
-        struct Mutex mutex;
-        /** The device state */
-        struct {
-            int start_result;
-            bool started : 1;
-            bool added : 1;
-        } state;
-        /** Private data */
-        struct DevicePrivate* device_private;
-    } internal;
+
+    /**
+     * Internal state managed by the kernel.
+     * Device implementers should initialize this to NULL.
+     * Do not access or modify directly; use device_* functions.
+     */
+    struct DeviceInternal* internal;
 };
 
 /**

TactilityKernel/Include/tactility/driver.h

@@ -12,7 +12,7 @@ extern "C" {
 struct Device;
 struct DeviceType;
 struct Module;
-struct DriverPrivate;
+struct DriverInternal;
 
 struct Driver {
     /** The driver name */
@@ -29,8 +29,12 @@ struct Driver {
     const struct DeviceType* device_type;
     /** The module that owns this driver. When it is NULL, the system owns the driver and it cannot be removed from registration. */
     const struct Module* owner;
-    /** Internal data */
-    struct DriverPrivate* driver_private;
+    /**
+     * Internal state managed by the kernel.
+     * Driver implementers should initialize this to NULL.
+     * Do not access or modify directly; use driver_* functions.
+     */
+    struct DriverInternal* internal;
 };
 
 /**

TactilityKernel/Include/tactility/module.h

@@ -25,6 +25,8 @@ struct ModuleSymbol {
     const void* symbol;
 };
 
+struct ModuleInternal;
+
 /**
  * A module is a collection of drivers or other functionality that can be loaded and unloaded at runtime.
  */
@@ -36,7 +38,6 @@ struct Module {
      * Desirable format "platform-esp32", "lilygo-tdeck", etc.
      */
     const char* name;
-
     /**
      * A function to initialize the module.
      * Should never be NULL.
@@ -44,24 +45,24 @@ struct Module {
      * @return ERROR_NONE if successful
      */
     error_t (*start)(void);
-
     /**
      * Deinitializes the module.
      * Should never be NULL.
      * @return ERROR_NONE if successful
      */
     error_t (*stop)(void);
-
     /**
      * A list of symbols exported by the module.
      * Should be terminated by MODULE_SYMBOL_TERMINATOR.
      * Can be a NULL value.
      */
     const struct ModuleSymbol* symbols;
-
-    struct {
-        bool started;
-    } internal;
+    /**
+     * Internal state managed by the kernel.
+     * Module implementers should initialize this to NULL.
+     * Do not access or modify directly; use module_* functions.
+     */
+    struct ModuleInternal* internal;
 };
 
 /**

TactilityKernel/Source/device.cpp

@@ -13,8 +13,21 @@
 
 #define TAG "device"
 
-struct DevicePrivate {
-    std::vector<Device*> children;
+struct DeviceInternal {
+    /** Address of the API exposed by the device instance. */
+    struct Driver* driver = nullptr;
+    /** The driver data for this device (e.g. a mutex) */
+    void* driver_data = nullptr;
+    /** The mutex for device operations */
+    struct Mutex mutex {};
+    /** The device state */
+    struct {
+        int start_result = 0;
+        bool started : 1 = false;
+        bool added : 1 = false;
+    } state;
+    /** Attached child devices */
+    std::vector<Device*> children {};
 };
 
 struct DeviceLedger {
@@ -42,47 +55,55 @@ extern "C" {
 #define ledger_lock() mutex_lock(&ledger.mutex)
 #define ledger_unlock() mutex_unlock(&ledger.mutex)
 
-#define get_device_private(device) static_cast<DevicePrivate*>(device->internal.device_private)
+#define lock_internal(internal) mutex_lock(&internal->mutex)
+#define unlock_internal(internal) mutex_unlock(&internal->mutex)
 
 error_t device_construct(Device* device) {
-    device->internal.device_private = new(std::nothrow) DevicePrivate;
-    if (device->internal.device_private == nullptr) {
+    device->internal = new(std::nothrow) DeviceInternal;
+    if (device->internal == nullptr) {
         return ERROR_OUT_OF_MEMORY;
     }
     LOG_D(TAG, "construct %s", device->name);
-    mutex_construct(&device->internal.mutex);
+    mutex_construct(&device->internal->mutex);
     return ERROR_NONE;
 }
 
 error_t device_destruct(Device* device) {
-    if (device->internal.state.started || device->internal.state.added) {
+    lock_internal(device->internal);
+
+    auto* internal = device->internal;
+
+    if (internal->state.started || internal->state.added) {
+        unlock_internal(device->internal);
         return ERROR_INVALID_STATE;
     }
-    if (!get_device_private(device)->children.empty()) {
+    if (!internal->children.empty()) {
+        unlock_internal(device->internal);
         return ERROR_INVALID_STATE;
     }
     LOG_D(TAG, "destruct %s", device->name);
-    mutex_destruct(&device->internal.mutex);
-    delete get_device_private(device);
-    device->internal.device_private = nullptr;
+
+    device->internal = nullptr;
+    mutex_unlock(&internal->mutex);
+    delete internal;
+
     return ERROR_NONE;
 }
 
 /** Add a child to the list of children */
 static void device_add_child(struct Device* device, struct Device* child) {
     device_lock(device);
-    assert(device->internal.state.added);
-    get_device_private(device)->children.push_back(child);
+    check(device->internal->state.added);
+    device->internal->children.push_back(child);
     device_unlock(device);
 }
 
 /** Remove a child from the list of children */
 static void device_remove_child(struct Device* device, struct Device* child) {
     device_lock(device);
-    auto* parent_data = get_device_private(device);
-    const auto iterator = std::ranges::find(parent_data->children, child);
-    if (iterator != parent_data->children.end()) {
-        parent_data->children.erase(iterator);
+    const auto iterator = std::ranges::find(device->internal->children, child);
+    if (iterator != device->internal->children.end()) {
+        device->internal->children.erase(iterator);
     }
     device_unlock(device);
 }
@@ -91,7 +112,7 @@ error_t device_add(Device* device) {
     LOG_D(TAG, "add %s", device->name);
 
     // Already added
-    if (device->internal.state.started || device->internal.state.added) {
+    if (device->internal->state.started || device->internal->state.added) {
         return ERROR_INVALID_STATE;
     }
 
@@ -106,14 +127,14 @@ error_t device_add(Device* device) {
         device_add_child(parent, device);
     }
 
-    device->internal.state.added = true;
+    device->internal->state.added = true;
     return ERROR_NONE;
 }
 
 error_t device_remove(Device* device) {
     LOG_D(TAG, "remove %s", device->name);
 
-    if (device->internal.state.started || !device->internal.state.added) {
+    if (device->internal->state.started || !device->internal->state.added) {
         return ERROR_INVALID_STATE;
     }
 
@@ -132,7 +153,7 @@ error_t device_remove(Device* device) {
     ledger.devices.erase(iterator);
     ledger_unlock();
 
-    device->internal.state.added = false;
+    device->internal->state.added = false;
     return ERROR_NONE;
 
 failed_ledger_lookup:
@@ -147,41 +168,41 @@ failed_ledger_lookup:
 
 error_t device_start(Device* device) {
     LOG_I(TAG, "start %s", device->name);
-    if (!device->internal.state.added) {
+    if (!device->internal->state.added) {
         return ERROR_INVALID_STATE;
     }
 
-    if (device->internal.driver == nullptr) {
+    if (device->internal->driver == nullptr) {
         return ERROR_INVALID_STATE;
     }
 
     // Already started
-    if (device->internal.state.started) {
+    if (device->internal->state.started) {
         return ERROR_NONE;
     }
 
-    error_t bind_error = driver_bind(device->internal.driver, device);
-    device->internal.state.started = (bind_error == ERROR_NONE);
-    device->internal.state.start_result = bind_error;
+    error_t bind_error = driver_bind(device->internal->driver, device);
+    device->internal->state.started = (bind_error == ERROR_NONE);
+    device->internal->state.start_result = bind_error;
     return bind_error == ERROR_NONE ? ERROR_NONE : ERROR_RESOURCE;
 }
 
 error_t device_stop(struct Device* device) {
     LOG_I(TAG, "stop %s", device->name);
-    if (!device->internal.state.added) {
+    if (!device->internal->state.added) {
         return ERROR_INVALID_STATE;
     }
 
-    if (!device->internal.state.started) {
+    if (!device->internal->state.started) {
         return ERROR_NONE;
     }
 
-    if (driver_unbind(device->internal.driver, device) != ERROR_NONE) {
+    if (driver_unbind(device->internal->driver, device) != ERROR_NONE) {
         return ERROR_RESOURCE;
     }
 
-    device->internal.state.started = false;
-    device->internal.state.start_result = 0;
+    device->internal->state.started = false;
+    device->internal->state.start_result = 0;
     return ERROR_NONE;
 }
 
@@ -236,7 +257,7 @@ on_construct_add_error:
 }
 
 void device_set_parent(Device* device, Device* parent) {
-    assert(!device->internal.state.started);
+    assert(!device->internal->state.started);
     device->parent = parent;
 }
 
@@ -245,43 +266,43 @@ Device* device_get_parent(struct Device* device) {
 }
 
 void device_set_driver(struct Device* device, struct Driver* driver) {
-    device->internal.driver = driver;
+    device->internal->driver = driver;
 }
 
 struct Driver* device_get_driver(struct Device* device) {
-    return device->internal.driver;
+    return device->internal->driver;
 }
 
 bool device_is_ready(const struct Device* device) {
-    return device->internal.state.started;
+    return device->internal->state.started;
 }
 
 void device_set_driver_data(struct Device* device, void* driver_data) {
-    device->internal.driver_data = driver_data;
+    device->internal->driver_data = driver_data;
 }
 
 void* device_get_driver_data(struct Device* device) {
-    return device->internal.driver_data;
+    return device->internal->driver_data;
 }
 
 bool device_is_added(const struct Device* device) {
-    return device->internal.state.added;
+    return device->internal->state.added;
 }
 
 void device_lock(struct Device* device) {
-    mutex_lock(&device->internal.mutex);
+    mutex_lock(&device->internal->mutex);
 }
 
 bool device_try_lock(struct Device* device) {
-    return mutex_try_lock(&device->internal.mutex);
+    return mutex_try_lock(&device->internal->mutex);
 }
 
 void device_unlock(struct Device* device) {
-    mutex_unlock(&device->internal.mutex);
+    mutex_unlock(&device->internal->mutex);
 }
 
 const struct DeviceType* device_get_type(struct Device* device) {
-    return device->internal.driver ? device->internal.driver->device_type : NULL;
+    return device->internal->driver ? device->internal->driver->device_type : NULL;
 }
 
 void device_for_each(void* callback_context, bool(*on_device)(Device* device, void* context)) {
@@ -295,8 +316,7 @@ void device_for_each(void* callback_context, bool(*on_device)(Device* device, vo
 }
 
 void device_for_each_child(Device* device, void* callbackContext, bool(*on_device)(struct Device* device, void* context)) {
-    auto* data = get_device_private(device);
-    for (auto* child_device : data->children) {
+    for (auto* child_device : device->internal->children) {
         if (!on_device(child_device, callbackContext)) {
             break;
         }
@@ -306,7 +326,7 @@ void device_for_each_child(Device* device, void* callbackContext, bool(*on_devic
 void device_for_each_of_type(const DeviceType* type, void* callbackContext, bool(*on_device)(Device* device, void* context)) {
     ledger_lock();
     for (auto* device : ledger.devices) {
-        auto* driver = device->internal.driver;
+        auto* driver = device->internal->driver;
         if (driver != nullptr) {
             if (driver->device_type == type) {
                 if (!on_device(device, callbackContext)) {

TactilityKernel/Source/driver.cpp

@@ -12,23 +12,14 @@
 
 #define TAG "driver"
 
-struct DriverPrivate {
-    Mutex mutex { 0 };
+struct DriverInternal {
     int use_count = 0;
     bool destroying = false;
-
-    DriverPrivate() {
-        mutex_construct(&mutex);
-    }
-
-    ~DriverPrivate() {
-        mutex_destruct(&mutex);
-    }
 };
 
 struct DriverLedger {
     std::vector<Driver*> drivers;
-    Mutex mutex { 0 };
+    Mutex mutex {};
 
     DriverLedger() { mutex_construct(&mutex); }
     ~DriverLedger() { mutex_destruct(&mutex); }
@@ -37,43 +28,32 @@ struct DriverLedger {
     void unlock() { mutex_unlock(&mutex); }
 };
 
-static DriverLedger& get_ledger() {
-    static DriverLedger ledger;
-    return ledger;
-}
-
-#define ledger get_ledger()
-
-#define get_driver_private(driver) static_cast<DriverPrivate*>(driver->driver_private)
-#define driver_lock(driver) mutex_lock(&get_driver_private(driver)->mutex);
-#define driver_unlock(driver) mutex_unlock(&get_driver_private(driver)->mutex);
+static DriverLedger ledger;
 
 extern "C" {
 
 error_t driver_construct(Driver* driver) {
-    driver->driver_private = new(std::nothrow) DriverPrivate;
-    if (driver->driver_private == nullptr) {
+    driver->internal = new(std::nothrow) DriverInternal;
+    if (driver->internal == nullptr) {
         return ERROR_OUT_OF_MEMORY;
     }
     return ERROR_NONE;
 }
 
 error_t driver_destruct(Driver* driver) {
-    driver_lock(driver);
-    // No module means the system owns it and it cannot be destroyed
+    auto* internal = driver->internal;
+
     if (driver->owner == nullptr) {
-        driver_unlock(driver);
         return ERROR_NOT_ALLOWED;
     }
-    if (get_driver_private(driver)->use_count != 0 || get_driver_private(driver)->destroying) {
-        driver_unlock(driver);
+    if (internal->use_count != 0 || internal->destroying) {
         return ERROR_INVALID_STATE;
     }
-    get_driver_private(driver)->destroying = true;
+    internal->destroying = true;
 
-    driver_unlock(driver);
-    delete get_driver_private(driver);
-    driver->driver_private = nullptr;
+    // Nullify internal reference before deletion to prevent use-after-free
+    driver->internal = nullptr;
+    delete internal;
 
     return ERROR_NONE;
 }
@@ -143,10 +123,8 @@ Driver* driver_find_compatible(const char* compatible) {
 }
 
 error_t driver_bind(Driver* driver, Device* device) {
-    driver_lock(driver);
-
     error_t error = ERROR_NONE;
-    if (get_driver_private(driver)->destroying || !device_is_added(device)) {
+    if (driver->internal->destroying || !device_is_added(device)) {
         error = ERROR_INVALID_STATE;
         goto error;
     }
@@ -158,23 +136,18 @@ error_t driver_bind(Driver* driver, Device* device) {
         }
     }
 
-    get_driver_private(driver)->use_count++;
-    driver_unlock(driver);
+    driver->internal->use_count++;
 
     LOG_I(TAG, "bound %s to %s", driver->name, device->name);
     return ERROR_NONE;
 
 error:
-
-    driver_unlock(driver);
     return error;
 }
 
 error_t driver_unbind(Driver* driver, Device* device) {
-    driver_lock(driver);
-
     error_t error = ERROR_NONE;
-    if (get_driver_private(driver)->destroying || !device_is_added(device)) {
+    if (driver->internal->destroying || !device_is_added(device)) {
         error = ERROR_INVALID_STATE;
         goto error;
     }
@@ -186,16 +159,13 @@ error_t driver_unbind(Driver* driver, Device* device) {
         }
     }
 
-    get_driver_private(driver)->use_count--;
-    driver_unlock(driver);
+    driver->internal->use_count--;
 
     LOG_I(TAG, "unbound %s from %s", driver->name, device->name);
 
     return ERROR_NONE;
 
 error:
-
-    driver_unlock(driver);
     return error;
 }
 

TactilityKernel/Source/drivers/gpio_controller.cpp

@@ -32,6 +32,8 @@ error_t gpio_controller_get_pin_count(struct Device* device, uint32_t* count) {
     return GPIO_DRIVER_API(driver)->get_pin_count(device, count);
 }
 
-extern const struct DeviceType GPIO_CONTROLLER_TYPE { 0 };
+extern const struct DeviceType GPIO_CONTROLLER_TYPE {
+    .name = "gpio-controller"
+};
 
 }

TactilityKernel/Source/drivers/i2c_controller.cpp

@@ -49,6 +49,8 @@ error_t i2c_controller_has_device_at_address(Device* device, uint8_t address, Ti
     return I2C_DRIVER_API(driver)->write(device, address, message, 2, timeout);
 }
 
-extern const struct DeviceType I2C_CONTROLLER_TYPE { 0 };
+extern const struct DeviceType I2C_CONTROLLER_TYPE {
+    .name = "i2c-controller"
+};
 
 }

TactilityKernel/Source/drivers/i2s_controller.cpp

@@ -32,6 +32,8 @@ error_t i2s_controller_reset(struct Device* device) {
     return I2S_DRIVER_API(driver)->reset(device);
 }
 
-extern const struct DeviceType I2S_CONTROLLER_TYPE { 0 };
+extern const struct DeviceType I2S_CONTROLLER_TYPE {
+    .name = "i2s-controller"
+};
 
 }

TactilityKernel/Source/kernel_init.cpp

@@ -23,7 +23,8 @@ struct Module root_module = {
     .name = "kernel",
     .start = start,
     .stop = stop,
-    .symbols = (const struct ModuleSymbol*)KERNEL_SYMBOLS
+    .symbols = (const struct ModuleSymbol*)KERNEL_SYMBOLS,
+    .internal = nullptr
 };
 
 error_t kernel_init(struct Module* platform_module, struct Module* device_module, struct CompatibleDevice devicetree_devices[]) {

TactilityKernel/Source/module.cpp

@@ -1,14 +1,19 @@
-#include <vector>
-#include <string.h>
+#include <cstring>
 #include <algorithm>
+#include <new>
 #include <tactility/concurrent/mutex.h>
 #include <tactility/module.h>
+#include <vector>
 
 #define TAG "module"
 
+struct ModuleInternal {
+    bool started = false;
+};
+
 struct ModuleLedger {
     std::vector<struct Module*> modules;
-    struct Mutex mutex = { 0 };
+    struct Mutex mutex {};
 
     ModuleLedger() { mutex_construct(&mutex); }
     ~ModuleLedger() { mutex_destruct(&mutex); }
@@ -19,11 +24,14 @@ static ModuleLedger ledger;
 extern "C" {
 
 error_t module_construct(struct Module* module) {
-    module->internal.started = false;
+    module->internal = new (std::nothrow) ModuleInternal();
+    if (module->internal == nullptr) return ERROR_OUT_OF_MEMORY;
     return ERROR_NONE;
 }
 
 error_t module_destruct(struct Module* module) {
+    delete static_cast<ModuleInternal*>(module->internal);
+    module->internal = nullptr;
     return ERROR_NONE;
 }
 
@@ -44,28 +52,30 @@ error_t module_remove(struct Module* module) {
 error_t module_start(struct Module* module) {
     LOG_I(TAG, "start %s", module->name);
 
-    if (module->internal.started) return ERROR_NONE;
+    auto* internal = static_cast<ModuleInternal*>(module->internal);
+    if (internal->started) return ERROR_NONE;
 
     error_t error = module->start();
-    module->internal.started = (error == ERROR_NONE);
+    internal->started = (error == ERROR_NONE);
     return error;
 }
 
 bool module_is_started(struct Module* module) {
-    return module->internal.started;
+    return static_cast<ModuleInternal*>(module->internal)->started;
 }
 
 error_t module_stop(struct Module* module) {
     LOG_I(TAG, "stop %s", module->name);
 
-    if (!module->internal.started) return ERROR_NONE;
+    auto* internal = static_cast<ModuleInternal*>(module->internal);
+    if (!internal->started) return ERROR_NONE;
 
     error_t error = module->stop();
     if (error != ERROR_NONE) {
         return error;
     }
 
-    module->internal.started = false;
+    internal->started = false;
     return error;
 }
 
@@ -106,4 +116,3 @@ bool module_resolve_symbol_global(const char* symbol_name, uintptr_t* symbol_add
 }
 
 }
-