Added HAL docs, improved HAL init&locking (#218)

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();
 }
 
 }

TactilityCore/Include/Tactility/Lockable.h

@@ -43,7 +43,10 @@ public:
 
     void withLock(const std::function<void()>& onLockAcquired, const std::function<void()>& onLockFailed) const { withLock(portMAX_DELAY, onLockAcquired, onLockFailed); }
 
+    [[deprecated("use asScopedLock()")]]
     std::unique_ptr<ScopedLockableUsage> scoped() const;
+
+    ScopedLockableUsage asScopedLock() const;
 };
 
 

TactilityCore/Source/Lockable.cpp

@@ -6,4 +6,8 @@ std::unique_ptr<ScopedLockableUsage> Lockable::scoped() const {
     return std::make_unique<ScopedLockableUsage>(*this);
 }
 
+ScopedLockableUsage Lockable::asScopedLock() const {
+    return ScopedLockableUsage(*this);
+}
+
 }

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. */

TactilityHeadless/Include/Tactility/hal/i2c/I2c.h

@@ -1,15 +1,17 @@
 #pragma once
 
 #include "./I2cCompat.h"
+#include "Tactility/Lockable.h"
 
 #include <Tactility/RtosCompat.h>
 
 #include <climits>
 #include <string>
-#include <vector>
 
 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<i2c::Configuration>& 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);
+/** Read bytes in master mode. */
-bool masterReadRegister(i2c_port_t port, uint8_t address, uint8_t reg, uint8_t* data, size_t dataSize, TickType_t timeout);
+bool masterRead(i2c_port_t port, uint8_t address, uint8_t* data, size_t dataSize, TickType_t timeout = defaultTimeout);
-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);
 
-bool lock(i2c_port_t port, TickType_t timeout = 10 / portTICK_PERIOD_MS);
+/** Read bytes from the specified register in master mode. */
-bool unlock(i2c_port_t port);
+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

TactilityHeadless/Include/Tactility/hal/spi/Spi.h

@@ -5,11 +5,10 @@
 #include <Tactility/Lockable.h>
 #include <Tactility/RtosCompat.h>
 
-#include <vector>
-#include <memory>
-
 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<spi::Configuration>& 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);
+/** @return the lock that represents the specified device. Can be used with third party SPI implementations or native API calls (e.g. ESP-IDF). */
-bool unlock(spi_host_device_t device);
+Lockable& getLock(spi_host_device_t device);
 
 } // namespace tt::hal::spi

TactilityHeadless/Include/Tactility/hal/uart/Uart.h

@@ -2,14 +2,17 @@
 
 #include <Tactility/RtosCompat.h>
 
-#include "UartCompat.h"
 #include "../Gpio.h"
+#include "Tactility/Lockable.h"
+#include "UartCompat.h"
 
-#include <vector>
 #include <memory>
+#include <vector>
 
 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<uart::Configuration>& 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);
+/** @return a lock that is usable for using ESP-IDF directly, or for use with third party APIs */
-bool unlock(uart_port_t port);
+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);
+ * Read up to a specified amount of bytes
-size_t write(uart_port_t port, const uint8_t* buffer, size_t bufferSize, TickType_t timeout = 10 / portTICK_PERIOD_MS);
+ * @param[in] port
-bool writeString(uart_port_t port, const char* buffer, TickType_t timeout = 10 / portTICK_PERIOD_MS);
+ * @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);
+/** Flush input buffers */
-void flushInput(uart_port_t port, TickType_t timeout = 10 / portTICK_PERIOD_MS);
+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

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<GpsDevice::Configuration>& configurations);
 
 }

TactilityHeadless/Private/Tactility/hal/i2c/I2cInit.h

@@ -0,0 +1,15 @@
+#pragma once
+
+#include "Tactility/hal/i2c/I2c.h"
+#include <vector>
+
+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<i2c::Configuration>& configurations);
+
+}

TactilityHeadless/Private/Tactility/hal/spi/SpiInit.h

