From 2e86d4774be38af08ddf8a1f1dc06bc3a7a6f4b8 Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Wed, 12 Feb 2025 18:12:20 +0100 Subject: [PATCH] Added HAL docs, improved HAL init&locking (#218) --- TactilityC/Source/tt_hal_i2c.cpp | 4 +- TactilityCore/Include/Tactility/Lockable.h | 3 + TactilityCore/Source/Lockable.cpp | 4 + .../Include/Tactility/hal/Device.h | 7 +- .../Include/Tactility/hal/i2c/I2c.h | 61 ++++-- .../Include/Tactility/hal/spi/Spi.h | 16 +- .../Include/Tactility/hal/uart/Uart.h | 75 ++++++-- .../Private/Tactility/hal/gps/GpsInit.h | 5 + .../Private/Tactility/hal/i2c/I2cInit.h | 15 ++ .../Private/Tactility/hal/spi/SpiInit.h | 17 ++ .../Private/Tactility/hal/uart/UartInit.h | 9 + TactilityHeadless/Source/hal/Hal.cpp | 6 +- .../Source/hal/gps/GpsDeviceInitDefault.cpp | 12 +- TactilityHeadless/Source/hal/i2c/I2c.cpp | 141 +++++--------- TactilityHeadless/Source/hal/spi/Spi.cpp | 83 +++----- TactilityHeadless/Source/hal/uart/Uart.cpp | 177 +++++++----------- 16 files changed, 333 insertions(+), 302 deletions(-) create mode 100644 TactilityHeadless/Private/Tactility/hal/i2c/I2cInit.h create mode 100644 TactilityHeadless/Private/Tactility/hal/spi/SpiInit.h create mode 100644 TactilityHeadless/Private/Tactility/hal/uart/UartInit.h diff --git a/TactilityC/Source/tt_hal_i2c.cpp b/TactilityC/Source/tt_hal_i2c.cpp index e2ca0798..b26c9979 100644 --- a/TactilityC/Source/tt_hal_i2c.cpp +++ b/TactilityC/Source/tt_hal_i2c.cpp @@ -40,11 +40,11 @@ bool tt_hal_i2c_master_has_device_at_address(i2c_port_t port, uint8_t address, T } bool tt_hal_i2c_lock(i2c_port_t port, TickType_t timeout) { - return tt::hal::i2c::lock(port, timeout); + return tt::hal::i2c::getLock(port).lock(timeout); } bool tt_hal_i2c_unlock(i2c_port_t port) { - return tt::hal::i2c::unlock(port); + return tt::hal::i2c::getLock(port).unlock(); } } \ No newline at end of file diff --git a/TactilityCore/Include/Tactility/Lockable.h b/TactilityCore/Include/Tactility/Lockable.h index 1ccf03bc..033e61a9 100644 --- a/TactilityCore/Include/Tactility/Lockable.h +++ b/TactilityCore/Include/Tactility/Lockable.h @@ -43,7 +43,10 @@ public: void withLock(const std::function& onLockAcquired, const std::function& onLockFailed) const { withLock(portMAX_DELAY, onLockAcquired, onLockFailed); } + [[deprecated("use asScopedLock()")]] std::unique_ptr scoped() const; + + ScopedLockableUsage asScopedLock() const; }; diff --git a/TactilityCore/Source/Lockable.cpp b/TactilityCore/Source/Lockable.cpp index 643e369b..2ec1c207 100644 --- a/TactilityCore/Source/Lockable.cpp +++ b/TactilityCore/Source/Lockable.cpp @@ -6,4 +6,8 @@ std::unique_ptr Lockable::scoped() const { return std::make_unique(*this); } +ScopedLockableUsage Lockable::asScopedLock() const { + return ScopedLockableUsage(*this); +} + } diff --git a/TactilityHeadless/Include/Tactility/hal/Device.h b/TactilityHeadless/Include/Tactility/hal/Device.h index b1668702..0f01fe18 100644 --- a/TactilityHeadless/Include/Tactility/hal/Device.h +++ b/TactilityHeadless/Include/Tactility/hal/Device.h @@ -9,9 +9,7 @@ namespace tt::hal { -/** - * Base class for HAL-related devices. - */ +/** Base class for HAL-related devices. */ class Device { public: @@ -37,9 +35,10 @@ public: Device(); virtual ~Device() = default; + /** Unique identifier */ inline Id getId() const { return id; } - /** The type of device. */ + /** The type of device */ virtual Type getType() const = 0; /** The part number or hardware name e.g. TdeckTouch, TdeckDisplay, BQ24295, etc. */ diff --git a/TactilityHeadless/Include/Tactility/hal/i2c/I2c.h b/TactilityHeadless/Include/Tactility/hal/i2c/I2c.h index c82c98e4..91346112 100644 --- a/TactilityHeadless/Include/Tactility/hal/i2c/I2c.h +++ b/TactilityHeadless/Include/Tactility/hal/i2c/I2c.h @@ -1,15 +1,17 @@ #pragma once #include "./I2cCompat.h" +#include "Tactility/Lockable.h" #include #include #include -#include namespace tt::hal::i2c { +constexpr TickType_t defaultTimeout = 10 / portTICK_PERIOD_MS; + enum class InitMode { ByTactility, // Tactility will initialize it in the correct bootup phase ByExternal, // The device is already initialized and Tactility should assume it works @@ -36,22 +38,57 @@ enum class Status { Unknown }; -bool init(const std::vector& configurations); - +/** + * Reconfigure a port with the provided settings. + * @warning This fails when the HAL Configuration does not allow for reinit. + * @warning This fails when the HAL Configuration does not allow for mutation of the device. + * @param[in] port the port to reconfigure + * @param[in] configuration the new configuration + * @return true on success + */ bool configure(i2c_port_t port, const i2c_config_t& configuration); + +/** + * Start the bus for the specified port. + * Devices might be started automatically at boot if their HAL configuration requires it. + */ bool start(i2c_port_t port); + +/** Stop the bus for the specified port. */ bool stop(i2c_port_t port); + +/** @return true if the bus is started */ bool isStarted(i2c_port_t port); -bool masterRead(i2c_port_t port, uint8_t address, uint8_t* data, size_t dataSize, TickType_t timeout); -bool masterReadRegister(i2c_port_t port, uint8_t address, uint8_t reg, uint8_t* data, size_t dataSize, TickType_t timeout); -bool masterWrite(i2c_port_t port, uint8_t address, const uint8_t* data, uint16_t dataSize, TickType_t timeout); -bool masterWriteRegister(i2c_port_t port, uint8_t address, uint8_t reg, const uint8_t* data, uint16_t dataSize, TickType_t timeout); -bool masterWriteRegisterArray(i2c_port_t port, uint8_t address, const uint8_t* data, uint16_t dataSize, TickType_t timeout); -bool masterWriteRead(i2c_port_t port, uint8_t address, const uint8_t* writeData, size_t writeDataSize, uint8_t* readData, size_t readDataSize, TickType_t timeout); -bool masterHasDeviceAtAddress(i2c_port_t port, uint8_t address, TickType_t timeout); +/** Read bytes in master mode. */ +bool masterRead(i2c_port_t port, uint8_t address, uint8_t* data, size_t dataSize, TickType_t timeout = defaultTimeout); -bool lock(i2c_port_t port, TickType_t timeout = 10 / portTICK_PERIOD_MS); -bool unlock(i2c_port_t port); +/** Read bytes from the specified register in master mode. */ +bool masterReadRegister(i2c_port_t port, uint8_t address, uint8_t reg, uint8_t* data, size_t dataSize, TickType_t timeout = defaultTimeout); + +/** Write bytes in master mode. */ +bool masterWrite(i2c_port_t port, uint8_t address, const uint8_t* data, uint16_t dataSize, TickType_t timeout = defaultTimeout); + +/** Write bytes to a register in master mode */ +bool masterWriteRegister(i2c_port_t port, uint8_t address, uint8_t reg, const uint8_t* data, uint16_t dataSize, TickType_t timeout = defaultTimeout); + +/** + * Write multiple values to multiple registers in master mode. + * The input is as follows: { register1, value1, register2, value2, ... } + * @return false if any of the write operations failed + */ +bool masterWriteRegisterArray(i2c_port_t port, uint8_t address, const uint8_t* data, uint16_t dataSize, TickType_t timeout = defaultTimeout); + +/** Write bytes and then read the response bytes in master mode*/ +bool masterWriteRead(i2c_port_t port, uint8_t address, const uint8_t* writeData, size_t writeDataSize, uint8_t* readData, size_t readDataSize, TickType_t timeout = defaultTimeout); + +/** @return true when a device is detected at the specified address */ +bool masterHasDeviceAtAddress(i2c_port_t port, uint8_t address, TickType_t timeout = defaultTimeout); + +/** + * The lock for the specified bus. + * This can be used when calling native I2C functionality outside of Tactility. + */ +Lockable& getLock(i2c_port_t port); } // namespace diff --git a/TactilityHeadless/Include/Tactility/hal/spi/Spi.h b/TactilityHeadless/Include/Tactility/hal/spi/Spi.h index c3097c68..24137a97 100644 --- a/TactilityHeadless/Include/Tactility/hal/spi/Spi.h +++ b/TactilityHeadless/Include/Tactility/hal/spi/Spi.h @@ -5,11 +5,10 @@ #include #include -#include -#include - namespace tt::hal::spi { +constexpr TickType_t defaultTimeout = 10 / portTICK_PERIOD_MS; + enum class InitMode { ByTactility, // Tactility will initialize it in the correct bootup phase ByExternal, // The device is already initialized and Tactility should assume it works @@ -36,13 +35,16 @@ enum class Status { Unknown }; -bool init(const std::vector& configurations); - +/** Start communications */ bool start(spi_host_device_t device); + +/** Stop communications */ bool stop(spi_host_device_t device); + +/** @return true if communications were started successfully */ bool isStarted(spi_host_device_t device); -bool lock(spi_host_device_t device, TickType_t timeout = 10 / portTICK_PERIOD_MS); -bool unlock(spi_host_device_t device); +/** @return the lock that represents the specified device. Can be used with third party SPI implementations or native API calls (e.g. ESP-IDF). */ +Lockable& getLock(spi_host_device_t device); } // namespace tt::hal::spi diff --git a/TactilityHeadless/Include/Tactility/hal/uart/Uart.h b/TactilityHeadless/Include/Tactility/hal/uart/Uart.h index c891e303..3a0acac0 100644 --- a/TactilityHeadless/Include/Tactility/hal/uart/Uart.h +++ b/TactilityHeadless/Include/Tactility/hal/uart/Uart.h @@ -2,14 +2,17 @@ #include -#include "UartCompat.h" #include "../Gpio.h" +#include "Tactility/Lockable.h" +#include "UartCompat.h" -#include #include +#include namespace tt::hal::uart { +constexpr TickType_t defaultTimeout = 10 / portTICK_PERIOD_MS; + enum class InitMode { ByTactility, // Tactility will initialize it in the correct bootup phase ByExternal, // The device is already initialized and Tactility should assume it works @@ -46,29 +49,69 @@ enum class Status { Unknown }; -bool init(const std::vector& configurations); - +/** Start communications */ bool start(uart_port_t port); + +/** Stop communications */ bool stop(uart_port_t port); + +/** @return true when communications were successfully started */ bool isStarted(uart_port_t port); -bool lock(uart_port_t port, TickType_t timeout = 10 / portTICK_PERIOD_MS); -bool unlock(uart_port_t port); +/** @return a lock that is usable for using ESP-IDF directly, or for use with third party APIs */ +Lockable& getLock(uart_port_t port); -size_t read(uart_port_t port, uint8_t* buffer, size_t bufferSize, TickType_t timeout = 10 / portTICK_PERIOD_MS); -bool readByte(uart_port_t port, uint8_t* output, TickType_t timeout = 10 / portTICK_PERIOD_MS); -size_t write(uart_port_t port, const uint8_t* buffer, size_t bufferSize, TickType_t timeout = 10 / portTICK_PERIOD_MS); -bool writeString(uart_port_t port, const char* buffer, TickType_t timeout = 10 / portTICK_PERIOD_MS); +/** + * Read up to a specified amount of bytes + * @param[in] port + * @param[out] buffer + * @param[in] bufferSize + * @param[in] timeout + * @return the amount of bytes that were read + */ +size_t readBytes(uart_port_t port, uint8_t* buffer, size_t bufferSize, TickType_t timeout = defaultTimeout); -size_t available(uart_port_t port, TickType_t timeout = 10 / portTICK_PERIOD_MS); +/** Read a single byte */ +bool readByte(uart_port_t port, uint8_t* output, TickType_t timeout = defaultTimeout); -bool setBaudRate(uart_port_t port, uint32_t baudRate, TickType_t timeout = 10 / portTICK_PERIOD_MS); +/** + * Read up to a specified amount of bytes + * @param[in] port + * @param[in] buffer + * @param[in] bufferSize + * @param[in] timeout + * @return the amount of bytes that were read + */ +size_t writeBytes(uart_port_t port, const uint8_t* buffer, size_t bufferSize, TickType_t timeout = defaultTimeout); + +/** + * Write a string (excluding null terminator character) + * @param[in] port + * @param[in] buffer + * @param[in] timeout + * @return the amount of bytes that were written + */ +bool writeString(uart_port_t port, const char* buffer, TickType_t timeout = defaultTimeout); + +/** @return the amount of bytes available for reading */ +size_t available(uart_port_t port, TickType_t timeout = defaultTimeout); + +/** Set the baud rate for the specified port */ +bool setBaudRate(uart_port_t port, uint32_t baudRate, TickType_t timeout = defaultTimeout); + +/** Get the baud rate for the specified port */ uint32_t getBaudRate(uart_port_t port); -void flush(uart_port_t port, TickType_t timeout = 10 / portTICK_PERIOD_MS); -void flushInput(uart_port_t port, TickType_t timeout = 10 / portTICK_PERIOD_MS); +/** Flush input buffers */ +void flushInput(uart_port_t port); -std::string readStringUntil(uart_port_t port, char untilChar, TickType_t timeout = 10 / portTICK_PERIOD_MS); -bool readUntil(uart_port_t port, uint8_t* buffer, size_t bufferSize, uint8_t untilByte, TickType_t timeout = 10 / portTICK_PERIOD_MS); +/** + * Read a buffer as a string until the specified character (the "untilChar" is included in the result) + * @warning if the input data doesn't return "untilByte" then the device goes out of memory + */ +std::string readStringUntil(uart_port_t port, char untilChar, TickType_t timeout = defaultTimeout); + +/** Read a buffer as a byte array until the specified character (the "untilChar" is included in the result) */ +bool readUntil(uart_port_t port, uint8_t* buffer, size_t bufferSize, uint8_t untilByte, TickType_t timeout = defaultTimeout); } // namespace tt::hal::uart diff --git a/TactilityHeadless/Private/Tactility/hal/gps/GpsInit.h b/TactilityHeadless/Private/Tactility/hal/gps/GpsInit.h index 6dfa5962..8f57845c 100644 --- a/TactilityHeadless/Private/Tactility/hal/gps/GpsInit.h +++ b/TactilityHeadless/Private/Tactility/hal/gps/GpsInit.h @@ -4,6 +4,11 @@ namespace tt::hal::gps { +/** + * Called by main HAL init to ready the internal state of the GPS HAL. + * @param[in] configurations HAL configuration for a board + * @return true on success + */ bool init(const std::vector& configurations); } diff --git a/TactilityHeadless/Private/Tactility/hal/i2c/I2cInit.h b/TactilityHeadless/Private/Tactility/hal/i2c/I2cInit.h new file mode 100644 index 00000000..717473c6 --- /dev/null +++ b/TactilityHeadless/Private/Tactility/hal/i2c/I2cInit.h @@ -0,0 +1,15 @@ +#pragma once + +#include "Tactility/hal/i2c/I2c.h" +#include + +namespace tt::hal::i2c { + +/** + * Called by main HAL init to ready the internal state of the I2C HAL. + * @param[in] configurations HAL configuration for a board + * @return true on success + */ +bool init(const std::vector& configurations); + +} \ No newline at end of file diff --git a/TactilityHeadless/Private/Tactility/hal/spi/SpiInit.h b/TactilityHeadless/Private/Tactility/hal/spi/SpiInit.h new file mode 100644 index 00000000..6e6856e3 --- /dev/null +++ b/TactilityHeadless/Private/Tactility/hal/spi/SpiInit.h @@ -0,0 +1,17 @@ +#pragma once + +#include "Tactility/hal/spi/Spi.h" + +#include +#include + +namespace tt::hal::spi { + +/** + * Called by main HAL init to ready the internal state of the SPI HAL. + * @param[in] configurations HAL configuration for a board + * @return true on success + */ +bool init(const std::vector& configurations); + +} diff --git a/TactilityHeadless/Private/Tactility/hal/uart/UartInit.h b/TactilityHeadless/Private/Tactility/hal/uart/UartInit.h new file mode 100644 index 00000000..c4dd58d6 --- /dev/null +++ b/TactilityHeadless/Private/Tactility/hal/uart/UartInit.h @@ -0,0 +1,9 @@ +#pragma once + +#include "Tactility/hal/uart/Uart.h" + +namespace tt::hal::uart { + +bool init(const std::vector& configurations); + +} diff --git a/TactilityHeadless/Source/hal/Hal.cpp b/TactilityHeadless/Source/hal/Hal.cpp index 894d497e..82c588a3 100644 --- a/TactilityHeadless/Source/hal/Hal.cpp +++ b/TactilityHeadless/Source/hal/Hal.cpp @@ -1,10 +1,10 @@ #include "Tactility/hal/Configuration.h" #include "Tactility/hal/Device.h" #include "Tactility/hal/gps/GpsInit.h" -#include "Tactility/hal/i2c/I2c.h" +#include "Tactility/hal/i2c/I2cInit.h" #include "Tactility/hal/power/PowerDevice.h" -#include "Tactility/hal/spi/Spi.h" -#include "Tactility/hal/uart/Uart.h" +#include "Tactility/hal/spi/SpiInit.h" +#include "Tactility/hal/uart/UartInit.h" #include diff --git a/TactilityHeadless/Source/hal/gps/GpsDeviceInitDefault.cpp b/TactilityHeadless/Source/hal/gps/GpsDeviceInitDefault.cpp index 1c23900d..bc7ffaab 100644 --- a/TactilityHeadless/Source/hal/gps/GpsDeviceInitDefault.cpp +++ b/TactilityHeadless/Source/hal/gps/GpsDeviceInitDefault.cpp @@ -59,7 +59,7 @@ static int ackGps(uart_port_t port, uint8_t* buffer, uint16_t size, uint8_t requ ubxFrameCounter = 0; break; } - if (uart::read(port, buffer, needRead) != needRead) { + if (uart::readBytes(port, buffer, needRead) != needRead) { ubxFrameCounter = 0; } else { return needRead; @@ -105,7 +105,7 @@ static bool configureGps(uart_port_t port, uint8_t* buffer, size_t bufferSize) { #ifdef ESP_PLATFORM esp_rom_printf("\n"); #endif - uart::flush(port); + uart::flushInput(port); kernel::delayMillis(200); if (!uart::writeString(port, "$PCAS06,0*1B\r\n")) { @@ -144,7 +144,7 @@ static bool recoverGps(uart_port_t port, uint8_t* buffer, size_t bufferSize) { uint8_t cfg_clear2[] = {0xB5, 0x62, 0x06, 0x09, 0x0D, 0x00, 0xFF, 0xFF, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x1B, 0xA1}; uint8_t cfg_clear3[] = {0xB5, 0x62, 0x06, 0x09, 0x0D, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xFF, 0xFF, 0x00, 0x00, 0x03, 0x1D, 0xB3}; - if (!uart::write(port, cfg_clear1, sizeof(cfg_clear1), 10)) { + if (!uart::writeBytes(port, cfg_clear1, sizeof(cfg_clear1), 10)) { return false; TT_LOG_E(TAG, "Failed to send ack 1"); } @@ -155,7 +155,7 @@ static bool recoverGps(uart_port_t port, uint8_t* buffer, size_t bufferSize) { TT_LOG_W(TAG, "Ack 1 failed"); } - if (!uart::write(port, cfg_clear2, sizeof(cfg_clear2))) { + if (!uart::writeBytes(port, cfg_clear2, sizeof(cfg_clear2))) { return false; TT_LOG_E(TAG, "Failed to send ack 2"); } @@ -166,7 +166,7 @@ static bool recoverGps(uart_port_t port, uint8_t* buffer, size_t bufferSize) { TT_LOG_W(TAG, "Ack 2 failed"); } - if (!uart::write(port, cfg_clear3, sizeof(cfg_clear3))) { + if (!uart::writeBytes(port, cfg_clear3, sizeof(cfg_clear3))) { TT_LOG_E(TAG, "Failed to send ack 3"); return false; } @@ -179,7 +179,7 @@ static bool recoverGps(uart_port_t port, uint8_t* buffer, size_t bufferSize) { // UBX-CFG-RATE, Size 8, 'Navigation/measurement rate settings' uint8_t cfg_rate[] = {0xB5, 0x62, 0x06, 0x08, 0x00, 0x00, 0x0E, 0x30}; - uart::write(port, cfg_rate, sizeof(cfg_rate)); + uart::writeBytes(port, cfg_rate, sizeof(cfg_rate)); if (ackGps(port, buffer, bufferSize, 0x06, 0x08)) { TT_LOG_I(TAG, "Ack completed"); } else { diff --git a/TactilityHeadless/Source/hal/i2c/I2c.cpp b/TactilityHeadless/Source/hal/i2c/I2c.cpp index 261a62c0..f7c45783 100644 --- a/TactilityHeadless/Source/hal/i2c/I2c.cpp +++ b/TactilityHeadless/Source/hal/i2c/I2c.cpp @@ -21,7 +21,7 @@ struct Data { static const uint8_t ACK_CHECK_EN = 1; static Data dataArray[I2C_NUM_MAX]; -static const char* initModeToString(InitMode mode) { +static const char* toString(InitMode mode) { switch (mode) { using enum InitMode; case ByTactility: @@ -38,7 +38,7 @@ static void printInfo(const Data& data) { TT_LOG_V(TAG, "I2C info for port %d", data.configuration.port); TT_LOG_V(TAG, " isStarted: %d", data.isStarted); TT_LOG_V(TAG, " isConfigured: %d", data.isConfigured); - TT_LOG_V(TAG, " initMode: %s", initModeToString(data.configuration.initMode)); + TT_LOG_V(TAG, " initMode: %s", toString(data.configuration.initMode)); TT_LOG_V(TAG, " canReinit: %d", data.configuration.canReinit); TT_LOG_V(TAG, " hasMutableConfiguration: %d", data.configuration.hasMutableConfiguration); #ifdef ESP_PLATFORM @@ -75,7 +75,10 @@ bool init(const std::vector& configurations) { return true; } -static bool configureLocked(i2c_port_t port, const i2c_config_t& configuration) { +bool configure(i2c_port_t port, const i2c_config_t& configuration) { + auto lockable = getLock(port).asScopedLock(); + lockable.lock(); + Data& data = dataArray[port]; if (data.isStarted) { TT_LOG_E(TAG, "(%d) Cannot reconfigure while interface is started", port); @@ -89,18 +92,10 @@ static bool configureLocked(i2c_port_t port, const i2c_config_t& configuration) } } -bool configure(i2c_port_t port, const i2c_config_t& configuration) { - if (lock(port)) { - bool result = configureLocked(port, configuration); - unlock(port); - return result; - } else { - TT_LOG_E(TAG, "(%d) Mutex timeout", port); - return false; - } -} +bool start(i2c_port_t port) { + auto lockable = getLock(port).asScopedLock(); + lockable.lock(); -static bool startLocked(i2c_port_t port) { Data& data = dataArray[port]; printInfo(data); Configuration& config = data.configuration; @@ -135,18 +130,10 @@ static bool startLocked(i2c_port_t port) { return true; } -bool start(i2c_port_t port) { - if (lock(port)) { - bool result = startLocked(port); - unlock(port); - return result; - } else { - TT_LOG_E(TAG, "(%d) Mutex timeout", port); - return false; - } -} +bool stop(i2c_port_t port) { + auto lockable = getLock(port).asScopedLock(); + lockable.lock(); -static bool stopLocked(i2c_port_t port) { Data& data = dataArray[port]; Configuration& config = data.configuration; @@ -174,51 +161,36 @@ static bool stopLocked(i2c_port_t port) { return true; } -bool stop(i2c_port_t port) { - if (lock(port)) { - bool result = stopLocked(port); - unlock(port); - return result; - } else { - TT_LOG_E(TAG, "(%d) Mutex timeout", port); - return false; - } -} - bool isStarted(i2c_port_t port) { - if (lock(port, 50 / portTICK_PERIOD_MS)) { - bool started = dataArray[port].isStarted; - unlock(port); - return started; - } else { - // If we can't get a lock, we assume the device is busy and thus has started - return true; - } + auto lockable = getLock(port).asScopedLock(); + lockable.lock(); + return dataArray[port].isStarted; } bool masterRead(i2c_port_t port, uint8_t address, uint8_t* data, size_t dataSize, TickType_t timeout) { -#ifdef ESP_PLATFORM - if (lock(port)) { - // TODO: We're passing an inaccurate timeout value as we already lost time with locking and previous writes in this loop - esp_err_t result = i2c_master_read_from_device(port, address, data, dataSize, timeout); - unlock(port); - return result == ESP_OK; - } else { + auto lockable = getLock(port).asScopedLock(); + if (!lockable.lock(timeout)) { TT_LOG_E(TAG, "(%d) Mutex timeout", port); return false; } + +#ifdef ESP_PLATFORM + auto result = i2c_master_read_from_device(port, address, data, dataSize, timeout); + ESP_ERROR_CHECK_WITHOUT_ABORT(result); + return result == ESP_OK; #else return false; #endif // ESP_PLATFORM } bool masterReadRegister(i2c_port_t port, uint8_t address, uint8_t reg, uint8_t* data, size_t dataSize, TickType_t timeout) { -#ifdef ESP_PLATFORM - if (!lock(port)) { + auto lockable = getLock(port).asScopedLock(); + if (!lockable.lock(timeout)) { TT_LOG_E(TAG, "(%d) Mutex timeout", port); return false; } +#ifdef ESP_PLATFORM i2c_cmd_handle_t cmd = i2c_cmd_link_create(); // Set address pointer i2c_master_start(cmd); @@ -236,9 +208,6 @@ bool masterReadRegister(i2c_port_t port, uint8_t address, uint8_t reg, uint8_t* esp_err_t result = i2c_master_cmd_begin(port, cmd, timeout); i2c_cmd_link_delete(cmd); - unlock(port); - - ESP_LOG_BUFFER_HEX_LEVEL(TAG, data, dataSize, ESP_LOG_DEBUG); ESP_ERROR_CHECK_WITHOUT_ABORT(result); return result == ESP_OK; @@ -248,30 +217,32 @@ bool masterReadRegister(i2c_port_t port, uint8_t address, uint8_t reg, uint8_t* } bool masterWrite(i2c_port_t port, uint8_t address, const uint8_t* data, uint16_t dataSize, TickType_t timeout) { -#ifdef ESP_PLATFORM - if (lock(port)) { - // TODO: We're passing an inaccurate timeout value as we already lost time with locking - esp_err_t result = i2c_master_write_to_device(port, address, data, dataSize, timeout); - unlock(port); - return result == ESP_OK; - } else { + auto lockable = getLock(port).asScopedLock(); + if (!lockable.lock(timeout)) { TT_LOG_E(TAG, "(%d) Mutex timeout", port); return false; } + +#ifdef ESP_PLATFORM + auto result = i2c_master_write_to_device(port, address, data, dataSize, timeout); + ESP_ERROR_CHECK_WITHOUT_ABORT(result); + return result == ESP_OK; #else return false; #endif // ESP_PLATFORM } bool masterWriteRegister(i2c_port_t port, uint8_t address, uint8_t reg, const uint8_t* data, uint16_t dataSize, TickType_t timeout) { -#ifdef ESP_PLATFORM tt_check(reg != 0); - if (!lock(port)) { + auto lockable = getLock(port).asScopedLock(); + if (!lockable.lock(timeout)) { TT_LOG_E(TAG, "(%d) Mutex timeout", port); return false; } +#ifdef ESP_PLATFORM + 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); @@ -282,10 +253,7 @@ bool masterWriteRegister(i2c_port_t port, uint8_t address, uint8_t reg, const ui esp_err_t result = i2c_master_cmd_begin(port, cmd, timeout); i2c_cmd_link_delete(cmd); - unlock(port); - ESP_ERROR_CHECK_WITHOUT_ABORT(result); - return result == ESP_OK; #else return false; @@ -309,44 +277,39 @@ bool masterWriteRegisterArray(i2c_port_t port, uint8_t address, const uint8_t* d } bool masterWriteRead(i2c_port_t port, uint8_t address, const uint8_t* writeData, size_t writeDataSize, uint8_t* readData, size_t readDataSize, TickType_t timeout) { -#ifdef ESP_PLATFORM - if (lock(port)) { - // TODO: We're passing an inaccurate timeout value as we already lost time with locking - esp_err_t result = i2c_master_write_read_device(port, address, writeData, writeDataSize, readData, readDataSize, timeout); - unlock(port); - return result == ESP_OK; - } else { + auto lockable = getLock(port).asScopedLock(); + if (!lockable.lock(timeout)) { TT_LOG_E(TAG, "(%d) Mutex timeout", port); return false; } + +#ifdef ESP_PLATFORM + esp_err_t result = i2c_master_write_read_device(port, address, writeData, writeDataSize, readData, readDataSize, timeout); + ESP_ERROR_CHECK_WITHOUT_ABORT(result); + return result == ESP_OK; #else return false; #endif // ESP_PLATFORM } bool masterHasDeviceAtAddress(i2c_port_t port, uint8_t address, TickType_t timeout) { -#ifdef ESP_PLATFORM - if (lock(port)) { - uint8_t message[2] = { 0, 0 }; - // TODO: We're passing an inaccurate timeout value as we already lost time with locking - esp_err_t result = i2c_master_write_to_device(port, address, message, 2, timeout); - unlock(port); - return result == ESP_OK; - } else { + auto lockable = getLock(port).asScopedLock(); + if (!lockable.lock(timeout)) { TT_LOG_E(TAG, "(%d) Mutex timeout", port); return false; } + +#ifdef ESP_PLATFORM + uint8_t message[2] = { 0, 0 }; + // TODO: We're passing an inaccurate timeout value as we already lost time with locking + return i2c_master_write_to_device(port, address, message, 2, timeout) == ESP_OK; #else return false; #endif // ESP_PLATFORM } -bool lock(i2c_port_t port, TickType_t timeout) { - return dataArray[port].mutex.lock(timeout); -} - -bool unlock(i2c_port_t port) { - return dataArray[port].mutex.unlock(); +Lockable& getLock(i2c_port_t port) { + return dataArray[port].mutex; } } // namespace diff --git a/TactilityHeadless/Source/hal/spi/Spi.cpp b/TactilityHeadless/Source/hal/spi/Spi.cpp index 1d42bb2e..29ef3318 100644 --- a/TactilityHeadless/Source/hal/spi/Spi.cpp +++ b/TactilityHeadless/Source/hal/spi/Spi.cpp @@ -15,7 +15,7 @@ struct Data { static Data dataArray[SPI_HOST_MAX]; -static const char* initModeToString(InitMode mode) { +static const char* toString(InitMode mode) { switch (mode) { using enum InitMode; case ByTactility: @@ -32,7 +32,7 @@ static void printInfo(const Data& data) { TT_LOG_V(TAG, "SPI info for device %d", data.configuration.device); TT_LOG_V(TAG, " isStarted: %d", data.isStarted); TT_LOG_V(TAG, " isConfigured: %d", data.isConfigured); - TT_LOG_V(TAG, " initMode: %s", initModeToString(data.configuration.initMode)); + TT_LOG_V(TAG, " initMode: %s", toString(data.configuration.initMode)); TT_LOG_V(TAG, " canReinit: %d", data.configuration.canReinit); TT_LOG_V(TAG, " hasMutableConfiguration: %d", data.configuration.hasMutableConfiguration); TT_LOG_V(TAG, " MISO pin: %d", data.configuration.config.miso_io_num); @@ -67,7 +67,10 @@ bool init(const std::vector& configurations) { return true; } -static bool configureLocked(spi_host_device_t device, const spi_bus_config_t& configuration) { +bool configure(spi_host_device_t device, const spi_bus_config_t& configuration) { + auto lock = getLock(device).asScopedLock(); + lock.lock(); + Data& data = dataArray[device]; if (data.isStarted) { TT_LOG_E(TAG, "(%d) Cannot reconfigure while interface is started", device); @@ -81,18 +84,10 @@ static bool configureLocked(spi_host_device_t device, const spi_bus_config_t& co } } -bool configure(spi_host_device_t device, const spi_bus_config_t& configuration) { - if (lock(device)) { - bool result = configureLocked(device, configuration); - unlock(device); - return result; - } else { - TT_LOG_E(TAG, "(%d) Mutex timeout", device); - return false; - } -} +bool start(spi_host_device_t device) { + auto lock = getLock(device).asScopedLock(); + lock.lock(); -static bool startLocked(spi_host_device_t device) { Data& data = dataArray[device]; printInfo(data); @@ -106,7 +101,7 @@ static bool startLocked(spi_host_device_t device) { return false; } - #ifdef ESP_PLATFORM +#ifdef ESP_PLATFORM Configuration& config = data.configuration; auto result = spi_bus_initialize(device, &data.configuration.config, data.configuration.dma); @@ -117,28 +112,20 @@ static bool startLocked(spi_host_device_t device) { data.isStarted = true; } - #else +#else data.isStarted = true; - #endif +#endif TT_LOG_I(TAG, "(%d) Started", device); return true; } -bool start(spi_host_device_t device) { - if (lock(device)) { - bool result = startLocked(device); - unlock(device); - return result; - } else { - TT_LOG_E(TAG, "(%d) Mutex timeout", device); - return false; - } -} +bool stop(spi_host_device_t device) { + auto lock = getLock(device).asScopedLock(); + lock.lock(); -static bool stopLocked(spi_host_device_t device) { Data& data = dataArray[device]; Configuration& config = data.configuration; @@ -152,7 +139,7 @@ static bool stopLocked(spi_host_device_t device) { return false; } - #ifdef ESP_PLATFORM +#ifdef ESP_PLATFORM auto result = spi_bus_free(device); if (result != ESP_OK) { @@ -162,44 +149,28 @@ static bool stopLocked(spi_host_device_t device) { data.isStarted = false; } - #else +#else data.isStarted = false; - #endif +#endif TT_LOG_I(TAG, "(%d) Stopped", device); return true; } -bool stop(spi_host_device_t device) { - if (lock(device)) { - bool result = stopLocked(device); - unlock(device); - return result; - } else { - TT_LOG_E(TAG, "(%d) Mutex timeout", device); - return false; - } -} - bool isStarted(spi_host_device_t device) { - if (lock(device, 50 / portTICK_PERIOD_MS)) { - bool started = dataArray[device].isStarted; - unlock(device); - return started; - } else { - // If we can't get a lock, we assume the device is busy and thus has started - return true; - } + auto lock = getLock(device).asScopedLock(); + lock.lock(); + + Data& data = dataArray[device]; + Configuration& config = data.configuration; + + return dataArray[device].isStarted; } -bool lock(spi_host_device_t device, TickType_t timeout) { - return dataArray[device].lock->lock(timeout); -} - -bool unlock(spi_host_device_t device) { - return dataArray[device].lock->unlock(); +Lockable& getLock(spi_host_device_t device) { + return *dataArray[device].lock; } } diff --git a/TactilityHeadless/Source/hal/uart/Uart.cpp b/TactilityHeadless/Source/hal/uart/Uart.cpp index ef56940a..98df6c27 100644 --- a/TactilityHeadless/Source/hal/uart/Uart.cpp +++ b/TactilityHeadless/Source/hal/uart/Uart.cpp @@ -71,7 +71,10 @@ bool init(const std::vector& configurations) { return true; } -static bool configureLocked(uart_port_t port, const uart_config_t& configuration) { +bool configure(uart_port_t port, const uart_config_t& configuration) { + auto lock = getLock(port).asScopedLock(); + lock.lock(); + Data& data = dataArray[port]; if (data.isStarted) { TT_LOG_E(TAG, "(%d) Cannot reconfigure while interface is started", port); @@ -85,18 +88,10 @@ static bool configureLocked(uart_port_t port, const uart_config_t& configuration } } -bool configure(uart_port_t port, const uart_config_t& configuration) { - if (lock(port)) { - bool result = configureLocked(port, configuration); - unlock(port); - return result; - } else { - TT_LOG_E(TAG, "(%d) Mutex timeout", port); - return false; - } -} +bool start(uart_port_t port) { + auto lock = getLock(port).asScopedLock(); + lock.lock(); -static bool startLocked(uart_port_t port) { Data& data = dataArray[port]; printInfo(data); @@ -147,18 +142,10 @@ static bool startLocked(uart_port_t port) { return true; } -bool start(uart_port_t port) { - if (lock(port)) { - bool result = startLocked(port); - unlock(port); - return result; - } else { - TT_LOG_E(TAG, "(%d) Mutex timeout", port); - return false; - } -} +bool stop(uart_port_t port) { + auto lock = getLock(port).asScopedLock(); + lock.lock(); -static bool stopLocked(uart_port_t port) { Data& data = dataArray[port]; Configuration& config = data.configuration; @@ -186,72 +173,54 @@ static bool stopLocked(uart_port_t port) { return true; } -bool stop(uart_port_t port) { - if (lock(port)) { - bool result = stopLocked(port); - unlock(port); - return result; - } else { +bool isStarted(uart_port_t port) { + auto lock = getLock(port).asScopedLock(); + lock.lock(); + + return dataArray[port].isStarted; +} + +Lockable& getLock(uart_port_t port) { + return dataArray[port].mutex; +} + +size_t readBytes(uart_port_t port, uint8_t* buffer, size_t bufferSize, TickType_t timeout) { + auto lock = getLock(port).asScopedLock(); + if (!lock.lock(timeout)) { TT_LOG_E(TAG, "(%d) Mutex timeout", port); return false; } -} -bool isStarted(uart_port_t port) { - if (lock(port, 50 / portTICK_PERIOD_MS)) { - bool started = dataArray[port].isStarted; - unlock(port); - return started; - } else { - // If we can't get a lock, we assume the device is busy and thus has started - return true; - } -} - -bool lock(uart_port_t port, TickType_t timeout) { - return dataArray[port].mutex.lock(timeout); -} - -bool unlock(uart_port_t port) { - return dataArray[port].mutex.unlock(); -} - -size_t read(uart_port_t port, uint8_t* buffer, size_t bufferSize, TickType_t timeout) { #ifdef ESP_PLATFORM auto start_time = kernel::getTicks(); - if (lock(port, timeout)) { - auto lock_time = kernel::getTicks() - start_time; - auto remaining_timeout = std::max(timeout - lock_time, 0UL); - auto result = uart_read_bytes(port, buffer, bufferSize, remaining_timeout); - unlock(port); - return result; - } else { - TT_LOG_E(TAG, LOG_MESSAGE_MUTEX_LOCK_FAILED_FMT, "read()"); - } + auto lock_time = kernel::getTicks() - start_time; + auto remaining_timeout = std::max(timeout - lock_time, 0UL); + auto result = uart_read_bytes(port, buffer, bufferSize, remaining_timeout); + return result; #endif // ESP_PLATFORM return 0; } bool readByte(uart_port_t port, uint8_t* output, TickType_t timeout) { - return read(port, output, 1, timeout) == 1; + return readBytes(port, output, 1, timeout) == 1; } -size_t write(uart_port_t port, const uint8_t* buffer, size_t bufferSize, TickType_t timeout) { -#ifdef ESP_PLATFORM - if (lock(port, timeout)) { - auto result = uart_write_bytes(port, buffer, bufferSize); - unlock(port); - return result; - } else { - TT_LOG_E(TAG, LOG_MESSAGE_MUTEX_LOCK_FAILED_FMT, "write()"); +size_t writeBytes(uart_port_t port, const uint8_t* buffer, size_t bufferSize, TickType_t timeout) { + auto lock = getLock(port).asScopedLock(); + if (!lock.lock(timeout)) { + TT_LOG_E(TAG, "(%d) Mutex timeout", port); + return false; } + +#ifdef ESP_PLATFORM + return uart_write_bytes(port, buffer, bufferSize); #endif // ESP_PLATFORM return 0; } bool writeString(uart_port_t port, const char* buffer, TickType_t timeout) { while (*buffer != 0) { - if (write(port, (const uint8_t*)buffer, 1, timeout)) { + if (writeBytes(port, (const uint8_t*)buffer, 1, timeout)) { buffer++; } else { TT_LOG_E(TAG, "Failed to write - breaking off"); @@ -263,47 +232,38 @@ bool writeString(uart_port_t port, const char* buffer, TickType_t timeout) { } size_t available(uart_port_t port, TickType_t timeout) { + auto lock = getLock(port).asScopedLock(); + if (!lock.lock(timeout)) { + TT_LOG_E(TAG, "(%d) Mutex timeout", port); + return false; + } + #ifdef ESP_PLATFORM size_t size = 0; - if (lock(port, timeout)) { - uart_get_buffered_data_len(port, &size); - unlock(port); - return size; - } else { - TT_LOG_E(TAG, LOG_MESSAGE_MUTEX_LOCK_FAILED_FMT, "write()"); - } -#endif // ESP_PLATFORM + uart_get_buffered_data_len(port, &size); + return size; +#else return 0; -} - -void flush(uart_port_t port, TickType_t timeout) { -#ifdef ESP_PLATFORM - size_t size = 0; - if (lock(port, timeout)) { - uart_flush(port); - unlock(port); - } else { - TT_LOG_E(TAG, LOG_MESSAGE_MUTEX_LOCK_FAILED_FMT, "write()"); - } #endif // ESP_PLATFORM } -void flushInput(uart_port_t port, TickType_t timeout) { +void flush(uart_port_t port) { #ifdef ESP_PLATFORM - size_t size = 0; - if (lock(port, timeout)) { - uart_flush_input(port); - unlock(port); - } else { - TT_LOG_E(TAG, LOG_MESSAGE_MUTEX_LOCK_FAILED_FMT, "write()"); - } + uart_flush(port); +#endif // ESP_PLATFORM +} + +void flushInput(uart_port_t port) { +#ifdef ESP_PLATFORM + uart_flush_input(port); #endif // ESP_PLATFORM } uint32_t getBaudRate(uart_port_t port) { #ifdef ESP_PLATFORM uint32_t baud_rate = 0; - uart_get_baudrate(port, &baud_rate); + auto result = uart_get_baudrate(port, &baud_rate); + ESP_ERROR_CHECK_WITHOUT_ABORT(result); return baud_rate; #else return 0; @@ -311,15 +271,19 @@ uint32_t getBaudRate(uart_port_t port) { } bool setBaudRate(uart_port_t port, uint32_t baudRate, TickType_t timeout) { -#ifdef ESP_PLATFORM - if (lock(port, timeout)) { - uart_set_baudrate(port, baudRate); - unlock(port); - } else { - TT_LOG_E(TAG, LOG_MESSAGE_MUTEX_LOCK_FAILED_FMT, "write()"); + auto lock = getLock(port).asScopedLock(); + if (!lock.lock(timeout)) { + TT_LOG_E(TAG, "(%d) Mutex timeout", port); + return false; } -#endif // ESP_PLATFORM + +#ifdef ESP_PLATFORM + auto result = uart_set_baudrate(port, baudRate); + ESP_ERROR_CHECK_WITHOUT_ABORT(result); + return result == ESP_OK; +#else return true; +#endif // ESP_PLATFORM } bool readUntil(uart_port_t port, uint8_t* buffer, size_t bufferSize, uint8_t untilByte, TickType_t timeout) { @@ -330,9 +294,8 @@ bool readUntil(uart_port_t port, uint8_t* buffer, size_t bufferSize, uint8_t unt if (*buffer == untilByte) { success = true; // We have the extra space because index < index_limit - if (buffer++) { - *buffer = 0; - } + buffer++; + *buffer = 0; break; } buffer++;