Added unit tests

Buildscripts/devicetree-compiler/source/generator.py

@@ -123,7 +123,7 @@ def write_device_structs(file, device: Device, parent_device: Device, bindings:
     # Write device struct
     file.write(f"static struct Device {identifier}" " = {\n")
     file.write(f"\t.name = \"{device.identifier}\",\n") # Use original name
-    file.write(f"\t.config = (void*)&{config_variable_name},\n")
+    file.write(f"\t.config = &{config_variable_name},\n")
     file.write(f"\t.parent = {parent_value},\n")
     file.write("};\n\n")
     # Child devices

TactilityKernel/Include/Tactility/Device.h

@@ -25,7 +25,7 @@ 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 */
-    void* config;
+    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 */
@@ -51,8 +51,9 @@ struct Device {
  * Initialize the properties of a device.
  *
  * @param[in] dev a device with all non-internal properties set
+ * @return the result code (0 for success)
  */
-void device_construct(struct Device* device);
+int device_construct(struct Device* device);
 
 /**
  * Deinitialize the properties of a device.
@@ -80,8 +81,9 @@ static inline bool device_is_ready(const struct Device* device) {
  * - a bus (if any)
  *
  * @param[in] device non-null device pointer
+ * @return 0 on success
  */
-void device_add(struct Device* device);
+int device_add(struct Device* device);
 
 /**
  * Deregister a device. Remove it from all relevant systems:
@@ -90,9 +92,9 @@ void device_add(struct Device* device);
  * - a bus (if any)
  *
  * @param[in] device non-null device pointer
- * @return true when the device was found and deregistered
+ * @return 0 on success
  */
-bool device_remove(struct Device* device);
+int device_remove(struct Device* device);
 
 /**
  * Attach the driver.

TactilityKernel/Include/Tactility/Driver.h

@@ -11,7 +11,7 @@ struct Driver {
     /** The driver name */
     const char* name;
     /** Array of const char*, terminated by NULL */
-    const char** compatible;
+    const char**compatible;
     /** Function to initialize the driver for a device */
     int (*start_device)(struct Device* dev);
     /** Function to deinitialize the driver for a device */
@@ -31,12 +31,14 @@ int driver_construct(struct Driver* driver);
 
 int driver_destruct(struct Driver* driver);
 
-struct Driver* driver_find(const char* compatible);
-
 int driver_bind(struct Driver* driver, struct Device* device);
 
 int driver_unbind(struct Driver* driver, struct Device* device);
 
+bool driver_is_compatible(struct Driver* driver, const char* compatible);
+
+struct Driver* driver_find_compatible(const char* compatible);
+
 static inline const struct DeviceType* driver_get_device_type(struct Driver* driver) {
     return driver->device_type;
 }

TactilityKernel/Include/Tactility/Error.h

@@ -2,9 +2,13 @@
 
 #include <errno.h>
 
-#define CUSTOM_ERROR_CODE(x) (-2000 - x)
+#define CUSTOM_ERROR_CODE(x) (2000 + x)
+
+// TODO: Make more aliases for common errors
 
 #define ERROR_UNDEFINED CUSTOM_ERROR_CODE(1)
 #define ERROR_INVALID_STATE CUSTOM_ERROR_CODE(2)
-#define ERROR_INVALID_PARAMETER CUSTOM_ERROR_CODE(3)
-#define ERROR_MISSING_PARAMETER CUSTOM_ERROR_CODE(4)
+#define ERROR_INVALID_ARGUMENT EINVAL
+#define ERROR_MISSING_PARAMETER CUSTOM_ERROR_CODE(3)
+#define ERROR_NOT_FOUND CUSTOM_ERROR_CODE(4)
+

TactilityKernel/Source/Device.cpp

@@ -28,7 +28,12 @@ struct DeviceLedger {
     }
 };
 
-static DeviceLedger ledger;
+static DeviceLedger& get_ledger() {
+    static DeviceLedger ledger;
+    return ledger;
+}
+
+#define ledger get_ledger()
 
 extern "C" {
 
@@ -37,10 +42,14 @@ extern "C" {
 
 #define get_device_data(device) static_cast<DeviceData*>(device->internal.data)
 
-void device_construct(Device* device) {
+int device_construct(Device* device) {
     LOG_I(TAG, "construct %s", device->name);
-    device->internal.data = new DeviceData();
+    device->internal.data = new(std::nothrow) DeviceData;
+    if (device->internal.data == nullptr) {
+        return ENOMEM;
+    }
     mutex_construct(&device->internal.mutex);
+    return 0;
 }
 
 int device_destruct(Device* device) {
@@ -63,21 +72,19 @@ static void device_add_child(struct Device* device, struct Device* child) {
 static void device_remove_child(struct Device* device, struct Device* child) {
     device_lock(device);
     auto* parent_data = get_device_data(device);
-    const auto iterator = std::ranges::find(parent_data->children, device);
+    const auto iterator = std::ranges::find(parent_data->children, child);
     if (iterator != parent_data->children.end()) {
         parent_data->children.erase(iterator);
     }
     device_unlock(device);
 }
 
-void device_add(Device* device) {
+int device_add(Device* device) {
     LOG_I(TAG, "add %s", device->name);
 
-    assert(!device->internal.state.started);
-
     // Already added
-    if (device->internal.state.added) {
-        return;
+    if (device->internal.state.started || device->internal.state.added) {
+        return ERROR_INVALID_STATE;
     }
 
     // Add to ledger
@@ -92,16 +99,14 @@ void device_add(Device* device) {
     }
 
     device->internal.state.added = true;
+    return 0;
 }
 
-bool device_remove(Device* device) {
+int device_remove(Device* device) {
     LOG_I(TAG, "remove %s", device->name);
 
-    assert(!device->internal.state.started);
-
-    // Already removed
-    if (!device->internal.state.added) {
-        return true;
+    if (device->internal.state.started || !device->internal.state.added) {
+        return ERROR_INVALID_STATE;
     }
 
     // Remove self from parent's children list
@@ -120,7 +125,7 @@ bool device_remove(Device* device) {
     ledger_unlock();
 
     device->internal.state.added = false;
-    return true;
+    return 0;
 
 failed_ledger_lookup:
 
@@ -129,7 +134,7 @@ failed_ledger_lookup:
         device_add_child(parent, device);
     }
 
-    return false;
+    return ERROR_NOT_FOUND;
 }
 
 int device_start(Device* device) {
@@ -137,23 +142,19 @@ int device_start(Device* device) {
         return ERROR_INVALID_STATE;
     }
 
+    if (device->internal.driver == nullptr) {
+        return ERROR_INVALID_STATE;
+    }
+
     // Already started
     if (device->internal.state.started) {
         return 0;
     }
 
-    if (device->internal.driver == nullptr) {
-        LOG_E(TAG, "start error: no driver for %s", device->name);
-        return ERROR_INVALID_STATE;
-    }
-
     int result = driver_bind(device->internal.driver, device);
-    if (result != 0) {
-        device->internal.state.started = true;
-        device->internal.state.start_result = result;
-    }
-
-    return 0;
+    device->internal.state.started = (result == 0);
+    device->internal.state.start_result = result;
+    return result;
 }
 
 int device_stop(struct Device* device) {
@@ -161,11 +162,16 @@ int device_stop(struct Device* device) {
         return ERROR_INVALID_STATE;
     }
 
-    // Already stopped
+    // Not started
     if (!device->internal.state.started) {
         return 0;
     }
 
+    int result = driver_unbind(device->internal.driver, device);
+    if (result != 0) {
+        return result;
+    }
+
     device->internal.state.started = false;
     device->internal.state.start_result = 0;
     return 0;

TactilityKernel/Source/Driver.cpp

@@ -24,7 +24,7 @@ struct DriverInternalData {
 };
 
 struct DriverLedger {
-    std::vector<Driver*> drivers = {};
+    std::vector<Driver*> drivers;
     Mutex mutex { 0 };
 
     DriverLedger() {
@@ -34,35 +34,45 @@ struct DriverLedger {
     ~DriverLedger() {
         mutex_destruct(&mutex);
     }
+
+    void lock() {
+        mutex_lock(&mutex);
+    }
+
+    void unlock() {
+        mutex_unlock(&mutex);
+    }
 };
 
-static DriverLedger ledger;
+static DriverLedger& get_ledger() {
+    static DriverLedger ledger;
+    return ledger;
+}
 
-#define ledger_lock() mutex_lock(&ledger.mutex);
-#define ledger_unlock() mutex_unlock(&ledger.mutex);
+#define ledger get_ledger()
 
 #define driver_internal_data(driver) static_cast<DriverInternalData*>(driver->internal.data)
 #define driver_lock(driver) mutex_lock(&driver_internal_data(driver)->mutex);
 #define driver_unlock(driver) mutex_unlock(&driver_internal_data(driver)->mutex);
 
-static void driver_add(Driver* dev) {
-    LOG_I(TAG, "add %s", dev->name);
-    ledger_lock();
-    ledger.drivers.push_back(dev);
-    ledger_unlock();
+static void driver_add(Driver* driver) {
+    LOG_I(TAG, "add %s", driver->name);
+    ledger.lock();
+    ledger.drivers.push_back(driver);
+    ledger.unlock();
 }
 
-static bool driver_remove(Driver* dev) {
-    LOG_I(TAG, "remove %s", dev->name);
+static bool driver_remove(Driver* driver) {
+    LOG_I(TAG, "remove %s", driver->name);
 
-    ledger_lock();
-    const auto iterator = std::ranges::find(ledger.drivers, dev);
+    ledger.lock();
+    const auto iterator = std::ranges::find(ledger.drivers, driver);
     // check that there actually is a 3 in our vector
     if (iterator == ledger.drivers.end()) {
         return false;
     }
     ledger.drivers.erase(iterator);
-    ledger_unlock();
+    ledger.unlock();
 
     return true;
 }
@@ -70,38 +80,51 @@ static bool driver_remove(Driver* dev) {
 extern "C" {
 
 int driver_construct(Driver* driver) {
-    driver->internal.data = new DriverInternalData();
+    driver->internal.data = new(std::nothrow) DriverInternalData;
+    if (driver->internal.data == nullptr) {
+        return ENOMEM;
+    }
     driver_add(driver);
     return 0;
 }
 
 int driver_destruct(Driver* driver) {
     // Check if in use
-    if (driver_internal_data(driver)->use_count == 0) {
+    if (driver_internal_data(driver)->use_count != 0) {
         return ERROR_INVALID_STATE;
     }
 
     driver_remove(driver);
     delete driver_internal_data(driver);
+    driver->internal.data = nullptr;
     return 0;
 }
 
-Driver* driver_find(const char* compatible) {
-    ledger_lock();
-    const auto it = std::ranges::find_if(ledger.drivers, [compatible](Driver* driver) {
-        const char** current_compatible = driver->compatible;
-        assert(current_compatible != nullptr);
-        while (*current_compatible != nullptr) {
-            if (strcmp(compatible, *current_compatible) == 0) {
-                return true;
-            }
-            current_compatible++;
-        }
+bool driver_is_compatible(Driver* driver, const char* compatible) {
+    if (compatible == nullptr || driver->compatible == nullptr) {
         return false;
-    });
-    auto* driver = (it != ledger.drivers.end()) ? *it : nullptr;
-    ledger_unlock();
-    return driver;
+    }
+    const char** compatible_iterator = driver->compatible;
+    while (*compatible_iterator != nullptr) {
+        if (strcmp(*compatible_iterator, compatible) == 0) {
+            return true;
+        }
+        compatible_iterator++;
+    }
+    return false;
+}
+
+Driver* driver_find_compatible(const char* compatible) {
+    ledger.lock();
+    Driver* result = nullptr;
+    for (auto* driver : ledger.drivers) {
+        if (driver_is_compatible(driver, compatible)) {
+            result = driver;
+            break;
+        }
+    }
+    ledger.unlock();
+    return result;
 }
 
 int driver_bind(Driver* driver, Device* device) {
@@ -109,12 +132,10 @@ int driver_bind(Driver* driver, Device* device) {
 
     int err = 0;
     if (!device_is_added(device)) {
-        err = -ENODEV;
+        err = ERROR_INVALID_STATE;
         goto error;
     }
 
-    device_set_driver(device, driver);
-
     if (driver->start_device != nullptr) {
         err = driver->start_device(device);
         if (err != 0) {
@@ -137,16 +158,19 @@ error:
 int driver_unbind(Driver* driver, Device* device) {
     driver_lock(driver);
 
-    if (driver->stop_device == nullptr) {
-        return 0;
-    }
-
-    int err = driver->stop_device(device);
-    if (err != 0) {
+    int err = 0;
+    if (!device_is_added(device)) {
+        err = ERROR_INVALID_STATE;
         goto error;
     }
 
-    device_set_driver(device, nullptr);
+    if (driver->stop_device != nullptr) {
+        err = driver->stop_device(device);
+        if (err != 0) {
+            goto error;
+        }
+    }
+
     driver_internal_data(driver)->use_count--;
     driver_unlock(driver);
 

Tests/TactilityKernel/DeviceTest.cpp

@@ -5,15 +5,19 @@
 
 #include <Tactility/Device.h>
 
+#include "Tactility/Log.h"
+
 TEST_CASE("device_construct and device_destruct should set and unset the correct fields") {
     Device device = { 0 };
 
-    device_construct(&device);
+    int error = device_construct(&device);
+    CHECK_EQ(error, 0);
 
     CHECK_NE(device.internal.data, nullptr);
     CHECK_NE(device.internal.mutex.handle, nullptr);
 
-    device_destruct(&device);
+    error = device_destruct(&device);
+    CHECK_EQ(error, 0);
 
     CHECK_EQ(device.internal.data, nullptr);
     CHECK_EQ(device.internal.mutex.handle, nullptr);
@@ -23,13 +27,13 @@ TEST_CASE("device_construct and device_destruct should set and unset the correct
     comparison_device.internal.mutex.handle = device.internal.mutex.handle;
 
     // Check that no other data was set
-    CHECK_EQ(memcmp(&device, &comparison_device, sizeof(struct Device)), 0);
+    CHECK_EQ(memcmp(&device, &comparison_device, sizeof(Device)), 0);
 }
 
-TEST_CASE("device_add should make the device discoverable") {
+TEST_CASE("device_add should add the device to the list of all devices") {
     Device device = { 0 };
-    device_construct(&device);
-    device_add(&device);
+    CHECK_EQ(device_construct(&device), 0);
+    CHECK_EQ(device_add(&device), 0);
 
     // Gather all devices
     std::vector<Device*> devices;
@@ -42,8 +46,8 @@ TEST_CASE("device_add should make the device discoverable") {
     CHECK_EQ(devices.size(), 1);
     CHECK_EQ(devices[0], &device);
 
-    device_remove(&device);
-    device_destruct(&device);
+    CHECK_EQ(device_remove(&device), 0);
+    CHECK_EQ(device_destruct(&device), 0);
 }
 
 TEST_CASE("device_add should add the device to its parent") {
@@ -55,11 +59,11 @@ TEST_CASE("device_add should add the device to its parent") {
         .parent = &parent
     };
 
-    device_construct(&parent);
-    device_add(&parent);
+    CHECK_EQ(device_construct(&parent), 0);
+    CHECK_EQ(device_add(&parent), 0);
 
-    device_construct(&child);
-    device_add(&child);
+    CHECK_EQ(device_construct(&child), 0);
+    CHECK_EQ(device_add(&child), 0);
 
     // Gather all child devices
     std::vector<Device*> children;
@@ -72,21 +76,117 @@ TEST_CASE("device_add should add the device to its parent") {
     CHECK_EQ(children.size(), 1);
     CHECK_EQ(children[0], &child);
 
-    device_remove(&child);
-    device_destruct(&child);
+    CHECK_EQ(device_remove(&child), 0);
+    CHECK_EQ(device_destruct(&child), 0);
 
-    device_remove(&parent);
-    device_destruct(&parent);
+    CHECK_EQ(device_remove(&parent), 0);
+    CHECK_EQ(device_destruct(&parent), 0);
 }
 
 TEST_CASE("device_add should set the state to 'added'") {
     Device device = { 0 };
-    device_construct(&device);
+    CHECK_EQ(device_construct(&device), 0);
 
     CHECK_EQ(device.internal.state.added, false);
-    device_add(&device);
+    CHECK_EQ(device_add(&device), 0);
     CHECK_EQ(device.internal.state.added, true);
 
+    CHECK_EQ(device_remove(&device), 0);
+    CHECK_EQ(device_destruct(&device), 0);
+}
+
+TEST_CASE("device_remove should remove it from the list of all devices") {
+    Device device = { 0 };
+    CHECK_EQ(device_construct(&device), 0);
+    CHECK_EQ(device_add(&device), 0);
+    CHECK_EQ(device_remove(&device), 0);
+
+    // Gather all devices
+    std::vector<Device*> devices;
+    for_each_device(&devices, [](auto* device, auto* context) {
+        auto* devices_ptr = (std::vector<Device*>*)context;
+        devices_ptr->push_back(device);
+        return true;
+    });
+
+    CHECK_EQ(devices.size(), 0);
+
+    CHECK_EQ(device_destruct(&device), 0);
+}
+
+TEST_CASE("device_remove should remove the device from its parent") {
+    Device parent = { 0 };
+
+    Device child = {
+        .name = nullptr,
+        .config = nullptr,
+        .parent = &parent
+    };
+
+    CHECK_EQ(device_construct(&parent), 0);
+    CHECK_EQ(device_add(&parent), 0);
+
+    CHECK_EQ(device_construct(&child), 0);
+    CHECK_EQ(device_add(&child), 0);
+    CHECK_EQ(device_remove(&child), 0);
+
+    // Gather all child devices
+    std::vector<Device*> children;
+    for_each_device_child(&parent, &children, [](auto* child_device, auto* context) {
+        auto* children_ptr = (std::vector<Device*>*)context;
+        children_ptr->push_back(child_device);
+        return true;
+    });
+
+    CHECK_EQ(children.size(), 0);
+
+    CHECK_EQ(device_destruct(&child), 0);
+
+    CHECK_EQ(device_remove(&parent), 0);
+    CHECK_EQ(device_destruct(&parent), 0);
+}
+
+TEST_CASE("device_remove should clear the state 'added'") {
+    Device device = { 0 };
+    device_construct(&device);
+
+    device_add(&device);
+    CHECK_EQ(device.internal.state.added, true);
     device_remove(&device);
+    CHECK_EQ(device.internal.state.added, false);
+
     device_destruct(&device);
 }
+
+TEST_CASE("device_is_ready should return true only when it is started") {
+    static Driver driver = {
+        .name = "test_driver",
+        .compatible = (const char*[]) { "test_compatible", nullptr },
+        .start_device = nullptr,
+        .stop_device = nullptr,
+        .api = nullptr,
+        .device_type = nullptr,
+        .internal = { 0 }
+    };
+
+    Device device = { 0 };
+
+    CHECK_EQ(driver_construct(&driver), 0);
+    CHECK_EQ(device_construct(&device), 0);
+
+    CHECK_EQ(device.internal.state.started, false);
+    device_set_driver(&device, &driver);
+    CHECK_EQ(device.internal.state.started, false);
+    CHECK_EQ(device_add(&device), 0);
+    CHECK_EQ(device.internal.state.started, false);
+    int result = device_start(&device);
+    CHECK_EQ(result, 0);
+    CHECK_EQ(device.internal.state.started, true);
+    CHECK_EQ(device_stop(&device), 0);
+    CHECK_EQ(device.internal.state.started, false);
+    CHECK_EQ(device_remove(&device), 0);
+    CHECK_EQ(device.internal.state.started, false);
+
+    CHECK_EQ(driver_destruct(&driver), 0);
+    CHECK_EQ(device_destruct(&device), 0);
+}

Tests/TactilityKernel/DriverIntegrationTest.cpp

@@ -0,0 +1,64 @@
+#include "doctest.h"
+#include <Tactility/Device.h>
+#include <Tactility/Driver.h>
+
+struct IntegrationDriverConfig {
+    int startResult;
+    int stopResult;
+};
+
+static int startCalled = 0;
+static int stopCalled = 0;
+
+#define integration_data(device) static_cast<IntegrationDriverData*>(device_get_driver_data(device))
+#define integration_config(device) static_cast<const IntegrationDriverConfig*>(device->config)
+
+static int start(Device* device) {
+    startCalled++;
+    return integration_config(device)->startResult;
+}
+
+static int stop(Device* device) {
+    stopCalled++;
+    return integration_config(device)->stopResult;
+}
+
+static Driver integration_driver = {
+    .name = "integration_test_driver",
+    .compatible = (const char*[]) { "integration", nullptr },
+    .start_device = start,
+    .stop_device = stop,
+    .api = nullptr,
+    .device_type = nullptr,
+    .internal = { 0 }
+};
+
+TEST_CASE("driver with with start success and stop success should start and stop a device") {
+    startCalled = 0;
+    stopCalled = 0;
+    static const IntegrationDriverConfig config {
+        .startResult = 0,
+        .stopResult = 0
+    };
+
+    static Device integration_device {
+        .name = "integration_device",
+        .config = &config,
+        .parent = nullptr,
+    };
+
+    CHECK_EQ(driver_construct(&integration_driver), 0);
+
+    CHECK_EQ(device_construct(&integration_device), 0);
+    device_add(&integration_device);
+    CHECK_EQ(startCalled, 0);
+    CHECK_EQ(driver_bind(&integration_driver, &integration_device), 0);
+    CHECK_EQ(startCalled, 1);
+    CHECK_EQ(stopCalled, 0);
+    CHECK_EQ(driver_unbind(&integration_driver, &integration_device), 0);
+    CHECK_EQ(stopCalled, 1);
+    CHECK_EQ(device_remove(&integration_device), 0);
+    CHECK_EQ(device_destruct(&integration_device), 0);
+
+    CHECK_EQ(driver_destruct(&integration_driver), 0);
+}

Tests/TactilityKernel/DriverTest.cpp

@@ -1,6 +1,71 @@
 #include "doctest.h"
+#include <cstring>
 #include <Tactility/Driver.h>
 
-TEST_CASE("placeholder driver test") {
-    // TODO: Implement
+TEST_CASE("driver_construct and driver_destruct should set and unset the correct fields") {
+    Driver driver = { 0 };
+
+    int error = driver_construct(&driver);
+    CHECK_EQ(error, 0);
+    CHECK_NE(driver.internal.data, nullptr);
+
+    error = driver_destruct(&driver);
+    CHECK_EQ(error, 0);
+    CHECK_EQ(driver.internal.data, nullptr);
+}
+
+TEST_CASE("driver_is_comptable should return true if a compatible value is found") {
+    Driver driver = {
+        .name = "test_driver",
+        .compatible = (const char*[]) { "test_compatible", nullptr },
+        .start_device = nullptr,
+        .stop_device = nullptr,
+        .api = nullptr,
+        .device_type = nullptr,
+        .internal = { 0 }
+    };
+    CHECK_EQ(driver_is_compatible(&driver, "test_compatible"), true);
+    CHECK_EQ(driver_is_compatible(&driver, "nope"), false);
+    CHECK_EQ(driver_is_compatible(&driver, nullptr), false);
+}
+
+TEST_CASE("driver_is_comptable should return true if a compatible value is found") {
+    Driver driver = {
+        .name = "test_driver",
+        .compatible = nullptr,
+        .start_device = nullptr,
+        .stop_device = nullptr,
+        .api = nullptr,
+        .device_type = nullptr,
+        .internal = { 0 }
+    };
+    CHECK_EQ(driver_is_compatible(&driver, nullptr), false);
+}
+
+TEST_CASE("driver_find should only find a compatible driver when the driver was constructed") {
+    // Must be static or outside of function to prevent SIGSEV crash due to memory corruption while iterating .compatible
+    static Driver driver = {
+        .name = "test_driver",
+        .compatible = (const char*[]) { "test_compatible", nullptr },
+        .start_device = nullptr,
+        .stop_device = nullptr,
+        .api = nullptr,
+        .device_type = nullptr,
+        .internal = { 0 }
+    };
+
+    Driver* found_driver = driver_find_compatible("test_compatible");
+    CHECK_EQ(found_driver, nullptr);
+
+    int error = driver_construct(&driver);
+    CHECK_EQ(error, 0);
+
+    found_driver = driver_find_compatible("test_compatible");
+    CHECK_EQ(found_driver, &driver);
+
+    error = driver_destruct(&driver);
+    CHECK_EQ(error, 0);
+
+    found_driver = driver_find_compatible("test_compatible");
+    CHECK_EQ(found_driver, nullptr);
 }