TactilityKernel improvements

CODING_STYLE_C.md

@@ -1,14 +1,14 @@
 # C coding Style
 
-## Naming
+## Files & Folders
 
 ### Files
 
 Files are lower snake case.
 
 - Files: `^[0-9a-z_]+$`
 - Directories: `^[0-9a-z_]+$`
- 
+
 Example:
 ```c
 some_feature.c
@@ -22,6 +22,8 @@ Project folders include:
 - `private` for private header files
 - `include` for projects that require separate header files
 
+## C language
+
 ### Macros and consts
 
 These are all upper snake case:
@@ -94,3 +96,35 @@ Examples:
 ```c
 typedef uint32_t thread_id_t;
 ```
+
+### Function comments
+
+```c
+/**
+ * @brief Validates a number
+ * @param[in] number the integer to validate
+ * @return true if validation was succesful and there were no issues
+ */
+bool validate(int number);
+
+/**
+ * @brief Run the action.
+ * @param timeout[in] the maximum time the task should run
+ * @retval ERROR_TIMEOUT when the task couldn't be completed on time
+ * @retval ERROR_NONE when the task completed successfully
+ */
+error_t runAction(TickType_t timeout);
+
+/**
+ * @brief Increase a number.
+ * @param[inout] number
+ */
+void increase(int* number);
+
+/**
+ * A function with a longer description here.
+ *
+ * @brief short description
+ */
+void something();
+```

Platforms/PlatformEsp32/Source/drivers/esp32_i2c.cpp

@@ -9,6 +9,7 @@
 #include <tactility/drivers/esp32_i2c.h>
 
 #define TAG LOG_TAG(esp32_i2c)
+#define ACK_CHECK_EN 1
 
 struct InternalData {
     Mutex mutex { 0 };
@@ -30,8 +31,9 @@ struct InternalData {
 
 extern "C" {
 
-static int read(Device* device, uint8_t address, uint8_t* data, size_t data_size, TickType_t timeout) {
-    vPortAssertIfInISR();
+static error_t read(Device* device, uint8_t address, uint8_t* data, size_t data_size, TickType_t timeout) {
+    if (xPortInIsrContext()) return ERROR_ISR_STATUS;
+    if (data_size == 0) return ERROR_INVALID_ARGUMENT;
     auto* driver_data = GET_DATA(device);
     lock(driver_data);
     const esp_err_t esp_error = i2c_master_read_from_device(GET_CONFIG(device)->port, address, data, data_size, timeout);
@@ -40,18 +42,19 @@ static int read(Device* device, uint8_t address, uint8_t* data, size_t data_size
     return esp_err_to_error(esp_error);
 }
 
-static int write(Device* device, uint8_t address, const uint8_t* data, uint16_t dataSize, TickType_t timeout) {
-    vPortAssertIfInISR();
+static error_t write(Device* device, uint8_t address, const uint8_t* data, uint16_t data_size, TickType_t timeout) {
+    if (xPortInIsrContext()) return ERROR_ISR_STATUS;
+    if (data_size == 0) return ERROR_INVALID_ARGUMENT;
     auto* driver_data = GET_DATA(device);
     lock(driver_data);
-    const esp_err_t esp_error = i2c_master_write_to_device(GET_CONFIG(device)->port, address, data, dataSize, timeout);
+    const esp_err_t esp_error = i2c_master_write_to_device(GET_CONFIG(device)->port, address, data, data_size, timeout);
     unlock(driver_data);
     ESP_ERROR_CHECK_WITHOUT_ABORT(esp_error);
     return esp_err_to_error(esp_error);
 }
 
-static int 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) {
-    vPortAssertIfInISR();
+static error_t 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) {
+    if (xPortInIsrContext()) return ERROR_ISR_STATUS;
     auto* driver_data = GET_DATA(device);
     lock(driver_data);
     const esp_err_t esp_error = i2c_master_write_read_device(GET_CONFIG(device)->port, address, write_data, write_data_size, read_data, read_data_size, timeout);
@@ -60,25 +63,77 @@ static int write_read(Device* device, uint8_t address, const uint8_t* write_data
     return esp_err_to_error(esp_error);
 }
 
-static int start(Device* device) {
+static error_t read_register(Device* device, uint8_t address, uint8_t reg, uint8_t* data, size_t data_size, TickType_t timeout) {
+    if (xPortInIsrContext()) return ERROR_ISR_STATUS;
+    if (data_size == 0) return ERROR_INVALID_ARGUMENT;
+    auto* driver_data = GET_DATA(device);
+
+    lock(driver_data);
+    i2c_cmd_handle_t cmd = i2c_cmd_link_create();
+    // Set address pointer
+    i2c_master_start(cmd);
+    i2c_master_write_byte(cmd, (address << 1) | I2C_MASTER_WRITE, ACK_CHECK_EN);
+    i2c_master_write(cmd, &reg, 1, ACK_CHECK_EN);
+    // Read length of response from current pointer
+    i2c_master_start(cmd);
+    i2c_master_write_byte(cmd, (address << 1) | I2C_MASTER_READ, ACK_CHECK_EN);
+    if (data_size > 1) {
+        i2c_master_read(cmd, data, data_size - 1, I2C_MASTER_ACK);
+    }
+    i2c_master_read_byte(cmd, data + data_size - 1, I2C_MASTER_NACK);
+    i2c_master_stop(cmd);
+    // TODO: We're passing an inaccurate timeout value as we already lost time with locking
+    esp_err_t esp_error = i2c_master_cmd_begin(GET_CONFIG(device)->port, cmd, timeout);
+    i2c_cmd_link_delete(cmd);
+    unlock(driver_data);
+
+    ESP_ERROR_CHECK_WITHOUT_ABORT(esp_error);
+    return esp_err_to_error(esp_error);
+}
+
+static error_t write_register(Device* device, uint8_t address, uint8_t reg, const uint8_t* data, uint16_t data_size, TickType_t timeout) {
+    if (xPortInIsrContext()) return ERROR_ISR_STATUS;
+    if (data_size == 0) return ERROR_INVALID_ARGUMENT;
+    auto* driver_data = GET_DATA(device);
+
+    lock(driver_data);
+    i2c_cmd_handle_t cmd = i2c_cmd_link_create();
+    i2c_master_start(cmd);
+    i2c_master_write_byte(cmd, (address << 1) | I2C_MASTER_WRITE, ACK_CHECK_EN);
+    i2c_master_write_byte(cmd, reg, ACK_CHECK_EN);
+    i2c_master_write(cmd, (uint8_t*) data, data_size, ACK_CHECK_EN);
+    i2c_master_stop(cmd);
+    // TODO: We're passing an inaccurate timeout value as we already lost time with locking
+    esp_err_t esp_error = i2c_master_cmd_begin(GET_CONFIG(device)->port, cmd, timeout);
+    i2c_cmd_link_delete(cmd);
+    unlock(driver_data);
+
+    ESP_ERROR_CHECK_WITHOUT_ABORT(esp_error);
+    return esp_err_to_error(esp_error);
+}
+
+static error_t start(Device* device) {
     ESP_LOGI(TAG, "start %s", device->name);
     auto* data = new InternalData();
     device_set_driver_data(device, data);
     return ERROR_NONE;
 }
 
-static int stop(Device* device) {
+static error_t stop(Device* device) {
     ESP_LOGI(TAG, "stop %s", device->name);
     auto* driver_data = static_cast<InternalData*>(device_get_driver_data(device));
     device_set_driver_data(device, nullptr);
     delete driver_data;
     return ERROR_NONE;
 }
 
+
 const static I2cControllerApi esp32_i2c_api = {
     .read = read,
     .write = write,
-    .write_read = write_read
+    .write_read = write_read,
+    .read_register = read_register,
+    .write_register = write_register
 };
 
 Driver esp32_i2c_driver = {

TactilityKernel/CMakeLists.txt

@@ -7,6 +7,8 @@ if (DEFINED ENV{ESP_IDF_VERSION})
     idf_component_register(
         SRCS ${SOURCES}
         INCLUDE_DIRS "Include/"
+        # TODO move the related logic for esp_time in Tactility/time.h into the Platform/ subproject
+        REQUIRES esp_timer
     )
 
 else ()

TactilityKernel/Include/tactility/concurrent/thread.h

@@ -0,0 +1,182 @@
+// SPDX-License-Identifier: Apache-2.0
+#pragma once
+
+#include "tactility/error.h"
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#ifdef ESP_PLATFORM
+#include <esp_log.h>
+#endif
+
+#include <tactility/freertos/task.h>
+#include <tactility/concurrent/mutex.h>
+
+#include <assert.h>
+#include <stdbool.h>
+#include <stdint.h>
+
+typedef enum {
+    THREAD_STATE_STOPPED,
+    THREAD_STATE_STARTING,
+    THREAD_STATE_RUNNING,
+} ThreadState;
+
+/** ThreadPriority */
+enum ThreadPriority {
+    THREAD_PRIORITY_NONE = 0U,
+    THREAD_PRIORITY_IDLE = 1U,
+    THREAD_PRIORITY_LOWER = 2U,
+    THREAD_PRIORITY_LOW = 3U,
+    THREAD_PRIORITY_NORMAL = 4U,
+    THREAD_PRIORITY_HIGH = 5U,
+    THREAD_PRIORITY_HIGHER = 6U,
+    THREAD_PRIORITY_CRITICAL = 7U
+};
+
+typedef int32_t (*thread_main_fn_t)(void* context);
+typedef void (*thread_state_callback_t)(ThreadState state, void* context);
+
+struct Thread;
+typedef struct Thread Thread;
+
+/**
+ * @brief Creates a new thread instance with default settings.
+ * @return A pointer to the created Thread instance, or NULL if allocation failed.
+ */
+Thread* thread_alloc(void);
+
+/**
+ * @brief Creates a new thread instance with specified parameters.
+ * @param[in] name The name of the thread.
+ * @param[in] stack_size The size of the thread stack in bytes.
+ * @param[in] function The main function to be executed by the thread.
+ * @param[in] function_context A pointer to the context to be passed to the main function.
+ * @param[in] affinity The CPU core affinity for the thread (e.g., tskNO_AFFINITY).
+ * @return A pointer to the created Thread instance, or NULL if allocation failed.
+ */
+Thread* thread_alloc_full(
+    const char* name,
+    configSTACK_DEPTH_TYPE stack_size,
+    thread_main_fn_t function,
+    void* function_context,
+    portBASE_TYPE affinity
+);
+
+/**
+ * @brief Destroys a thread instance.
+ * @param[in] thread The thread instance to destroy.
+ * @note The thread must be in the STOPPED state.
+ */
+void thread_free(Thread* thread);
+
+/**
+ * @brief Sets the name of the thread.
+ * @param[in] thread The thread instance.
+ * @param[in] name The new name for the thread.
+ * @note Can only be called when the thread is in the STOPPED state.
+ */
+void thread_set_name(Thread* thread, const char* name);
+
+/**
+ * @brief Sets the stack size for the thread.
+ * @param[in] thread The thread instance.
+ * @param[in] stack_size The stack size in bytes. Must be a multiple of 4.
+ * @note Can only be called when the thread is in the STOPPED state.
+ */
+void thread_set_stack_size(Thread* thread, size_t stack_size);
+
+/**
+ * @brief Sets the CPU core affinity for the thread.
+ * @param[in] thread The thread instance.
+ * @param[in] affinity The CPU core affinity.
+ * @note Can only be called when the thread is in the STOPPED state.
+ */
+void thread_set_affinity(Thread* thread, portBASE_TYPE affinity);
+
+/**
+ * @brief Sets the main function and context for the thread.
+ * @param[in] thread The thread instance.
+ * @param[in] function The main function to be executed.
+ * @param[in] context A pointer to the context to be passed to the main function.
+ * @note Can only be called when the thread is in the STOPPED state.
+ */
+void thread_set_main_function(Thread* thread, thread_main_fn_t function, void* context);
+
+/**
+ * @brief Sets the priority for the thread.
+ * @param[in] thread The thread instance.
+ * @param[in] priority The thread priority.
+ * @note Can only be called when the thread is in the STOPPED state.
+ */
+void thread_set_priority(Thread* thread, enum ThreadPriority priority);
+
+/**
+ * @brief Sets a callback to be invoked when the thread state changes.
+ * @param[in] thread The thread instance.
+ * @param[in] callback The callback function.
+ * @param[in] context A pointer to the context to be passed to the callback function.
+ * @note Can only be called when the thread is in the STOPPED state.
+ */
+void thread_set_state_callback(Thread* thread, thread_state_callback_t callback, void* context);
+
+/**
+ * @brief Gets the current state of the thread.
+ * @param[in] thread The thread instance.
+ * @return The current ThreadState.
+ */
+ThreadState thread_get_state(Thread* thread);
+
+/**
+ * @brief Starts the thread execution.
+ * @param[in] thread The thread instance.
+ * @note The thread must be in the STOPPED state and have a main function set.
+ * @retval ERROR_NONE when the thread was started
+ * @retval ERROR_UNDEFINED when the thread failed to start
+ */
+error_t thread_start(Thread* thread);
+
+/**
+ * @brief Waits for the thread to finish execution.
+ * @param[in] thread The thread instance.
+ * @param[in] timeout The maximum time to wait in ticks.
+ * @param[in] poll_interval The interval between status checks in ticks.
+ * @retval ERROR_NONE when the thread was stopped
+ * @retval ERROR_TIMEOUT when the thread was not stopped because the timeout has passed
+ * @note Cannot be called from the thread being joined.
+ */
+error_t thread_join(Thread* thread, TickType_t timeout, TickType_t poll_interval);
+
+/**
+ * @brief Gets the FreeRTOS task handle associated with the thread.
+ * @param[in] thread The thread instance.
+ * @return The TaskHandle_t, or NULL if the thread is not running.
+ */
+TaskHandle_t thread_get_task_handle(Thread* thread);
+
+/**
+ * @brief Gets the return code from the thread's main function.
+ * @param[in] thread The thread instance.
+ * @return The return code of the thread's main function.
+ * @note The thread must be in the STOPPED state.
+ */
+int32_t thread_get_return_code(Thread* thread);
+
+/**
+ * @brief Gets the minimum remaining stack space for the thread since it started.
+ * @param[in] thread The thread instance.
+ * @return The minimum remaining stack space in bytes.
+ * @note The thread must be in the RUNNING state.
+ */
+uint32_t thread_get_stack_space(Thread* thread);
+
+/**
+ * @brief Gets the current thread instance.
+ * @return A pointer to the current Thread instance, or NULL if not called from a thread created by this module.
+ */
+Thread* thread_get_current(void);
+
+#ifdef __cplusplus
+}
+#endif