TactilityKernel improvements and more (#459)

TactilityKernel/Source/Device.cpp

@@ -44,17 +44,17 @@ extern "C" {
 
 #define get_device_data(device) static_cast<DeviceData*>(device->internal.data)
 
-int device_construct(Device* device) {
+error_t device_construct(Device* device) {
     device->internal.data = new(std::nothrow) DeviceData;
     if (device->internal.data == nullptr) {
-        return ENOMEM;
+        return ERROR_OUT_OF_MEMORY;
     }
     LOG_I(TAG, "construct %s", device->name);
     mutex_construct(&device->internal.mutex);
-    return 0;
+    return ERROR_NONE;
 }
 
-int device_destruct(Device* device) {
+error_t device_destruct(Device* device) {
     if (device->internal.state.started || device->internal.state.added) {
         return ERROR_INVALID_STATE;
     }
@@ -65,7 +65,7 @@ int device_destruct(Device* device) {
     mutex_destruct(&device->internal.mutex);
     delete get_device_data(device);
     device->internal.data = nullptr;
-    return 0;
+    return ERROR_NONE;
 }
 
 /** Add a child to the list of children */
@@ -87,7 +87,7 @@ static void device_remove_child(struct Device* device, struct Device* child) {
     device_unlock(device);
 }
 
-int device_add(Device* device) {
+error_t device_add(Device* device) {
     LOG_I(TAG, "add %s", device->name);
 
     // Already added
@@ -107,10 +107,10 @@ int device_add(Device* device) {
     }
 
     device->internal.state.added = true;
-    return 0;
+    return ERROR_NONE;
 }
 
-int device_remove(Device* device) {
+error_t device_remove(Device* device) {
     LOG_I(TAG, "remove %s", device->name);
 
     if (device->internal.state.started || !device->internal.state.added) {
@@ -133,7 +133,7 @@ int device_remove(Device* device) {
     ledger_unlock();
 
     device->internal.state.added = false;
-    return 0;
+    return ERROR_NONE;
 
 failed_ledger_lookup:
 
@@ -145,7 +145,7 @@ failed_ledger_lookup:
     return ERROR_NOT_FOUND;
 }
 
-int device_start(Device* device) {
+error_t device_start(Device* device) {
     if (!device->internal.state.added) {
         return ERROR_INVALID_STATE;
     }
@@ -156,33 +156,31 @@ int device_start(Device* device) {
 
     // Already started
     if (device->internal.state.started) {
-        return 0;
+        return ERROR_NONE;
     }
 
-    int result = driver_bind(device->internal.driver, device);
-    device->internal.state.started = (result == 0);
-    device->internal.state.start_result = result;
-    return result;
+    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;
 }
 
-int device_stop(struct Device* device) {
+error_t device_stop(struct Device* device) {
     if (!device->internal.state.added) {
         return ERROR_INVALID_STATE;
     }
 
-    // Not started
     if (!device->internal.state.started) {
-        return 0;
+        return ERROR_NONE;
     }
 
-    int result = driver_unbind(device->internal.driver, device);
-    if (result != 0) {
-        return result;
+    if (driver_unbind(device->internal.driver, device) != ERROR_NONE) {
+        return ERROR_RESOURCE;
     }
 
     device->internal.state.started = false;
     device->internal.state.start_result = 0;
-    return 0;
+    return ERROR_NONE;
 }
 
 void device_set_parent(Device* device, Device* parent) {
@@ -200,22 +198,22 @@ void for_each_device(void* callback_context, bool(*on_device)(Device* device, vo
     ledger_unlock();
 }
 
-void for_each_device_child(Device* device, void* callback_context, 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);
     for (auto* child_device : data->children) {
-        if (!on_device(child_device, callback_context)) {
+        if (!on_device(child_device, callbackContext)) {
             break;
         }
     }
 }
 
-void for_each_device_of_type(const DeviceType* type, void* callback_context, bool(*on_device)(Device* device, void* context)) {
+void for_each_device_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;
         if (driver != nullptr) {
-            if (driver->device_type == type) {
-                if (!on_device(device, callback_context)) {
+            if (driver->deviceType == type) {
+                if (!on_device(device, callbackContext)) {
                     break;
                 }
             }

TactilityKernel/Source/Driver.cpp

@@ -15,6 +15,7 @@
 struct DriverInternalData {
     Mutex mutex { 0 };
     int use_count = 0;
+    bool destroying = false;
 
     DriverInternalData() {
         mutex_construct(&mutex);
@@ -64,7 +65,7 @@ static void driver_add(Driver* driver) {
     ledger.unlock();
 }
 
-static bool driver_remove(Driver* driver) {
+static error_t driver_remove(Driver* driver) {
     LOG_I(TAG, "remove %s", driver->name);
 
     ledger.lock();
@@ -72,35 +73,41 @@ static bool driver_remove(Driver* driver) {
     // check that there actually is a 3 in our vector
     if (iterator == ledger.drivers.end()) {
         ledger.unlock();
-        return false;
+        return ERROR_NOT_FOUND;
     }
     ledger.drivers.erase(iterator);
     ledger.unlock();
 
-    return true;
+    return ERROR_NONE;
 }
 
 extern "C" {
 
-int driver_construct(Driver* driver) {
+error_t driver_construct(Driver* driver) {
     driver->internal.data = new(std::nothrow) DriverInternalData;
     if (driver->internal.data == nullptr) {
-        return ENOMEM;
+        return ERROR_OUT_OF_MEMORY;
     }
     driver_add(driver);
-    return 0;
+    return ERROR_NONE;
 }
 
-int driver_destruct(Driver* driver) {
-    // Check if in use
-    if (driver_internal_data(driver)->use_count != 0) {
+error_t driver_destruct(Driver* driver) {
+    driver_lock(driver);
+    if (driver_internal_data(driver)->use_count != 0 || driver_internal_data(driver)->destroying) {
+        driver_unlock(driver);
         return ERROR_INVALID_STATE;
     }
+    driver_internal_data(driver)->destroying = true;
+    driver_unlock(driver);
+
+    if (driver_remove(driver) != ERROR_NONE) {
+        LOG_W(TAG, "Failed to remove driver from ledger: %s", driver->name);
+    }
 
-    driver_remove(driver);
     delete driver_internal_data(driver);
     driver->internal.data = nullptr;
-    return 0;
+    return ERROR_NONE;
 }
 
 bool driver_is_compatible(Driver* driver, const char* compatible) {
@@ -130,18 +137,18 @@ Driver* driver_find_compatible(const char* compatible) {
     return result;
 }
 
-int driver_bind(Driver* driver, Device* device) {
+error_t driver_bind(Driver* driver, Device* device) {
     driver_lock(driver);
 
-    int err = 0;
-    if (!device_is_added(device)) {
-        err = ERROR_INVALID_STATE;
+    error_t error = ERROR_NONE;
+    if (driver_internal_data(driver)->destroying || !device_is_added(device)) {
+        error = ERROR_INVALID_STATE;
         goto error;
     }
 
-    if (driver->start_device != nullptr) {
-        err = driver->start_device(device);
-        if (err != 0) {
+    if (driver->startDevice != nullptr) {
+        error = driver->startDevice(device);
+        if (error != ERROR_NONE) {
             goto error;
         }
     }
@@ -150,26 +157,26 @@ int driver_bind(Driver* driver, Device* device) {
     driver_unlock(driver);
 
     LOG_I(TAG, "bound %s to %s", driver->name, device->name);
-    return 0;
+    return ERROR_NONE;
 
 error:
 
     driver_unlock(driver);
-    return err;
+    return error;
 }
 
-int driver_unbind(Driver* driver, Device* device) {
+error_t driver_unbind(Driver* driver, Device* device) {
     driver_lock(driver);
 
-    int err = 0;
-    if (!device_is_added(device)) {
-        err = ERROR_INVALID_STATE;
+    error_t error = ERROR_NONE;
+    if (driver_internal_data(driver)->destroying || !device_is_added(device)) {
+        error = ERROR_INVALID_STATE;
         goto error;
     }
 
-    if (driver->stop_device != nullptr) {
-        err = driver->stop_device(device);
-        if (err != 0) {
+    if (driver->stopDevice != nullptr) {
+        error = driver->stopDevice(device);
+        if (error != ERROR_NONE) {
             goto error;
         }
     }
@@ -179,12 +186,12 @@ int driver_unbind(Driver* driver, Device* device) {
 
     LOG_I(TAG, "unbound %s to %s", driver->name, device->name);
 
-    return 0;
+    return ERROR_NONE;
 
 error:
 
     driver_unlock(driver);
-    return err;
+    return error;
 }
 
 } // extern "C"

TactilityKernel/Source/concurrent/Dispatcher.cpp

@@ -0,0 +1,142 @@
+// SPDX-License-Identifier: Apache-2.0
+
+#include <queue>
+
+#include <Tactility/concurrent/Dispatcher.h>
+
+#include "Tactility/Error.h"
+
+#include <Tactility/Log.h>
+#include <Tactility/concurrent/EventGroup.h>
+#include <Tactility/concurrent/Mutex.h>
+#include <atomic>
+
+#define TAG LOG_TAG("Dispatcher")
+
+static constexpr EventBits_t BACKPRESSURE_WARNING_COUNT = 100U;
+static constexpr EventBits_t WAIT_FLAG = 1U;
+
+struct QueuedItem {
+    DispatcherCallback callback;
+    void* context;
+};
+
+struct DispatcherData {
+    Mutex mutex = { 0 };
+    std::queue<QueuedItem> queue = {};
+    EventGroupHandle_t eventGroup = nullptr;
+    std::atomic<bool> shutdown{false}; // TODO: Use EventGroup
+
+    DispatcherData() {
+        event_group_construct(&eventGroup);
+        mutex_construct(&mutex);
+    }
+
+    ~DispatcherData() {
+        event_group_destruct(&eventGroup);
+        mutex_destruct(&mutex);
+    }
+};
+
+#define dispatcher_data(handle) static_cast<DispatcherData*>(handle)
+
+extern "C" {
+
+DispatcherHandle_t dispatcher_alloc(void) {
+    return new DispatcherData();
+}
+
+void dispatcher_free(DispatcherHandle_t dispatcher) {
+    auto* data = dispatcher_data(dispatcher);
+    data->shutdown.store(true, std::memory_order_release);
+    mutex_lock(&data->mutex);
+    mutex_unlock(&data->mutex);
+    delete data;
+}
+
+error_t dispatcher_dispatch_timed(DispatcherHandle_t dispatcher, void* callbackContext, DispatcherCallback callback, TickType_t timeout) {
+    auto* data = dispatcher_data(dispatcher);
+
+    // Mutate
+    if (!mutex_try_lock_timed(&data->mutex, timeout)) {
+#ifdef ESP_PLATFORM
+        LOG_E(TAG, "Mutex acquisition timeout");
+#endif
+        return ERROR_TIMEOUT;
+    }
+
+    if (data->shutdown.load(std::memory_order_acquire)) {
+        mutex_unlock(&data->mutex);
+        return ERROR_INVALID_STATE;
+    }
+
+    data->queue.push({
+        .callback = callback,
+        .context = callbackContext
+    });
+
+    if (data->queue.size() == BACKPRESSURE_WARNING_COUNT) {
+#ifdef ESP_PLATFORM
+        LOG_W(TAG, "Backpressure: You're not consuming fast enough (100 queued)");
+#endif
+    }
+
+    mutex_unlock(&data->mutex);
+
+    if (event_group_set(data->eventGroup, WAIT_FLAG) != ERROR_NONE) {
+#ifdef ESP_PLATFORM
+        LOG_E(TAG, "Failed to set flag");
+#endif
+        return ERROR_RESOURCE;
+    }
+
+    return ERROR_NONE;
+}
+
+error_t dispatcher_consume_timed(DispatcherHandle_t dispatcher, TickType_t timeout) {
+    auto* data = dispatcher_data(dispatcher);
+
+    // TODO: keep track of time and consider the timeout input as total timeout
+
+    // Wait for signal
+    error_t error = event_group_wait(data->eventGroup, WAIT_FLAG, false, true, nullptr, timeout);
+    if (error != ERROR_NONE) {
+        if (error == ERROR_TIMEOUT) {
+            return ERROR_TIMEOUT;
+        } else {
+            return ERROR_RESOURCE;
+        }
+    }
+
+    if (data->shutdown.load(std::memory_order_acquire)) {
+        return ERROR_INVALID_STATE;
+    }
+
+    // Mutate
+    bool processing = true;
+    do {
+        if (mutex_try_lock_timed(&data->mutex, 10)) {
+            if (!data->queue.empty()) {
+                // Make a copy, so it's thread-safe when we unlock
+                auto entry = data->queue.front();
+                data->queue.pop();
+                processing = !data->queue.empty();
+                // Don't keep lock as callback might be slow and we want to allow dispatch in the meanwhile
+                mutex_unlock(&data->mutex);
+                entry.callback(entry.context);
+            } else {
+                processing = false;
+                mutex_unlock(&data->mutex);
+            }
+        } else {
+#ifdef ESP_PLATFORM
+            LOG_W(TAG, "Mutex acquisition timeout");
+#endif
+        }
+
+    } while (processing && !data->shutdown.load(std::memory_order_acquire));
+
+    return ERROR_NONE;
+}
+
+}

TactilityKernel/Source/concurrent/EventGroup.cpp

@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: Apache-2.0
+
+#include <Tactility/concurrent/EventGroup.h>
+#include <Tactility/Error.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+error_t event_group_set(EventGroupHandle_t eventGroup, uint32_t inFlags) {
+    if (xPortInIsrContext() == pdTRUE) {
+        BaseType_t yield = pdFALSE;
+        if (xEventGroupSetBitsFromISR(eventGroup, inFlags, &yield) == pdFAIL) {
+            return ERROR_RESOURCE;
+        }
+        portYIELD_FROM_ISR(yield);
+    } else {
+        xEventGroupSetBits(eventGroup, inFlags);
+    }
+    return ERROR_NONE;
+}
+
+error_t event_group_clear(EventGroupHandle_t eventGroup, uint32_t flags) {
+    if (xPortInIsrContext() == pdTRUE) {
+        if (xEventGroupClearBitsFromISR(eventGroup, flags) == pdFAIL) {
+            return ERROR_RESOURCE;
+        }
+        portYIELD_FROM_ISR(pdTRUE);
+    } else {
+        xEventGroupClearBits(eventGroup, flags);
+    }
+    return ERROR_NONE;
+}
+
+uint32_t event_group_get(EventGroupHandle_t eventGroup) {
+    if (xPortInIsrContext() == pdTRUE) {
+        return xEventGroupGetBitsFromISR(eventGroup);
+    } else {
+        return xEventGroupGetBits(eventGroup);
+    }
+}
+
+error_t event_group_wait(
+    EventGroupHandle_t eventGroup,
+    uint32_t inFlags,
+    bool awaitAll,
+    bool clearOnExit,
+    uint32_t* outFlags,
+    TickType_t timeout
+) {
+    if (xPortInIsrContext()) {
+        return ERROR_ISR_STATUS;
+    }
+
+    uint32_t result_flags = xEventGroupWaitBits(
+        eventGroup,
+        inFlags,
+        clearOnExit ? pdTRUE : pdFALSE,
+        awaitAll ? pdTRUE : pdFALSE,
+        timeout
+    );
+
+    auto invalid_flags = awaitAll
+        ? ((inFlags & result_flags) != inFlags) // await all
+        : ((inFlags & result_flags) == 0U); // await any
+
+    if (invalid_flags) {
+        const uint32_t matched = inFlags & result_flags;
+        if (matched == 0U) {
+            return ERROR_TIMEOUT;
+        }
+        return ERROR_RESOURCE;
+    }
+
+    if (outFlags != nullptr) {
+        *outFlags = result_flags;
+    }
+
+    return ERROR_NONE;
+}
+
+#ifdef __cplusplus
+}
+#endif

TactilityKernel/Source/drivers/GpioController.cpp

@@ -7,22 +7,22 @@
 
 extern "C" {
 
-bool gpio_controller_set_level(Device* device, gpio_pin_t pin, bool high) {
+error_t gpio_controller_set_level(Device* device, gpio_pin_t pin, bool high) {
     const auto* driver = device_get_driver(device);
     return GPIO_DRIVER_API(driver)->set_level(device, pin, high);
 }
 
-bool gpio_controller_get_level(Device* device, gpio_pin_t pin, bool* high) {
+error_t gpio_controller_get_level(Device* device, gpio_pin_t pin, bool* high) {
     const auto* driver = device_get_driver(device);
     return GPIO_DRIVER_API(driver)->get_level(device, pin, high);
 }
 
-bool gpio_controller_set_options(Device* device, gpio_pin_t pin, gpio_flags_t options) {
+error_t gpio_controller_set_options(Device* device, gpio_pin_t pin, gpio_flags_t options) {
     const auto* driver = device_get_driver(device);
     return GPIO_DRIVER_API(driver)->set_options(device, pin, options);
 }
 
-bool gpio_controller_get_options(Device* device, gpio_pin_t pin, gpio_flags_t* options) {
+error_t gpio_controller_get_options(Device* device, gpio_pin_t pin, gpio_flags_t* options) {
     const auto* driver = device_get_driver(device);
     return GPIO_DRIVER_API(driver)->get_options(device, pin, options);
 }

TactilityKernel/Source/drivers/I2cController.cpp

@@ -2,24 +2,25 @@
 
 #include <Tactility/drivers/I2cController.h>
 #include <Tactility/Driver.h>
+#include <Tactility/Error.h>
 
 #define I2C_DRIVER_API(driver) ((struct I2cControllerApi*)driver->api)
 
 extern "C" {
 
-bool i2c_controller_read(Device* device, uint8_t address, uint8_t* data, size_t dataSize, TickType_t timeout) {
+error_t i2c_controller_read(Device* device, uint8_t address, uint8_t* data, size_t dataSize, TickType_t timeout) {
     const auto* driver = device_get_driver(device);
     return I2C_DRIVER_API(driver)->read(device, address, data, dataSize, timeout);
 }
 
-bool i2c_controller_write(Device* device, uint8_t address, const uint8_t* data, uint16_t dataSize, TickType_t timeout) {
+error_t i2c_controller_write(Device* device, uint8_t address, const uint8_t* data, uint16_t dataSize, TickType_t timeout) {
     const auto* driver = device_get_driver(device);
     return I2C_DRIVER_API(driver)->write(device, address, data, dataSize, timeout);
 }
 
-bool i2c_controller_write_read(Device* device, uint8_t address, const uint8_t* write_data, size_t write_data_size, uint8_t* read_data, size_t read_data_size, TickType_t timeout) {
+error_t i2c_controller_write_read(Device* device, uint8_t address, const uint8_t* writeData, size_t writeDataSize, uint8_t* readData, size_t readDataSize, TickType_t timeout) {
     const auto* driver = device_get_driver(device);
-    return I2C_DRIVER_API(driver)->write_read(device, address, write_data, write_data_size, read_data, read_data_size, timeout);
+    return I2C_DRIVER_API(driver)->write_read(device, address, writeData, writeDataSize, readData, readDataSize, timeout);
 }
 
 const struct DeviceType I2C_CONTROLLER_TYPE { 0 };

TactilityKernel/Source/drivers/Root.cpp

@@ -8,10 +8,10 @@ extern "C" {
 Driver root_driver = {
     .name = "root",
     .compatible = (const char*[]) { "root", nullptr },
-    .start_device = nullptr,
-    .stop_device = nullptr,
+    .startDevice = nullptr,
+    .stopDevice = nullptr,
     .api = nullptr,
-    .device_type = nullptr,
+    .deviceType = nullptr,
     .internal = { 0 }
 };