From ffa2ae7185f3083af7c043dafd8c9e4bc33465aa Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Fri, 30 Jan 2026 23:53:11 +0100 Subject: [PATCH] Made private API consistent --- .../Source/drivers/esp32_gpio.cpp | 2 +- .../Source/drivers/esp32_i2c.cpp | 2 +- Platforms/PlatformEsp32/Source/module.cpp | 2 +- TactilityKernel/Include/tactility/device.h | 3 +- TactilityKernel/Include/tactility/driver.h | 3 +- TactilityKernel/Include/tactility/module.h | 5 ++- TactilityKernel/Source/device.cpp | 20 ++++++------ TactilityKernel/Source/driver.cpp | 32 +++++++++---------- TactilityKernel/Source/drivers/root.cpp | 2 +- TactilityKernel/Source/kernel_init.cpp | 5 ++- TactilityKernel/Source/module.cpp | 20 ++++++------ Tests/TactilityKernel/DeviceTest.cpp | 8 ++--- .../TactilityKernel/DriverIntegrationTest.cpp | 2 +- Tests/TactilityKernel/DriverTest.cpp | 8 ++--- Tests/TactilityKernel/ModuleTest.cpp | 18 +++++------ 15 files changed, 68 insertions(+), 64 deletions(-) diff --git a/Platforms/PlatformEsp32/Source/drivers/esp32_gpio.cpp b/Platforms/PlatformEsp32/Source/drivers/esp32_gpio.cpp index d2e55482..1ff07cfb 100644 --- a/Platforms/PlatformEsp32/Source/drivers/esp32_gpio.cpp +++ b/Platforms/PlatformEsp32/Source/drivers/esp32_gpio.cpp @@ -117,7 +117,7 @@ Driver esp32_gpio_driver = { .api = (void*)&esp32_gpio_api, .deviceType = nullptr, .owner = &platform_module, - .internal = nullptr + .driver_private = nullptr }; } // extern "C" diff --git a/Platforms/PlatformEsp32/Source/drivers/esp32_i2c.cpp b/Platforms/PlatformEsp32/Source/drivers/esp32_i2c.cpp index 2e22f4e9..309aec82 100644 --- a/Platforms/PlatformEsp32/Source/drivers/esp32_i2c.cpp +++ b/Platforms/PlatformEsp32/Source/drivers/esp32_i2c.cpp @@ -216,7 +216,7 @@ Driver esp32_i2c_driver = { .api = (void*)&esp32_i2c_api, .deviceType = &I2C_CONTROLLER_TYPE, .owner = &platform_module, - .internal = nullptr + .driver_private = nullptr }; } // extern "C" diff --git a/Platforms/PlatformEsp32/Source/module.cpp b/Platforms/PlatformEsp32/Source/module.cpp index c165d202..13f47c34 100644 --- a/Platforms/PlatformEsp32/Source/module.cpp +++ b/Platforms/PlatformEsp32/Source/module.cpp @@ -8,7 +8,7 @@ extern Driver esp32_gpio_driver; extern Driver esp32_i2c_driver; static error_t start() { - /* We crash when destruct fails, because if a single driver fails to construct, + /* We crash when construct fails, because if a single driver fails to construct, * there is no guarantee that the previously constructed drivers can be destroyed */ check(driver_construct(&esp32_gpio_driver) == ERROR_NONE); check(driver_construct(&esp32_i2c_driver) == ERROR_NONE); diff --git a/TactilityKernel/Include/tactility/device.h b/TactilityKernel/Include/tactility/device.h index 64cb25f8..e1be7d12 100644 --- a/TactilityKernel/Include/tactility/device.h +++ b/TactilityKernel/Include/tactility/device.h @@ -16,6 +16,7 @@ extern "C" { #include struct Driver; +struct DevicePrivate; /** Enables discovering devices of the same type */ struct DeviceType { @@ -46,7 +47,7 @@ struct Device { bool added : 1; } state; /** Private data */ - void* data; + struct DevicePrivate* device_private; } internal; }; diff --git a/TactilityKernel/Include/tactility/driver.h b/TactilityKernel/Include/tactility/driver.h index fe316e45..92a1bcff 100644 --- a/TactilityKernel/Include/tactility/driver.h +++ b/TactilityKernel/Include/tactility/driver.h @@ -12,6 +12,7 @@ extern "C" { struct Device; struct DeviceType; struct Module; +struct DriverPrivate; struct Driver { /** The driver name */ @@ -29,7 +30,7 @@ struct Driver { /** 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 */ - void* internal; + struct DriverPrivate* driver_private; }; error_t driver_construct(struct Driver* driver); diff --git a/TactilityKernel/Include/tactility/module.h b/TactilityKernel/Include/tactility/module.h index 45a8bb71..b94db3bb 100644 --- a/TactilityKernel/Include/tactility/module.h +++ b/TactilityKernel/Include/tactility/module.h @@ -8,6 +8,7 @@ extern "C" { #include struct ModuleParent; +struct ModuleParentPrivate; /** * A module is a collection of drivers or other functionality that can be loaded and unloaded at runtime. @@ -46,9 +47,7 @@ struct Module { struct ModuleParent { /** The name of the parent module, for logging/debugging purposes */ const char* name; - struct { - void* data; - } internal; + struct ModuleParentPrivate* module_parent_private; }; /** diff --git a/TactilityKernel/Source/device.cpp b/TactilityKernel/Source/device.cpp index dcb3db86..5860165a 100644 --- a/TactilityKernel/Source/device.cpp +++ b/TactilityKernel/Source/device.cpp @@ -13,7 +13,7 @@ #define TAG LOG_TAG(device) -struct DeviceData { +struct DevicePrivate { std::vector children; }; @@ -42,11 +42,11 @@ extern "C" { #define ledger_lock() mutex_lock(&ledger.mutex) #define ledger_unlock() mutex_unlock(&ledger.mutex) -#define get_device_data(device) static_cast(device->internal.data) +#define get_device_private(device) static_cast(device->internal.device_private) error_t device_construct(Device* device) { - device->internal.data = new(std::nothrow) DeviceData; - if (device->internal.data == nullptr) { + device->internal.device_private = new(std::nothrow) DevicePrivate; + if (device->internal.device_private == nullptr) { return ERROR_OUT_OF_MEMORY; } LOG_I(TAG, "construct %s", device->name); @@ -58,13 +58,13 @@ error_t device_destruct(Device* device) { if (device->internal.state.started || device->internal.state.added) { return ERROR_INVALID_STATE; } - if (!get_device_data(device)->children.empty()) { + if (!get_device_private(device)->children.empty()) { return ERROR_INVALID_STATE; } LOG_I(TAG, "destruct %s", device->name); mutex_destruct(&device->internal.mutex); - delete get_device_data(device); - device->internal.data = nullptr; + delete get_device_private(device); + device->internal.device_private = nullptr; return ERROR_NONE; } @@ -72,14 +72,14 @@ error_t device_destruct(Device* device) { static void device_add_child(struct Device* device, struct Device* child) { device_lock(device); assert(device->internal.state.added); - get_device_data(device)->children.push_back(child); + get_device_private(device)->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_data(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); @@ -251,7 +251,7 @@ void for_each_device(void* callback_context, bool(*on_device)(Device* device, vo } void for_each_device_child(Device* device, void* callbackContext, bool(*on_device)(struct Device* device, void* context)) { - auto* data = get_device_data(device); + auto* data = get_device_private(device); for (auto* child_device : data->children) { if (!on_device(child_device, callbackContext)) { break; diff --git a/TactilityKernel/Source/driver.cpp b/TactilityKernel/Source/driver.cpp index 9fd875cd..17da77fe 100644 --- a/TactilityKernel/Source/driver.cpp +++ b/TactilityKernel/Source/driver.cpp @@ -12,16 +12,16 @@ #define TAG LOG_TAG(driver) -struct DriverInternal { +struct DriverPrivate { Mutex mutex { 0 }; int use_count = 0; bool destroying = false; - DriverInternal() { + DriverPrivate() { mutex_construct(&mutex); } - ~DriverInternal() { + ~DriverPrivate() { mutex_destruct(&mutex); } }; @@ -54,9 +54,9 @@ static DriverLedger& get_ledger() { #define ledger get_ledger() -#define get_internal(driver) static_cast(driver->internal) -#define driver_lock(driver) mutex_lock(&get_internal(driver)->mutex); -#define driver_unlock(driver) mutex_unlock(&get_internal(driver)->mutex); +#define get_driver_private(driver) static_cast(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 void driver_add(Driver* driver) { LOG_I(TAG, "add %s", driver->name); @@ -83,8 +83,8 @@ static error_t driver_remove(Driver* driver) { extern "C" { error_t driver_construct(Driver* driver) { - driver->internal = new(std::nothrow) DriverInternal; - if (driver->internal == nullptr) { + driver->driver_private = new(std::nothrow) DriverPrivate; + if (driver->driver_private == nullptr) { return ERROR_OUT_OF_MEMORY; } driver_add(driver); @@ -98,19 +98,19 @@ error_t driver_destruct(Driver* driver) { driver_unlock(driver); return ERROR_NOT_ALLOWED; } - if (get_internal(driver)->use_count != 0 || get_internal(driver)->destroying) { + if (get_driver_private(driver)->use_count != 0 || get_driver_private(driver)->destroying) { driver_unlock(driver); return ERROR_INVALID_STATE; } - get_internal(driver)->destroying = true; + get_driver_private(driver)->destroying = true; if (driver_remove(driver) != ERROR_NONE) { LOG_W(TAG, "Failed to remove driver from ledger: %s", driver->name); } driver_unlock(driver); - delete get_internal(driver); - driver->internal = nullptr; + delete get_driver_private(driver); + driver->driver_private = nullptr; return ERROR_NONE; } @@ -146,7 +146,7 @@ error_t driver_bind(Driver* driver, Device* device) { driver_lock(driver); error_t error = ERROR_NONE; - if (get_internal(driver)->destroying || !device_is_added(device)) { + if (get_driver_private(driver)->destroying || !device_is_added(device)) { error = ERROR_INVALID_STATE; goto error; } @@ -158,7 +158,7 @@ error_t driver_bind(Driver* driver, Device* device) { } } - get_internal(driver)->use_count++; + get_driver_private(driver)->use_count++; driver_unlock(driver); LOG_I(TAG, "bound %s to %s", driver->name, device->name); @@ -174,7 +174,7 @@ error_t driver_unbind(Driver* driver, Device* device) { driver_lock(driver); error_t error = ERROR_NONE; - if (get_internal(driver)->destroying || !device_is_added(device)) { + if (get_driver_private(driver)->destroying || !device_is_added(device)) { error = ERROR_INVALID_STATE; goto error; } @@ -186,7 +186,7 @@ error_t driver_unbind(Driver* driver, Device* device) { } } - get_internal(driver)->use_count--; + get_driver_private(driver)->use_count--; driver_unlock(driver); LOG_I(TAG, "unbound %s from %s", driver->name, device->name); diff --git a/TactilityKernel/Source/drivers/root.cpp b/TactilityKernel/Source/drivers/root.cpp index 1be6b474..524a37f9 100644 --- a/TactilityKernel/Source/drivers/root.cpp +++ b/TactilityKernel/Source/drivers/root.cpp @@ -13,7 +13,7 @@ Driver root_driver = { .api = nullptr, .deviceType = nullptr, .owner = nullptr, - .internal = nullptr + .driver_private = nullptr }; } diff --git a/TactilityKernel/Source/kernel_init.cpp b/TactilityKernel/Source/kernel_init.cpp index da46d446..083926e9 100644 --- a/TactilityKernel/Source/kernel_init.cpp +++ b/TactilityKernel/Source/kernel_init.cpp @@ -7,7 +7,10 @@ extern "C" { #define TAG LOG_TAG(kernel) -struct ModuleParent kernel_module_parent; +struct ModuleParent kernel_module_parent = { + "kernel", + nullptr +}; static error_t init_kernel_drivers() { extern Driver root_driver; diff --git a/TactilityKernel/Source/module.cpp b/TactilityKernel/Source/module.cpp index da730df1..92f347af 100644 --- a/TactilityKernel/Source/module.cpp +++ b/TactilityKernel/Source/module.cpp @@ -3,7 +3,7 @@ #include #include -struct ModuleParentInternal { +struct ModuleParentPrivate { std::vector modules; struct Mutex mutex = { 0 }; }; @@ -13,17 +13,17 @@ extern "C" { #pragma region module_parent error_t module_parent_construct(struct ModuleParent* parent) { - parent->internal.data = new (std::nothrow) ModuleParentInternal(); - if (!parent->internal.data) return ERROR_OUT_OF_MEMORY; + parent->module_parent_private = new (std::nothrow) ModuleParentPrivate(); + if (!parent->module_parent_private) return ERROR_OUT_OF_MEMORY; - auto* data = static_cast(parent->internal.data); + auto* data = static_cast(parent->module_parent_private); mutex_construct(&data->mutex); return ERROR_NONE; } error_t module_parent_destruct(struct ModuleParent* parent) { - auto* data = static_cast(parent->internal.data); + auto* data = static_cast(parent->module_parent_private); if (data == nullptr) return ERROR_NONE; mutex_lock(&data->mutex); @@ -35,7 +35,7 @@ error_t module_parent_destruct(struct ModuleParent* parent) { mutex_destruct(&data->mutex); delete data; - parent->internal.data = nullptr; + parent->module_parent_private = nullptr; return ERROR_NONE; } @@ -48,8 +48,8 @@ error_t module_set_parent(struct Module* module, struct ModuleParent* parent) { if (module->internal.parent == parent) return ERROR_NONE; // Remove from old parent - if (module->internal.parent && module->internal.parent->internal.data) { - auto* old_data = static_cast(module->internal.parent->internal.data); + if (module->internal.parent && module->internal.parent->module_parent_private) { + auto* old_data = static_cast(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()) { @@ -61,8 +61,8 @@ error_t module_set_parent(struct Module* module, struct ModuleParent* parent) { module->internal.parent = parent; // Add to new parent - if (parent && parent->internal.data) { - auto* new_data = static_cast(parent->internal.data); + if (parent && parent->module_parent_private) { + auto* new_data = static_cast(parent->module_parent_private); mutex_lock(&new_data->mutex); new_data->modules.push_back(module); mutex_unlock(&new_data->mutex); diff --git a/Tests/TactilityKernel/DeviceTest.cpp b/Tests/TactilityKernel/DeviceTest.cpp index 06acb6fa..c53633a1 100644 --- a/Tests/TactilityKernel/DeviceTest.cpp +++ b/Tests/TactilityKernel/DeviceTest.cpp @@ -18,16 +18,16 @@ TEST_CASE("device_construct and device_destruct should set and unset the correct error_t error = device_construct(&device); CHECK_EQ(error, ERROR_NONE); - CHECK_NE(device.internal.data, nullptr); + CHECK_NE(device.internal.device_private, nullptr); CHECK_NE(device.internal.mutex.handle, nullptr); CHECK_EQ(device_destruct(&device), ERROR_NONE); - CHECK_EQ(device.internal.data, nullptr); + CHECK_EQ(device.internal.device_private, nullptr); CHECK_EQ(device.internal.mutex.handle, nullptr); Device comparison_device = { 0 }; - comparison_device.internal.data = device.internal.data; + comparison_device.internal.device_private = device.internal.device_private; comparison_device.internal.mutex.handle = device.internal.mutex.handle; // Check that no other data was set @@ -172,7 +172,7 @@ TEST_CASE("device_is_ready should return true only when it is started") { .api = nullptr, .deviceType = nullptr, .owner = &module, - .internal = nullptr + .driver_private = nullptr }; Device device = { 0 }; diff --git a/Tests/TactilityKernel/DriverIntegrationTest.cpp b/Tests/TactilityKernel/DriverIntegrationTest.cpp index 70fae766..fd54bd07 100644 --- a/Tests/TactilityKernel/DriverIntegrationTest.cpp +++ b/Tests/TactilityKernel/DriverIntegrationTest.cpp @@ -38,7 +38,7 @@ static Driver integration_driver = { .api = nullptr, .deviceType = nullptr, .owner = &module, - .internal = nullptr, + .driver_private = nullptr, }; TEST_CASE("driver with with start success and stop success should start and stop a device") { diff --git a/Tests/TactilityKernel/DriverTest.cpp b/Tests/TactilityKernel/DriverTest.cpp index 5661f8f1..82cb227b 100644 --- a/Tests/TactilityKernel/DriverTest.cpp +++ b/Tests/TactilityKernel/DriverTest.cpp @@ -14,9 +14,9 @@ TEST_CASE("driver_construct and driver_destruct should set and unset the correct driver.owner = &module; CHECK_EQ(driver_construct(&driver), ERROR_NONE); - CHECK_NE(driver.internal, nullptr); + CHECK_NE(driver.driver_private, nullptr); CHECK_EQ(driver_destruct(&driver), ERROR_NONE); - CHECK_EQ(driver.internal, nullptr); + CHECK_EQ(driver.driver_private, nullptr); } TEST_CASE("a driver without a module should not be destrucable") { @@ -38,7 +38,7 @@ TEST_CASE("driver_is_compatible should return true if a compatible value is foun .api = nullptr, .deviceType = nullptr, .owner = &module, - .internal = nullptr + .driver_private = nullptr }; CHECK_EQ(driver_is_compatible(&driver, "test_compatible"), true); CHECK_EQ(driver_is_compatible(&driver, "nope"), false); @@ -55,7 +55,7 @@ TEST_CASE("driver_find should only find a compatible driver when the driver was .api = nullptr, .deviceType = nullptr, .owner = &module, - .internal = nullptr + .driver_private = nullptr }; Driver* found_driver = driver_find_compatible("test_compatible"); diff --git a/Tests/TactilityKernel/ModuleTest.cpp b/Tests/TactilityKernel/ModuleTest.cpp index 8af7a531..303ec824 100644 --- a/Tests/TactilityKernel/ModuleTest.cpp +++ b/Tests/TactilityKernel/ModuleTest.cpp @@ -16,19 +16,19 @@ static error_t test_stop() { } TEST_CASE("ModuleParent construction and destruction") { - struct ModuleParent parent = { "test_parent", { nullptr } }; + struct ModuleParent parent = { "test_parent", nullptr }; // Test successful construction CHECK_EQ(module_parent_construct(&parent), ERROR_NONE); - CHECK_NE(parent.internal.data, nullptr); + CHECK_NE(parent.module_parent_private, nullptr); // Test successful destruction CHECK_EQ(module_parent_destruct(&parent), ERROR_NONE); - CHECK_EQ(parent.internal.data, nullptr); + CHECK_EQ(parent.module_parent_private, nullptr); } TEST_CASE("ModuleParent destruction with children") { - struct ModuleParent parent = { "parent", { nullptr } }; + struct ModuleParent parent = { "parent", nullptr }; REQUIRE_EQ(module_parent_construct(&parent), ERROR_NONE); struct Module module = { @@ -42,19 +42,19 @@ TEST_CASE("ModuleParent destruction with children") { // Should fail to destruct because it has a child CHECK_EQ(module_parent_destruct(&parent), ERROR_INVALID_STATE); - CHECK_NE(parent.internal.data, nullptr); + CHECK_NE(parent.module_parent_private, nullptr); // Remove child REQUIRE_EQ(module_set_parent(&module, nullptr), ERROR_NONE); // Now it should succeed CHECK_EQ(module_parent_destruct(&parent), ERROR_NONE); - CHECK_EQ(parent.internal.data, nullptr); + CHECK_EQ(parent.module_parent_private, nullptr); } TEST_CASE("Module parent management") { - struct ModuleParent parent1 = { "parent1", { nullptr } }; - struct ModuleParent parent2 = { "parent2", { nullptr } }; + struct ModuleParent parent1 = { "parent1", nullptr }; + struct ModuleParent parent2 = { "parent2", nullptr }; REQUIRE_EQ(module_parent_construct(&parent1), ERROR_NONE); REQUIRE_EQ(module_parent_construct(&parent2), ERROR_NONE); @@ -88,7 +88,7 @@ TEST_CASE("Module parent management") { } TEST_CASE("Module lifecycle") { - struct ModuleParent parent = { "parent", { nullptr } }; + struct ModuleParent parent = { "parent", nullptr }; REQUIRE_EQ(module_parent_construct(&parent), ERROR_NONE); start_called = false;