Cleanup and improvements (#194)

- Lots of changes for migrating C code to C++
- Improved `Lockable` in several ways like adding `withLock()` (+ tests)
- Improved `Semaphore` a bit for improved readability, and also added some tests
- Upgrade Linux machine in GitHub Actions so that we can compile with a newer GCC
- Simplification of WiFi connection
- Updated funding options
- (and more)

TactilityCore/Source/CoreExtraDefines.h

@@ -1,37 +1,3 @@
 #pragma once
 
-/** Find the largest value
- * @param[in] a first value to compare
- * @param[in] b second value to compare
- * @return the largest value of a and b
- */
-#define TT_MAX(a, b)            \
-    ({                          \
-        __typeof__(a) _a = (a); \
-        __typeof__(b) _b = (b); \
-        _a > _b ? _a : _b;      \
-    })
-
-/** Find the smallest value
- * @param[in] a first value to compare
- * @param[in] b second value to compare
- * @return the smallest value of a and b
- */
-#define TT_MIN(a, b)            \
-    ({                          \
-        __typeof__(a) _a = (a); \
-        __typeof__(b) _b = (b); \
-        _a < _b ? _a : _b;      \
-    })
-
-/** @return the absolute value of the input */
-#define TT_ABS(a) ({ (a) < 0 ? -(a) : (a); })
-
-/** Clamp a value between a min and a max.
- * @param[in] x value to clamp
- * @param[in] upper upper bounds for x
- * @param[in] lower lower bounds for x
- */
-#define TT_CLAMP(x, upper, lower) (TT_MIN(upper, TT_MAX(x, lower)))
-
 #define TT_STRINGIFY(x) #x

TactilityCore/Source/Dispatcher.h

@@ -13,7 +13,6 @@
 
 namespace tt {
 
-
 /**
  * A thread-safe way to defer code execution.
  * Generally, one task would dispatch the execution,

TactilityCore/Source/Lockable.h

@@ -12,18 +12,41 @@ class ScopedLockableUsage;
 
 /** Represents a lock/mutex */
 class Lockable {
+
 public:
+
     virtual ~Lockable() = default;
 
-    virtual bool lock(TickType_t timeoutTicks) const = 0;
+    virtual bool lock(TickType_t timeout) const = 0;
 
-    virtual bool lock() const { return lock(portMAX_DELAY); }
+    bool lock() const { return lock(portMAX_DELAY); }
 
     virtual bool unlock() const = 0;
 
+    void withLock(TickType_t timeout, const std::function<void()>& onLockAcquired) const {
+        if (lock(timeout)) {
+            onLockAcquired();
+            unlock();
+        }
+    }
+
+    void withLock(TickType_t timeout, const std::function<void()>& onLockAcquired, const std::function<void()>& onLockFailure) const {
+        if (lock(timeout)) {
+            onLockAcquired();
+            unlock();
+        } else {
+            onLockFailure();
+        }
+    }
+
+    void withLock(const std::function<void()>& onLockAcquired) const { withLock(portMAX_DELAY, onLockAcquired); }
+
+    void withLock(const std::function<void()>& onLockAcquired, const std::function<void()>& onLockFailed) const { withLock(portMAX_DELAY, onLockAcquired, onLockFailed); }
+
     std::unique_ptr<ScopedLockableUsage> scoped() const;
 };
 
+
 /**
  * Represents a lockable instance that is scoped to a specific lifecycle.
  * Once the ScopedLockableUsage is destroyed, unlock() is called automatically.
@@ -37,6 +60,8 @@ class ScopedLockableUsage final : public Lockable {
 
 public:
 
+    using Lockable::lock;
+
     explicit ScopedLockableUsage(const Lockable& lockable) : lockable(lockable) {}
 
     ~ScopedLockableUsage() final {
@@ -47,8 +72,6 @@ public:
         return lockable.lock(timeout);
     }
 
-    bool lock() const override { return lock(portMAX_DELAY); }
-
     bool unlock() const override {
         return lockable.unlock();
     }

TactilityCore/Source/Log.cpp

@@ -4,8 +4,8 @@
 
 namespace tt {
 
-static LogEntry* logEntries = nullptr;
-static unsigned int nextLogEntryIndex;
+static std::array<LogEntry, TT_LOG_ENTRY_COUNT> logEntries;
+static size_t nextLogEntryIndex;
 
 /**
  * This used to be a simple static value, but that crashes on device boot where early logging happens.
@@ -20,18 +20,8 @@ Mutex& getLogMutex() {
     return *logMutex;
 }
 
-static void ensureLogEntriesExist() {
-    if (logEntries == nullptr) {
-        logEntries = new LogEntry[TT_LOG_ENTRY_COUNT];
-        assert(logEntries != nullptr);
-        nextLogEntryIndex = 0;
-    }
-}
-
 static void storeLog(LogLevel level, const char* format, va_list args) {
     if (getLogMutex().lock(5 / portTICK_PERIOD_MS)) {
-        ensureLogEntriesExist();
-
         logEntries[nextLogEntryIndex].level = level;
         vsnprintf(logEntries[nextLogEntryIndex].message, TT_LOG_MESSAGE_SIZE, format, args);
 
@@ -44,16 +34,12 @@ static void storeLog(LogLevel level, const char* format, va_list args) {
     }
 }
 
-LogEntry* copyLogEntries(unsigned int& outIndex) {
+std::unique_ptr<std::array<LogEntry, TT_LOG_ENTRY_COUNT>> copyLogEntries(std::size_t& outIndex) {
     if (getLogMutex().lock(5 / portTICK_PERIOD_MS)) {
-        auto* newEntries = new LogEntry[TT_LOG_ENTRY_COUNT];
-        assert(newEntries != nullptr);
-        for (int i = 0; i < TT_LOG_ENTRY_COUNT; ++i) {
-            memcpy(&newEntries[i], &logEntries[i], sizeof(LogEntry));
-        }
-        outIndex = nextLogEntryIndex;
+        auto copy = std::make_unique<std::array<LogEntry, TT_LOG_ENTRY_COUNT>>(logEntries);
         getLogMutex().unlock();
-        return newEntries;
+        outIndex = nextLogEntryIndex;
+        return copy;
     } else {
         return nullptr;
     }

TactilityCore/Source/Log.h

@@ -1,6 +1,8 @@
 #pragma once
 
 #include "LogMessages.h"
+#include <array>
+#include <memory>
 
 #ifdef ESP_PLATFORM
 #include <esp_log.h>
@@ -38,7 +40,7 @@ struct LogEntry {
  * The array size is TT_LOG_ENTRY_COUNT
  * @param[out] outIndex the write index for the next log entry.
  */
-LogEntry* copyLogEntries(unsigned int& outIndex);
+std::unique_ptr<std::array<LogEntry, TT_LOG_ENTRY_COUNT>> copyLogEntries(std::size_t& outIndex);
 
 } // namespace tt
 

TactilityCore/Source/Mutex.h

@@ -39,24 +39,21 @@ private:
 
 public:
 
+    using Lockable::lock;
+
     explicit Mutex(Type type = Type::Normal);
-    ~Mutex() override = default;
+    ~Mutex() final = default;
 
     /** Attempt to lock the mutex. Blocks until timeout passes or lock is acquired.
      * @param[in] timeout
      * @return success result
      */
-    bool lock(TickType_t timeout) const override;
-
-    /** Attempt to lock the mutex. Blocks until lock is acquired, without timeout.
-     * @return success result
-     */
-    bool lock() const override { return lock(portMAX_DELAY); }
+    bool lock(TickType_t timeout) const final;
 
     /** Attempt to unlock the mutex.
      * @return success result
      */
-    bool unlock() const override;
+    bool unlock() const final;
 
     /** @return the owner of the thread */
     ThreadId getOwner() const;

TactilityCore/Source/Semaphore.cpp

@@ -4,7 +4,7 @@
 
 namespace tt {
 
-static inline struct QueueDefinition* createHandle(uint32_t maxCount, uint32_t initialCount) {
+static inline QueueHandle_t createHandle(uint32_t maxCount, uint32_t initialCount) {
     assert((maxCount > 0U) && (initialCount <= maxCount));
 
     if (maxCount == 1U) {
@@ -21,7 +21,7 @@ static inline struct QueueDefinition* createHandle(uint32_t maxCount, uint32_t i
     }
 }
 
-Semaphore::Semaphore(uint32_t maxCount, uint32_t initialCount) : handle(createHandle(maxCount, initialCount)){
+Semaphore::Semaphore(uint32_t maxAvailable, uint32_t initialAvailable) : handle(createHandle(maxAvailable, initialAvailable)) {
     assert(!TT_IS_IRQ_MODE());
     tt_check(handle != nullptr);
 }
@@ -30,7 +30,7 @@ Semaphore::~Semaphore() {
     assert(!TT_IS_IRQ_MODE());
 }
 
-bool Semaphore::acquire(uint32_t timeout) const {
+bool Semaphore::acquire(TickType_t timeout) const {
     if (TT_IS_IRQ_MODE()) {
         if (timeout != 0U) {
             return false;
@@ -45,7 +45,7 @@ bool Semaphore::acquire(uint32_t timeout) const {
             }
         }
     } else {
-        return xSemaphoreTake(handle.get(), (TickType_t)timeout) == pdPASS;
+        return xSemaphoreTake(handle.get(), timeout) == pdPASS;
     }
 }
 
@@ -63,13 +63,13 @@ bool Semaphore::release() const {
     }
 }
 
-uint32_t Semaphore::getCount() const {
+uint32_t Semaphore::getAvailable() const {
     if (TT_IS_IRQ_MODE()) {
         // TODO: uxSemaphoreGetCountFromISR is not supported on esp-idf 5.1.2 - perhaps later on?
 #ifdef uxSemaphoreGetCountFromISR
         return uxSemaphoreGetCountFromISR(handle.get());
 #else
-        return uxQueueMessagesWaitingFromISR((QueueHandle_t)hSemaphore);
+        return uxQueueMessagesWaitingFromISR(handle.get());
 #endif
     } else {
         return uxSemaphoreGetCount(handle.get());

TactilityCore/Source/Semaphore.h

@@ -1,6 +1,6 @@
 #pragma once
 
-#include "Thread.h"
+#include "Lockable.h"
 #include <cassert>
 #include <memory>
 
@@ -18,7 +18,7 @@ namespace tt {
  * Wrapper for xSemaphoreCreateBinary (max count == 1) and xSemaphoreCreateCounting (max count > 1)
  * Can be used from IRQ/ISR mode, but cannot be created/destroyed from such a context.
  */
-class Semaphore {
+class Semaphore final : public Lockable {
 
 private:
 
@@ -32,24 +32,39 @@ private:
     std::unique_ptr<std::remove_pointer_t<QueueHandle_t>, SemaphoreHandleDeleter> handle;
 
 public:
+
+    using Lockable::lock;
+
     /**
      * Cannot be called from IRQ/ISR mode.
-     * @param[in] maxCount The maximum count
-     * @param[in] initialCount The initial count
+     * @param[in] maxAvailable The maximum count
+     * @param[in] initialAvailable The initial count
      */
-    Semaphore(uint32_t maxCount, uint32_t initialCount);
+    Semaphore(uint32_t maxAvailable, uint32_t initialAvailable);
+
+    /**
+     * Cannot be called from IRQ/ISR mode.
+     * @param[in] maxAvailable The maximum count
+     */
+    explicit Semaphore(uint32_t maxAvailable) : Semaphore(maxAvailable, maxAvailable) {};
 
     /** Cannot be called from IRQ/ISR mode. */
-    ~Semaphore();
+    ~Semaphore() override;
+
+    Semaphore(Semaphore& other) : handle(std::move(other.handle)) {}
 
     /** Acquire semaphore */
-    bool acquire(uint32_t timeout) const;
+    bool acquire(TickType_t timeout) const;
 
     /** Release semaphore */
     bool release() const;
 
-    /** @return semaphore count */
-    uint32_t getCount() const;
+    bool lock(TickType_t timeout) const override { return acquire(timeout); }
+
+    bool unlock() const override { return release(); }
+
+    /** @return return the amount of times this semaphore can be acquired/locked */
+    uint32_t getAvailable() const;
 };
 
 } // namespace

TactilityCore/Source/StringUtils.cpp

@@ -1,21 +1,13 @@
 #include "StringUtils.h"
 #include <cstring>
 #include <sstream>
+#include <string>
 
 namespace tt::string {
 
-int findLastIndex(const char* text, size_t from_index, char find) {
-    for (int i = (int)from_index; i >= 0; i--) {
-        if (text[i] == find) {
-            return (int)i;
-        }
-    }
-    return -1;
-}
-
 bool getPathParent(const std::string& path, std::string& output) {
-    int index = findLastIndex(path.c_str(), path.length() - 1, '/');
-    if (index == -1) {
+    auto index = path.find_last_of('/');
+    if (index == std::string::npos) {
         return false;
     } else if (index == 0) {
         output = "/";

TactilityCore/Source/StringUtils.h

@@ -7,15 +7,6 @@
 
 namespace tt::string {
 
-/**
- * Find the last occurrence of a character.
- * @param[in] text the text to search in
- * @param[in] from_index the index to search from (searching from right to left)
- * @param[in] find the character to search for
- * @return the index of the found character, or -1 if none found
- */
-int findLastIndex(const char* text, size_t from_index, char find);
-
 /**
  * Given a filesystem path as input, try and get the parent path.
  * @param[in] path input path

TactilityCore/Source/kernel/critical/Critical.cpp

@@ -11,8 +11,8 @@ static portMUX_TYPE critical_mutex;
 
 namespace tt::kernel::critical {
 
-TtCriticalInfo enter() {
-    TtCriticalInfo info = {
+CriticalInfo enter() {
+    CriticalInfo info = {
         .isrm = 0,
         .fromIsr = TT_IS_ISR(),
         .kernelRunning = (xTaskGetSchedulerState() == taskSCHEDULER_RUNNING)
@@ -29,7 +29,7 @@ TtCriticalInfo enter() {
     return info;
 }
 
-void exit(TtCriticalInfo info) {
+void exit(CriticalInfo info) {
     if (info.fromIsr) {
         taskEXIT_CRITICAL_FROM_ISR(info.isrm);
     } else if (info.kernelRunning) {

TactilityCore/Source/kernel/critical/Critical.h

@@ -4,21 +4,21 @@
 
 namespace tt::kernel::critical {
 
-typedef struct {
+struct CriticalInfo {
     uint32_t isrm;
     bool fromIsr;
     bool kernelRunning;
-} TtCriticalInfo;
+};
 
 /** Enter a critical section
  * @return info on the status
  */
-TtCriticalInfo enter();
+CriticalInfo enter();
 
 /**
  * Exit a critical section
  * @param[in] info the info from when the critical section was started
  */
-void exit(TtCriticalInfo info);
+void exit(CriticalInfo info);
 
 } // namespace