From 2cd2c5f180c081ed7c6aefc66d5bd8d54c5082c7 Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Wed, 28 Jan 2026 17:52:49 +0100 Subject: [PATCH] Fixes and improvements --- .../Include/tactility/error_esp32.h | 1 + .../Source/drivers/esp32_i2c.cpp | 85 +++++++++++++------ .../PlatformEsp32/Source/error_esp32.cpp | 4 + TactilityKernel/Include/tactility/defines.h | 21 +++++ TactilityKernel/Include/tactility/error.h | 4 + TactilityKernel/Include/tactility/time.h | 12 +++ TactilityKernel/Source/error.cpp | 31 +++++++ 7 files changed, 133 insertions(+), 25 deletions(-) create mode 100644 TactilityKernel/Include/tactility/defines.h create mode 100644 TactilityKernel/Source/error.cpp diff --git a/Platforms/PlatformEsp32/Include/tactility/error_esp32.h b/Platforms/PlatformEsp32/Include/tactility/error_esp32.h index e0ab898c..d885efcd 100644 --- a/Platforms/PlatformEsp32/Include/tactility/error_esp32.h +++ b/Platforms/PlatformEsp32/Include/tactility/error_esp32.h @@ -4,4 +4,5 @@ #include +/** Convert an esp_err_t to an error_t */ error_t esp_err_to_error(esp_err_t error); diff --git a/Platforms/PlatformEsp32/Source/drivers/esp32_i2c.cpp b/Platforms/PlatformEsp32/Source/drivers/esp32_i2c.cpp index 23c8067f..1d8834b4 100644 --- a/Platforms/PlatformEsp32/Source/drivers/esp32_i2c.cpp +++ b/Platforms/PlatformEsp32/Source/drivers/esp32_i2c.cpp @@ -5,6 +5,7 @@ #include #include +#include #include #include @@ -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 - i2c_master_start(cmd); - i2c_master_write_byte(cmd, (address << 1) | I2C_MASTER_WRITE, ACK_CHECK_EN); - i2c_master_write(cmd, ®, 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); + esp_err_t error = ESP_OK; + if (cmd == nullptr) { + error = ESP_ERR_NO_MEM; + goto on_error; } - 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); + // Set address pointer + error = i2c_master_start(cmd); + 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(cmd, ®, 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); - return esp_err_to_error(esp_error); +on_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); - 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); + esp_err_t error = ESP_OK; + if (cmd == nullptr) { + error = ESP_ERR_NO_MEM; + goto on_error; + } + error = i2c_master_start(cmd); + 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); - return esp_err_to_error(esp_error); +on_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, diff --git a/Platforms/PlatformEsp32/Source/error_esp32.cpp b/Platforms/PlatformEsp32/Source/error_esp32.cpp index 62f72dd5..da5d4a2c 100644 --- a/Platforms/PlatformEsp32/Source/error_esp32.cpp +++ b/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; } diff --git a/TactilityKernel/Include/tactility/defines.h b/TactilityKernel/Include/tactility/defines.h new file mode 100644 index 00000000..93b8448a --- /dev/null +++ b/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 diff --git a/TactilityKernel/Include/tactility/error.h b/TactilityKernel/Include/tactility/error.h index 000dba6c..f3b98be9 100644 --- a/TactilityKernel/Include/tactility/error.h +++ b/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); diff --git a/TactilityKernel/Include/tactility/time.h b/TactilityKernel/Include/tactility/time.h index 0701ddeb..98471e7b 100644 --- a/TactilityKernel/Include/tactility/time.h +++ b/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 +#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(); diff --git a/TactilityKernel/Source/error.cpp b/TactilityKernel/Source/error.cpp new file mode 100644 index 00000000..51a594c9 --- /dev/null +++ b/TactilityKernel/Source/error.cpp @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: Apache-2.0 +#include + +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"; + } +}