Made private API consistent

This commit is contained in:
Ken Van Hoeylandt 2026-01-30 23:53:11 +01:00
parent 7e0bd90e73
commit ffa2ae7185
15 changed files with 68 additions and 64 deletions

View File

@ -117,7 +117,7 @@ Driver esp32_gpio_driver = {
.api = (void*)&esp32_gpio_api, .api = (void*)&esp32_gpio_api,
.deviceType = nullptr, .deviceType = nullptr,
.owner = &platform_module, .owner = &platform_module,
.internal = nullptr .driver_private = nullptr
}; };
} // extern "C" } // extern "C"

View File

@ -216,7 +216,7 @@ Driver esp32_i2c_driver = {
.api = (void*)&esp32_i2c_api, .api = (void*)&esp32_i2c_api,
.deviceType = &I2C_CONTROLLER_TYPE, .deviceType = &I2C_CONTROLLER_TYPE,
.owner = &platform_module, .owner = &platform_module,
.internal = nullptr .driver_private = nullptr
}; };
} // extern "C" } // extern "C"

View File

@ -8,7 +8,7 @@ extern Driver esp32_gpio_driver;
extern Driver esp32_i2c_driver; extern Driver esp32_i2c_driver;
static error_t start() { 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 */ * there is no guarantee that the previously constructed drivers can be destroyed */
check(driver_construct(&esp32_gpio_driver) == ERROR_NONE); check(driver_construct(&esp32_gpio_driver) == ERROR_NONE);
check(driver_construct(&esp32_i2c_driver) == ERROR_NONE); check(driver_construct(&esp32_i2c_driver) == ERROR_NONE);

View File

@ -16,6 +16,7 @@ extern "C" {
#include <tactility/concurrent/mutex.h> #include <tactility/concurrent/mutex.h>
struct Driver; struct Driver;
struct DevicePrivate;
/** Enables discovering devices of the same type */ /** Enables discovering devices of the same type */
struct DeviceType { struct DeviceType {
@ -46,7 +47,7 @@ struct Device {
bool added : 1; bool added : 1;
} state; } state;
/** Private data */ /** Private data */
void* data; struct DevicePrivate* device_private;
} internal; } internal;
}; };

View File