@@ -0,0 +1,17 @@
+#pragma once
+
+#include "Tactility/hal/spi/Spi.h"
+
+#include <vector>
+#include <memory>
+
+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<spi::Configuration>& configurations);
+
+}

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<uart::Configuration>& configurations);
+
+}

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/spi/SpiInit.h"
-#include "Tactility/hal/uart/Uart.h"
+#include "Tactility/hal/uart/UartInit.h"
 
 #include <Tactility/kernel/SystemEvents.h>
 

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 {

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<i2c::Configuration>& 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) {
+bool start(i2c_port_t port) {
-    if (lock(port)) {
+    auto lockable = getLock(port).asScopedLock();
-        bool result = configureLocked(port, configuration);
+    lockable.lock();
-        unlock(port);
-        return result;
-    } else {
-        TT_LOG_E(TAG, "(%d) Mutex timeout", port);
-        return false;
-    }
-}
 
-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) {
+bool stop(i2c_port_t port) {
-    if (lock(port)) {
+    auto lockable = getLock(port).asScopedLock();
-        bool result = startLocked(port);
+    lockable.lock();
-        unlock(port);
-        return result;
-    } else {
-        TT_LOG_E(TAG, "(%d) Mutex timeout", port);
-        return false;
-    }
-}
 
-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)) {
+    auto lockable = getLock(port).asScopedLock();
-        bool started = dataArray[port].isStarted;
+    lockable.lock();
-        unlock(port);
+    return dataArray[port].isStarted;
-        return started;
-    } else {
-        // If we can't get a lock, we assume the device is busy and thus has started
-        return true;
-    }
 }
 
 bool masterRead(i2c_port_t port, uint8_t address, uint8_t* data, size_t dataSize, TickType_t timeout) {
-#ifdef ESP_PLATFORM
+    auto lockable = getLock(port).asScopedLock();
-    if (lock(port)) {
+    if (!lockable.lock(timeout)) {
-        // 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 {
         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
+    auto lockable = getLock(port).asScopedLock();
-    if (!lock(port)) {
+    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
+    auto lockable = getLock(port).asScopedLock();
-    if (lock(port)) {
+    if (!lockable.lock(timeout)) {
-        // 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 {
         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
+    auto lockable = getLock(port).asScopedLock();
-    if (lock(port)) {
+    if (!lockable.lock(timeout)) {
-        // 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 {
         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
+    auto lockable = getLock(port).asScopedLock();
-    if (lock(port)) {
+    if (!lockable.lock(timeout)) {
-        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 {
         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) {
+Lockable& getLock(i2c_port_t port) {
-    return dataArray[port].mutex.lock(timeout);
+    return dataArray[port].mutex;
-}
-
-bool unlock(i2c_port_t port) {
-    return dataArray[port].mutex.unlock();
 }
 
 } // namespace

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<spi::Configuration>& 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) {
+bool start(spi_host_device_t device) {
-    if (lock(device)) {
+    auto lock = getLock(device).asScopedLock();
-        bool result = configureLocked(device, configuration);
+    lock.lock();
-        unlock(device);
-        return result;
-    } else {
-        TT_LOG_E(TAG, "(%d) Mutex timeout", device);
-        return false;
-    }
-}
 
-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) {
+bool stop(spi_host_device_t device) {
-    if (lock(device)) {
+    auto lock = getLock(device).asScopedLock();
-        bool result = startLocked(device);
+    lock.lock();
-        unlock(device);
-        return result;
-    } else {
-        TT_LOG_E(TAG, "(%d) Mutex timeout", device);
-        return false;
-    }
-}
 
-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)) {
+    auto lock = getLock(device).asScopedLock();
-        bool started = dataArray[device].isStarted;
+    lock.lock();
-        unlock(device);
+
-        return started;
+    Data& data = dataArray[device];
-    } else {
+    Configuration& config = data.configuration;
-        // If we can't get a lock, we assume the device is busy and thus has started
+
-        return true;
+    return dataArray[device].isStarted;
-    }
 }
 
-bool lock(spi_host_device_t device, TickType_t timeout) {
+Lockable& getLock(spi_host_device_t device) {
-    return dataArray[device].lock->lock(timeout);
+    return *dataArray[device].lock;
-}
-
-bool unlock(spi_host_device_t device) {
-    return dataArray[device].lock->unlock();
 }
 
 }

TactilityHeadless/Source/hal/uart/Uart.cpp

@@ -71,7 +71,10 @@ bool init(const std::vector<uart::Configuration>& 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) {
+bool start(uart_port_t port) {
-    if (lock(port)) {
+    auto lock = getLock(port).asScopedLock();
-        bool result = configureLocked(port, configuration);
+    lock.lock();
-        unlock(port);
-        return result;
-    } else {
-        TT_LOG_E(TAG, "(%d) Mutex timeout", port);
-        return false;
-    }
-}
 
-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) {
+bool stop(uart_port_t port) {
-    if (lock(port)) {
+    auto lock = getLock(port).asScopedLock();
-        bool result = startLocked(port);
+    lock.lock();
-        unlock(port);
-        return result;
-    } else {
-        TT_LOG_E(TAG, "(%d) Mutex timeout", port);
-        return false;
-    }
-}
 
-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) {
+bool isStarted(uart_port_t port) {
-    if (lock(port)) {
+    auto lock = getLock(port).asScopedLock();
-        bool result = stopLocked(port);
+    lock.lock();
-        unlock(port);
+
-        return result;
+    return dataArray[port].isStarted;
-    } else {
+}
+
+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 lock_time = kernel::getTicks() - start_time;
+    auto remaining_timeout = std::max(timeout - lock_time, 0UL);
-        auto remaining_timeout = std::max(timeout - lock_time, 0UL);
+    auto result = uart_read_bytes(port, buffer, bufferSize, remaining_timeout);
-        auto result = uart_read_bytes(port, buffer, bufferSize, remaining_timeout);
+    return result;
-        unlock(port);
-        return result;
-    } else {
-        TT_LOG_E(TAG, LOG_MESSAGE_MUTEX_LOCK_FAILED_FMT, "read()");
-    }
 #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) {
+size_t writeBytes(uart_port_t port, const uint8_t* buffer, size_t bufferSize, TickType_t timeout) {
-#ifdef ESP_PLATFORM
+    auto lock = getLock(port).asScopedLock();
-    if (lock(port, timeout)) {
+    if (!lock.lock(timeout)) {
-        auto result = uart_write_bytes(port, buffer, bufferSize);
+        TT_LOG_E(TAG, "(%d) Mutex timeout", port);
-        unlock(port);
+        return false;
-        return result;
-    } else {
-        TT_LOG_E(TAG, LOG_MESSAGE_MUTEX_LOCK_FAILED_FMT, "write()");
     }
+
+#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);
-        uart_get_buffered_data_len(port, &size);
+    return size;
-        unlock(port);
+#else
-        return size;
-    } else {
-        TT_LOG_E(TAG, LOG_MESSAGE_MUTEX_LOCK_FAILED_FMT, "write()");
-    }
-#endif // ESP_PLATFORM
     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;
+    uart_flush(port);
-    if (lock(port, timeout)) {
+#endif // ESP_PLATFORM
-        uart_flush_input(port);
+}
-        unlock(port);
+
-    } else {
+void flushInput(uart_port_t port) {
-        TT_LOG_E(TAG, LOG_MESSAGE_MUTEX_LOCK_FAILED_FMT, "write()");
+#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
+    auto lock = getLock(port).asScopedLock();
-    if (lock(port, timeout)) {
+    if (!lock.lock(timeout)) {
-        uart_set_baudrate(port, baudRate);
+        TT_LOG_E(TAG, "(%d) Mutex timeout", port);
-        unlock(port);
+        return false;
-    } else {
-        TT_LOG_E(TAG, LOG_MESSAGE_MUTEX_LOCK_FAILED_FMT, "write()");
     }
-#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++;
-                *buffer = 0;
+            *buffer = 0;
-            }
             break;
         }
         buffer++;