Fixes and improvements

Platforms/PlatformEsp32/Include/tactility/error_esp32.h

@@ -4,4 +4,5 @@
 
 #include <tactility/error.h>
 
+/** Convert an esp_err_t to an error_t */
 error_t esp_err_to_error(esp_err_t error);

Platforms/PlatformEsp32/Source/drivers/esp32_i2c.cpp

@@ -5,6 +5,7 @@
 #include <tactility/drivers/i2c_controller.h>
 #include <tactility/log.h>
 
+#include <tactility/time.h>
 #include <tactility/error_esp32.h>
 #include <tactility/drivers/esp32_i2c.h>
 
@@ -64,52 +65,87 @@ static error_t write_read(Device* device, uint8_t address, const uint8_t* write_
 }
 
 static error_t read_register(Device* device, uint8_t address, uint8_t reg, uint8_t* data, size_t data_size, TickType_t timeout) {
+    auto start_time = get_ticks();
     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
+    esp_err_t error = ESP_OK;
-    i2c_master_start(cmd);
+    if (cmd == nullptr) {
-    i2c_master_write_byte(cmd, (address << 1) | I2C_MASTER_WRITE, ACK_CHECK_EN);
+        error = ESP_ERR_NO_MEM;
-    i2c_master_write(cmd, &reg, 1, ACK_CHECK_EN);
+        goto on_error;
-    // 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);
+    // Set address pointer
-    i2c_master_stop(cmd);
+    error = i2c_master_start(cmd);
-    // TODO: We're passing an inaccurate timeout value as we already lost time with locking
+    if (error != ESP_OK) goto on_error;
-    esp_err_t esp_error = i2c_master_cmd_begin(GET_CONFIG(device)->port, cmd, timeout);
+    error = i2c_master_write_byte(cmd, (address << 1) | I2C_MASTER_WRITE, ACK_CHECK_EN);
+    if (error != ESP_OK) goto on_error;
+    error = i2c_master_write(cmd, &reg, 1, ACK_CHECK_EN);
+    if (error != ESP_OK) goto on_error;
+    // Read length of response from current pointer
+    error = i2c_master_start(cmd);
+    if (error != ESP_OK) goto on_error;
+    error = i2c_master_write_byte(cmd, (address << 1) | I2C_MASTER_READ, ACK_CHECK_EN);
+    if (error != ESP_OK) goto on_error;
+    if (data_size > 1) {
+        error = i2c_master_read(cmd, data, data_size - 1, I2C_MASTER_ACK);
+        if (error != ESP_OK) goto on_error;
+    }
+    error = i2c_master_read_byte(cmd, data + data_size - 1, I2C_MASTER_NACK);
+    if (error != ESP_OK) goto on_error;
+    error = i2c_master_stop(cmd);
+    if (error != ESP_OK) goto on_error;
+    error = i2c_master_cmd_begin(GET_CONFIG(device)->port, cmd, get_timeout_remaining_ticks(timeout, start_time));
+    if (error != ESP_OK) goto on_error;
+
     i2c_cmd_link_delete(cmd);
     unlock(driver_data);
+    ESP_ERROR_CHECK_WITHOUT_ABORT(error);
+    return esp_err_to_error(error);
 
-    ESP_ERROR_CHECK_WITHOUT_ABORT(esp_error);
+on_error:
-    return esp_err_to_error(esp_error);
+    i2c_cmd_link_delete(cmd);
+    unlock(driver_data);
+    return esp_err_to_error(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) {
+    auto start_time = get_ticks();
     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);
+    esp_err_t error = ESP_OK;
-    i2c_master_write_byte(cmd, (address << 1) | I2C_MASTER_WRITE, ACK_CHECK_EN);
+    if (cmd == nullptr) {
-    i2c_master_write_byte(cmd, reg, ACK_CHECK_EN);
+        error = ESP_ERR_NO_MEM;
-    i2c_master_write(cmd, (uint8_t*) data, data_size, ACK_CHECK_EN);
+        goto on_error;
-    i2c_master_stop(cmd);
+    }
-    // TODO: We're passing an inaccurate timeout value as we already lost time with locking
+    error = i2c_master_start(cmd);
-    esp_err_t esp_error = i2c_master_cmd_begin(GET_CONFIG(device)->port, cmd, timeout);
+    if (error != ESP_OK) goto on_error;
+    error = i2c_master_write_byte(cmd, (address << 1) | I2C_MASTER_WRITE, ACK_CHECK_EN);
+    if (error != ESP_OK) goto on_error;
+    error = i2c_master_write_byte(cmd, reg, ACK_CHECK_EN);
+    if (error != ESP_OK) goto on_error;
+    error = i2c_master_write(cmd, (uint8_t*) data, data_size, ACK_CHECK_EN);
+    if (error != ESP_OK) goto on_error;
+    error = i2c_master_stop(cmd);
+    if (error != ESP_OK) goto on_error;
+    error = i2c_master_cmd_begin(GET_CONFIG(device)->port, cmd, get_timeout_remaining_ticks(timeout, start_time));
+    if (error != ESP_OK) goto on_error;
+
     i2c_cmd_link_delete(cmd);
     unlock(driver_data);
+    ESP_ERROR_CHECK_WITHOUT_ABORT(error);
+    return esp_err_to_error(error);
 
-    ESP_ERROR_CHECK_WITHOUT_ABORT(esp_error);
+on_error:
-    return esp_err_to_error(esp_error);
+    i2c_cmd_link_delete(cmd);
+    unlock(driver_data);
+    return esp_err_to_error(error);
 }
 
 static error_t start(Device* device) {
@@ -127,7 +163,6 @@ static error_t stop(Device* device) {
     return ERROR_NONE;
 }
 
-
 const static I2cControllerApi esp32_i2c_api = {
     .read = read,
     .write = write,

Platforms/PlatformEsp32/Source/error_esp32.cpp

@@ -11,6 +11,10 @@ error_t esp_err_to_error(esp_err_t error) {
             return ERROR_INVALID_STATE;
         case ESP_ERR_TIMEOUT:
             return ERROR_TIMEOUT;
+        case ESP_ERR_NO_MEM:
+            return ERROR_OUT_OF_MEMORY;
+        case ESP_ERR_NOT_SUPPORTED:
+            return ERROR_NOT_SUPPORTED;
         default:
             return ERROR_UNDEFINED;
     }

TactilityKernel/Include/tactility/defines.h

@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: Apache-2.0
+/**
+ * @brief Contains various unsorted defines
+ * @note Preprocessor defines with potentially clashing names implement an #ifdef check.
+ */
+#pragma once
+
+#ifndef MIN
+/** @brief Get the minimum value of 2 values */
+#define MIN(a, b) (a < b ? a : b)
+#endif
+
+#ifndef MAX
+/** @brief Get the maximum value of 2 values */
+#define MAX(a, b) (a > b ? a : b)
+#endif
+
+#ifndef CLAMP
+/** @brief Clamp a value between the provided minimum and maximum */
+#define CLAMP(min, max, value) (value < min) ? min : (value > max ? max : value)
+#endif

TactilityKernel/Include/tactility/error.h

@@ -17,3 +17,7 @@ typedef int error_t;
 #define ERROR_RESOURCE 7 // A problem with a resource/dependency
 #define ERROR_TIMEOUT 8
 #define ERROR_OUT_OF_MEMORY 9
+#define ERROR_NOT_SUPPORTED 10
+
+/** Convert an error_t to a human-readable text. Useful for logging. */
+const char* error_to_string(error_t error);

TactilityKernel/Include/tactility/time.h

@@ -1,7 +1,13 @@
+// SPDX-License-Identifier: Apache-2.0
+/**
+ * Time-keeping related functionality.
+ * This includes functionality for both ticks and seconds.
+ */
 #pragma once
 
 #include <stdint.h>
 
+#include "defines.h"
 #include "tactility/freertos/task.h"
 
 #ifdef ESP_PLATFORM
@@ -32,6 +38,12 @@ static inline size_t get_millis() {
     return get_ticks() * portTICK_PERIOD_MS;
 }
 
+static inline TickType_t get_timeout_remaining_ticks(TickType_t timeout, TickType_t start_time) {
+    auto ticks_passed = get_ticks() - start_time;
+    auto ticks_remaining = (timeout - ticks_passed);
+    return MAX(ticks_remaining, 0);
+}
+
 /** @return the frequency at which the kernel task schedulers operate */
 uint32_t kernel_get_tick_frequency();
 

TactilityKernel/Source/error.cpp

@@ -0,0 +1,31 @@
+// SPDX-License-Identifier: Apache-2.0
+#include <tactility/error.h>
+
+const char* error_to_string(error_t error) {
+    switch (error) {
+        case ERROR_NONE:
+            return "no error";
+        case ERROR_UNDEFINED:
+            return "undefined";
+        case ERROR_INVALID_STATE:
+            return "invalid state";
+        case ERROR_INVALID_ARGUMENT:
+            return "invalid argument";
+        case ERROR_MISSING_PARAMETER:
+            return "missing parameter";
+        case ERROR_NOT_FOUND:
+            return "not found";
+        case ERROR_ISR_STATUS:
+            return "ISR status";
+        case ERROR_RESOURCE:
+            return "resource";
+        case ERROR_TIMEOUT:
+            return "timeout";
+        case ERROR_OUT_OF_MEMORY:
+            return "out of memory";
+        case ERROR_NOT_SUPPORTED:
+            return "not supported";
+        default:
+            return "unknown";
+    }
+}