@ -12,6 +12,7 @@ extern "C" {
struct Device; struct Device;
struct DeviceType; struct DeviceType;
struct Module; struct Module;
struct DriverPrivate;
struct Driver { struct Driver {
/** The driver name */ /** 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. */ /** 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; const struct Module* owner;
/** Internal data */ /** Internal data */
void* internal; struct DriverPrivate* driver_private;
}; };
error_t driver_construct(struct Driver* driver); error_t driver_construct(struct Driver* driver);

View File

@ -8,6 +8,7 @@ extern "C" {
#include <stdbool.h> #include <stdbool.h>
struct ModuleParent; struct ModuleParent;
struct ModuleParentPrivate;
/** /**
* A module is a collection of drivers or other functionality that can be loaded and unloaded at runtime. * 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 { struct ModuleParent {
/** The name of the parent module, for logging/debugging purposes */ /** The name of the parent module, for logging/debugging purposes */
const char* name; const char* name;
struct { struct ModuleParentPrivate* module_parent_private;
void* data;
} internal;
}; };
/** /**

View File

@ -13,7 +13,7 @@
#define TAG LOG_TAG(device) #define TAG LOG_TAG(device)
struct DeviceData { struct DevicePrivate {
std::vector<Device*> children; std::vector<Device*> children;
}; };
@ -42,11 +42,11 @@ extern "C" {
#define ledger_lock() mutex_lock(&ledger.mutex) #define ledger_lock() mutex_lock(&ledger.mutex)
#define ledger_unlock() mutex_unlock(&ledger.mutex) #define ledger_unlock() mutex_unlock(&ledger.mutex)
#define get_device_data(device) static_cast<DeviceData*>(device->internal.data) #define get_device_private(device) static_cast<DevicePrivate*>(device->internal.device_private)
error_t device_construct(Device* device) { error_t device_construct(Device* device) {
device->internal.data = new(std::nothrow) DeviceData; device->internal.device_private = new(std::nothrow) DevicePrivate;
if (device->internal.data == nullptr) { if (device->internal.device_private == nullptr) {
return ERROR_OUT_OF_MEMORY; return ERROR_OUT_OF_MEMORY;
} }
LOG_I(TAG, "construct %s", device->name); 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) { if (device->internal.state.started || device->internal.state.added) {
return ERROR_INVALID_STATE; return ERROR_INVALID_STATE;
} }
if (!get_device_data(device)->children.empty()) { if (!get_device_private(device)->children.empty()) {
return ERROR_INVALID_STATE; return ERROR_INVALID_STATE;
} }
LOG_I(TAG, "destruct %s", device->name); LOG_I(TAG, "destruct %s", device->name);
mutex_destruct(&device->internal.mutex); mutex_destruct(&device->internal.mutex);
delete get_device_data(device); delete get_device_private(device);
device->internal.data = nullptr; device->internal.device_private = nullptr;
return ERROR_NONE; return ERROR_NONE;
} }
@ -72,14 +72,14 @@ error_t device_destruct(Device* device) {
static void device_add_child(struct Device* device, struct Device* child) { static void device_add_child(struct Device* device, struct Device* child) {
device_lock(device); device_lock(device);
assert(device->internal.state.added); assert(device->internal.state.added);
get_device_data(device)->children.push_back(child); get_device_private(device)->children.push_back(child);
device_unlock(device); device_unlock(device);
} }
/** Remove a child from the list of children */ /** Remove a child from the list of children */
static void device_remove_child(struct Device* device, struct Device* child) { static void device_remove_child(struct Device* device, struct Device* child) {
device_lock(device); 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); const auto iterator = std::ranges::find(parent_data->children, child);
if (iterator != parent_data->children.end()) { if (iterator != parent_data->children.end()) {
parent_data->children.erase(iterator); 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)) { 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) { for (auto* child_device : data->children) {
if (!on_device(child_device, callbackContext)) { if (!on_device(child_device, callbackContext)) {
break; break;

View File

@ -12,16 +12,16 @@
#define TAG LOG_TAG(driver) #define TAG LOG_TAG(driver)
struct DriverInternal { struct DriverPrivate {
Mutex mutex { 0 }; Mutex mutex { 0 };
int use_count = 0; int use_count = 0;
bool destroying = false; bool destroying = false;
DriverInternal() { DriverPrivate() {
mutex_construct(&mutex); mutex_construct(&mutex);
} }
~DriverInternal() { ~DriverPrivate() {
mutex_destruct(&mutex); mutex_destruct(&mutex);
} }
}; };
@ -54,9 +54,9 @@ static DriverLedger& get_ledger() {
#define ledger get_ledger() #define ledger get_ledger()
#define get_internal(driver) static_cast<DriverInternal*>(driver->internal) #define get_driver_private(driver) static_cast<DriverPrivate*>(driver->driver_private)
#define driver_lock(driver) mutex_lock(&get_internal(driver)->mutex); #define driver_lock(driver) mutex_lock(&get_driver_private(driver)->mutex);
#define driver_unlock(driver) mutex_unlock(&get_internal(driver)->mutex); #define driver_unlock(driver) mutex_unlock(&get_driver_private(driver)->mutex);
static void driver_add(Driver* driver) { static void driver_add(Driver* driver) {
LOG_I(TAG, "add %s", driver->name); LOG_I(TAG, "add %s", driver->name);
@ -83,8 +83,8 @@ static error_t driver_remove(Driver* driver) {
extern "C" { extern "C" {
error_t driver_construct(Driver* driver) { error_t driver_construct(Driver* driver) {
driver->internal = new(std::nothrow) DriverInternal; driver->driver_private = new(std::nothrow) DriverPrivate;
if (driver->internal == nullptr) { if (driver->driver_private == nullptr) {
return ERROR_OUT_OF_MEMORY; return ERROR_OUT_OF_MEMORY;
} }
driver_add(driver); driver_add(driver);
@ -98,19 +98,19 @@ error_t driver_destruct(Driver* driver) {
driver_unlock(driver); driver_unlock(driver);
return ERROR_NOT_ALLOWED; 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); driver_unlock(driver);
return ERROR_INVALID_STATE; return ERROR_INVALID_STATE;
} }
get_internal(driver)->destroying = true; get_driver_private(driver)->destroying = true;
if (driver_remove(driver) != ERROR_NONE) { if (driver_remove(driver) != ERROR_NONE) {
LOG_W(TAG, "Failed to remove driver from ledger: %s", driver->name); LOG_W(TAG, "Failed to remove driver from ledger: %s", driver->name);
} }
driver_unlock(driver); driver_unlock(driver);
delete get_internal(driver); delete get_driver_private(driver);
driver->internal = nullptr; driver->driver_private = nullptr;
return ERROR_NONE; return ERROR_NONE;
} }
@ -146,7 +146,7 @@ error_t driver_bind(Driver* driver, Device* device) {
driver_lock(driver); driver_lock(driver);
error_t error = ERROR_NONE; 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; error = ERROR_INVALID_STATE;
goto error; 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); driver_unlock(driver);
LOG_I(TAG, "bound %s to %s", driver->name, device->name); 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); driver_lock(driver);
error_t error = ERROR_NONE; 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; error = ERROR_INVALID_STATE;
goto error; 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); driver_unlock(driver);
LOG_I(TAG, "unbound %s from %s", driver->name, device->name); LOG_I(TAG, "unbound %s from %s", driver->name, device->name);

View File

@ -13,7 +13,7 @@ Driver root_driver = {
.api = nullptr, .api = nullptr,
.deviceType = nullptr, .deviceType = nullptr,
.owner = nullptr, .owner = nullptr,
.internal = nullptr .driver_private = nullptr
}; };
} }

View File

@ -7,7 +7,10 @@ extern "C" {
#define TAG LOG_TAG(kernel) #define TAG LOG_TAG(kernel)
struct ModuleParent kernel_module_parent; struct ModuleParent kernel_module_parent = {
"kernel",
nullptr
};
static error_t init_kernel_drivers() { static error_t init_kernel_drivers() {
extern Driver root_driver; extern Driver root_driver;

View File

@ -3,7 +3,7 @@
#include <tactility/concurrent/mutex.h> #include <tactility/concurrent/mutex.h>
#include <tactility/module.h> #include <tactility/module.h>
struct ModuleParentInternal { struct ModuleParentPrivate {
std::vector<struct Module*> modules; std::vector<struct Module*> modules;
struct Mutex mutex = { 0 }; struct Mutex mutex = { 0 };
}; };
@ -13,17 +13,17 @@ extern "C" {
#pragma region module_parent #pragma region module_parent
error_t module_parent_construct(struct ModuleParent* parent) { error_t module_parent_construct(struct ModuleParent* parent) {
parent->internal.data = new (std::nothrow) ModuleParentInternal(); parent->module_parent_private = new (std::nothrow) ModuleParentPrivate();
if (!parent->internal.data) return ERROR_OUT_OF_MEMORY; if (!parent->module_parent_private) return ERROR_OUT_OF_MEMORY;
auto* data = static_cast<ModuleParentInternal*>(parent->internal.data); auto* data = static_cast<ModuleParentPrivate*>(parent->module_parent_private);
mutex_construct(&data->mutex); mutex_construct(&data->mutex);
return ERROR_NONE; return ERROR_NONE;
} }
error_t module_parent_destruct(struct ModuleParent* parent) { error_t module_parent_destruct(struct ModuleParent* parent) {
auto* data = static_cast<ModuleParentInternal*>(parent->internal.data); auto* data = static_cast<ModuleParentPrivate*>(parent->module_parent_private);
if (data == nullptr) return ERROR_NONE; if (data == nullptr) return ERROR_NONE;
mutex_lock(&data->mutex); mutex_lock(&data->mutex);
@ -35,7 +35,7 @@ error_t module_parent_destruct(struct ModuleParent* parent) {
mutex_destruct(&data->mutex); mutex_destruct(&data->mutex);
delete data; delete data;
parent->internal.data = nullptr; parent->module_parent_private = nullptr;
return ERROR_NONE; 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; if (module->internal.parent == parent) return ERROR_NONE;
// Remove from old parent // Remove from old parent
if (module->internal.parent && module->internal.parent->internal.data) { if (module->internal.parent && module->internal.parent->module_parent_private) {
auto* old_data = static_cast<ModuleParentInternal*>(module->internal.parent->internal.data); auto* old_data = static_cast<ModuleParentPrivate*>(module->internal.parent->module_parent_private);
mutex_lock(&old_data->mutex); mutex_lock(&old_data->mutex);
auto it = std::find(old_data->modules.begin(), old_data->modules.end(), module); auto it = std::find(old_data->modules.begin(), old_data->modules.end(), module);
if (it != old_data->modules.end()) { 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; module->internal.parent = parent;
// Add to new parent // Add to new parent
if (parent && parent->internal.data) { if (parent && parent->module_parent_private) {
auto* new_data = static_cast<ModuleParentInternal*>(parent->internal.data); auto* new_data = static_cast<ModuleParentPrivate*>(parent->module_parent_private);
mutex_lock(&new_data->mutex); mutex_lock(&new_data->mutex);
new_data->modules.push_back(module); new_data->modules.push_back(module);
mutex_unlock(&new_data->mutex); mutex_unlock(&new_data->mutex);

View File

@ -18,16 +18,16 @@ TEST_CASE("device_construct and device_destruct should set and unset the correct
error_t error = device_construct(&device); error_t error = device_construct(&device);
CHECK_EQ(error, ERROR_NONE); 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_NE(device.internal.mutex.handle, nullptr);
CHECK_EQ(device_destruct(&device), ERROR_NONE); 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); CHECK_EQ(device.internal.mutex.handle, nullptr);
Device comparison_device = { 0 }; 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; comparison_device.internal.mutex.handle = device.internal.mutex.handle;
// Check that no other data was set // 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, .api = nullptr,
.deviceType = nullptr, .deviceType = nullptr,
.owner = &module, .owner = &module,
.internal = nullptr .driver_private = nullptr
}; };
Device device = { 0 }; Device device = { 0 };

View File

@ -38,7 +38,7 @@ static Driver integration_driver = {
.api = nullptr, .api = nullptr,
.deviceType = nullptr, .deviceType = nullptr,
.owner = &module, .owner = &module,
.internal = nullptr, .driver_private = nullptr,
}; };
TEST_CASE("driver with with start success and stop success should start and stop a device") { TEST_CASE("driver with with start success and stop success should start and stop a device") {

View File

@ -14,9 +14,9 @@ TEST_CASE("driver_construct and driver_destruct should set and unset the correct
driver.owner = &module; driver.owner = &module;
CHECK_EQ(driver_construct(&driver), ERROR_NONE); 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_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") { 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, .api = nullptr,
.deviceType = nullptr, .deviceType = nullptr,
.owner = &module, .owner = &module,
.internal = nullptr .driver_private = nullptr
}; };
CHECK_EQ(driver_is_compatible(&driver, "test_compatible"), true); CHECK_EQ(driver_is_compatible(&driver, "test_compatible"), true);
CHECK_EQ(driver_is_compatible(&driver, "nope"), false); 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, .api = nullptr,
.deviceType = nullptr, .deviceType = nullptr,
.owner = &module, .owner = &module,
.internal = nullptr .driver_private = nullptr
}; };
Driver* found_driver = driver_find_compatible("test_compatible"); Driver* found_driver = driver_find_compatible("test_compatible");

View File

@ -16,19 +16,19 @@ static error_t test_stop() {
} }
TEST_CASE("ModuleParent construction and destruction") { TEST_CASE("ModuleParent construction and destruction") {
struct ModuleParent parent = { "test_parent", { nullptr } }; struct ModuleParent parent = { "test_parent", nullptr };
// Test successful construction // Test successful construction
CHECK_EQ(module_parent_construct(&parent), ERROR_NONE); CHECK_EQ(module_parent_construct(&parent), ERROR_NONE);
CHECK_NE(parent.internal.data, nullptr); CHECK_NE(parent.module_parent_private, nullptr);
// Test successful destruction // Test successful destruction
CHECK_EQ(module_parent_destruct(&parent), ERROR_NONE); 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") { TEST_CASE("ModuleParent destruction with children") {
struct ModuleParent parent = { "parent", { nullptr } }; struct ModuleParent parent = { "parent", nullptr };
REQUIRE_EQ(module_parent_construct(&parent), ERROR_NONE); REQUIRE_EQ(module_parent_construct(&parent), ERROR_NONE);
struct Module module = { struct Module module = {
@ -42,19 +42,19 @@ TEST_CASE("ModuleParent destruction with children") {
// Should fail to destruct because it has a child // Should fail to destruct because it has a child
CHECK_EQ(module_parent_destruct(&parent), ERROR_INVALID_STATE); CHECK_EQ(module_parent_destruct(&parent), ERROR_INVALID_STATE);
CHECK_NE(parent.internal.data, nullptr); CHECK_NE(parent.module_parent_private, nullptr);
// Remove child // Remove child
REQUIRE_EQ(module_set_parent(&module, nullptr), ERROR_NONE); REQUIRE_EQ(module_set_parent(&module, nullptr), ERROR_NONE);
// Now it should succeed // Now it should succeed
CHECK_EQ(module_parent_destruct(&parent), ERROR_NONE); 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") { TEST_CASE("Module parent management") {
struct ModuleParent parent1 = { "parent1", { nullptr } }; struct ModuleParent parent1 = { "parent1", nullptr };
struct ModuleParent parent2 = { "parent2", { nullptr } }; struct ModuleParent parent2 = { "parent2", nullptr };
REQUIRE_EQ(module_parent_construct(&parent1), ERROR_NONE); REQUIRE_EQ(module_parent_construct(&parent1), ERROR_NONE);
REQUIRE_EQ(module_parent_construct(&parent2), ERROR_NONE); REQUIRE_EQ(module_parent_construct(&parent2), ERROR_NONE);
@ -88,7 +88,7 @@ TEST_CASE("Module parent management") {
} }
TEST_CASE("Module lifecycle") { TEST_CASE("Module lifecycle") {
struct ModuleParent parent = { "parent", { nullptr } }; struct ModuleParent parent = { "parent", nullptr };
REQUIRE_EQ(module_parent_construct(&parent), ERROR_NONE); REQUIRE_EQ(module_parent_construct(&parent), ERROR_NONE);
start_called = false; start_called = false;