Thread, Timer and flash.sh improvements (#165)

- Various improvements to Thread and Timer:
  - Remove "mark as static" option as it is unused
  - Implemented core pinning for ESP32 platforms
  - Use `TickType_t` consistently (instead of `uint32_t`)
  - Use `enum class` instead of `enum`
- Fix for `flash.sh` not working when using `pip` to install `esptool`

TactilityCore/Source/DispatcherThread.cpp

@@ -18,7 +18,7 @@ DispatcherThread::DispatcherThread(const std::string& threadName, size_t threadS
 }
 
 DispatcherThread::~DispatcherThread() {
-    if (thread->getState() != Thread::StateStopped) {
+    if (thread->getState() != Thread::State::Stopped) {
         stop();
     }
 }

TactilityCore/Source/Thread.cpp

@@ -19,7 +19,7 @@ namespace tt {
 #define THREAD_FLAGS_INVALID_BITS (~((1UL << MAX_BITS_TASK_NOTIFY) - 1U))
 #define EVENT_FLAGS_INVALID_BITS (~((1UL << MAX_BITS_EVENT_GROUPS) - 1U))
 
-static_assert(Thread::PriorityHighest <= TT_CONFIG_THREAD_MAX_PRIORITIES, "highest thread priority is higher than max priority");
+static_assert(static_cast<UBaseType_t>(Thread::Priority::Critical) <= TT_CONFIG_THREAD_MAX_PRIORITIES, "highest thread priority is higher than max priority");
 static_assert(TT_CONFIG_THREAD_MAX_PRIORITIES <= configMAX_PRIORITIES, "highest tactility priority is higher than max FreeRTOS priority");
 
 void setState(Thread::Data* data, Thread::State state) {
@@ -29,6 +29,8 @@ void setState(Thread::Data* data, Thread::State state) {
     }
 }
 
+static_assert(configSUPPORT_DYNAMIC_ALLOCATION == 1);
+
 /** Catch threads that are trying to exit wrong way */
 __attribute__((__noreturn__)) void thread_catch() { //-V1082
     // If you're here it means you're probably doing something wrong
@@ -47,20 +49,12 @@ static void thread_body(void* context) {
     tt_assert(pvTaskGetThreadLocalStoragePointer(nullptr, 0) == nullptr);
     vTaskSetThreadLocalStoragePointer(nullptr, 0, data->thread);
 
-    tt_assert(data->state == Thread::StateStarting);
-    setState(data, Thread::StateRunning);
+    tt_assert(data->state == Thread::State::Starting);
+    setState(data, Thread::State::Running);
     data->callbackResult = data->callback(data->callbackContext);
-    tt_assert(data->state == Thread::StateRunning);
+    tt_assert(data->state == Thread::State::Running);
 
-    if (data->isStatic) {
-        TT_LOG_I(
-            TAG,
-            "%s static task memory will not be reclaimed",
-            data->name.empty() ? "<unnamed service>" : data->name.c_str()
-        );
-    }
-
-    setState(data, Thread::StateStopped);
+    setState(data, Thread::State::Stopped);
 
     vTaskSetThreadLocalStoragePointer(nullptr, 0, nullptr);
     data->taskHandle = nullptr;
@@ -73,15 +67,14 @@ Thread::Thread() :
     data({
         .thread = nullptr,
         .taskHandle = nullptr,
-        .state = StateStopped,
+        .state = State::Stopped,
         .callback = nullptr,
         .callbackContext = nullptr,
         .callbackResult = 0,
         .stateCallback = nullptr,
         .stateCallbackContext = nullptr,
         .name = std::string(),
-        .priority = PriorityNormal,
-        .isStatic = false,
+        .priority = Priority::Normal,
         .stackSize = 0,
     }) { }
 
@@ -89,64 +82,56 @@ Thread::Thread(
     const std::string& name,
     configSTACK_DEPTH_TYPE stackSize,
     Callback callback,
-    _Nullable void* callbackContext) :
+    _Nullable void* callbackContext,
+    portBASE_TYPE affinity
+) :
     data({
         .thread = nullptr,
         .taskHandle = nullptr,
-        .state = StateStopped,
+        .state = State::Stopped,
         .callback = callback,
         .callbackContext = callbackContext,
         .callbackResult = 0,
         .stateCallback = nullptr,
         .stateCallbackContext = nullptr,
         .name = name,
-        .priority = PriorityNormal,
-        .isStatic = false,
+        .priority = Priority::Normal,
         .stackSize = stackSize,
+        .affinity = affinity
     }) { }
 
 Thread::~Thread() {
     // Ensure that use join before free
-    tt_assert(data.state == StateStopped);
+    tt_assert(data.state == State::Stopped);
     tt_assert(data.taskHandle == nullptr);
 }
 
 void Thread::setName(const std::string& newName) {
-    tt_assert(data.state == StateStopped);
+    tt_assert(data.state == State::Stopped);
     data.name = newName;
 }
 
-
-void Thread::markAsStatic() {
-    data.isStatic = true;
-}
-
-bool Thread::isMarkedAsStatic() const {
-    return data.isStatic;
-}
-
 void Thread::setStackSize(size_t stackSize) {
-    tt_assert(data.state == StateStopped);
+    tt_assert(data.state == State::Stopped);
     tt_assert(stackSize % 4 == 0);
     data.stackSize = stackSize;
 }
 
 void Thread::setCallback(Callback callback, _Nullable void* callbackContext) {
-    tt_assert(data.state == StateStopped);
+    tt_assert(data.state == State::Stopped);
     data.callback = callback;
     data.callbackContext = callbackContext;
 }
 
 
 void Thread::setPriority(Priority priority) {
-    tt_assert(data.state == StateStopped);
-    tt_assert(priority >= 0 && priority <= TT_CONFIG_THREAD_MAX_PRIORITIES);
+    tt_assert(data.state == State::Stopped);
     data.priority = priority;
 }
 
 
 void Thread::setStateCallback(StateCallback callback, _Nullable void* callbackContext) {
-    tt_assert(data.state == StateStopped);
+    tt_assert(data.state == State::Stopped);
     data.stateCallback = callback;
     data.stateCallbackContext = callbackContext;
 }
@@ -157,38 +142,49 @@ Thread::State Thread::getState() const {
 
 void Thread::start() {
     tt_assert(data.callback);
-    tt_assert(data.state == StateStopped);
-    tt_assert(data.stackSize > 0 && data.stackSize < (UINT16_MAX * sizeof(StackType_t)));
+    tt_assert(data.state == State::Stopped);
+    tt_assert(data.stackSize > 0U && data.stackSize < (UINT16_MAX * sizeof(StackType_t)));
 
-    setState(&data, StateStarting);
+    setState(&data, State::Starting);
 
     uint32_t stack_depth = data.stackSize / sizeof(StackType_t);
-    if (data.isStatic) {
-#if configSUPPORT_STATIC_ALLOCATION == 1
-        data.taskHandle = xTaskCreateStatic(
+
+    BaseType_t result;
+    if (data.affinity != -1) {
+#ifdef ESP_PLATFORM
+        result = xTaskCreatePinnedToCore(
             thread_body,
             data.name.c_str(),
             stack_depth,
-            &data,
-            data.priority,
-            static_cast<StackType_t*>(malloc(sizeof(StackType_t) * stack_depth)),
-            static_cast<StaticTask_t*>(malloc(sizeof(StaticTask_t)))
+            this,
+            static_cast<UBaseType_t>(data.priority),
+            &(data.taskHandle),
+            data.affinity
         );
 #else
-        TT_LOG_E(TAG, "static tasks are not supported by current FreeRTOS config/platform - creating regular one");
-        BaseType_t result = xTaskCreate(
-            thread_body, data.name.c_str(), stack_depth, this, data.priority, &(data.taskHandle)
+        TT_LOG_W(TAG, "Pinned tasks are not supported by current FreeRTOS platform - creating regular one");
+        result = xTaskCreate(
+            thread_body,
+            data.name.c_str(),
+            stack_depth,
+            this,
+            static_cast<UBaseType_t>(data.priority),
+            &(data.taskHandle)
         );
-        tt_check(result == pdPASS);
 #endif
     } else {
-        BaseType_t result = xTaskCreate(
-            thread_body, data.name.c_str(), stack_depth, this, data.priority, &(data.taskHandle)
+        result = xTaskCreate(
+            thread_body,
+            data.name.c_str(),
+            stack_depth,
+            this,
+            static_cast<UBaseType_t>(data.priority),
+            &(data.taskHandle)
         );
-        tt_check(result == pdPASS);
     }
 
-    tt_check(data.state == StateStopped || data.taskHandle);
+    tt_check(result == pdPASS);
+    tt_check(data.state == State::Stopped || data.taskHandle);
 }
 
 bool Thread::join() {
@@ -205,12 +201,12 @@ bool Thread::join() {
     return true;
 }
 
-ThreadId Thread::getId() {
+ThreadId Thread::getId() const {
     return data.taskHandle;
 }
 
-int32_t Thread::getReturnCode() {
-    tt_assert(data.state == StateStopped);
+int32_t Thread::getReturnCode() const {
+    tt_assert(data.state == State::Stopped);
     return data.callbackResult;
 }
 
@@ -224,8 +220,7 @@ Thread* thread_get_current() {
 }
 
 void thread_set_current_priority(Thread::Priority priority) {
-    UBaseType_t new_priority = priority ? priority : Thread::PriorityNormal;
-    vTaskPrioritySet(nullptr, new_priority);
+    vTaskPrioritySet(nullptr, static_cast<UBaseType_t>(priority));
 }
 
 Thread::Priority thread_get_current_priority() {
@@ -237,14 +232,6 @@ void thread_yield() {
     taskYIELD();
 }
 
-bool thread_mark_is_static(ThreadId thread_id) {
-    auto hTask = (TaskHandle_t)thread_id;
-    assert(!TT_IS_IRQ_MODE() && (hTask != nullptr));
-    auto* thread = (Thread*)pvTaskGetThreadLocalStoragePointer(hTask, 0);
-    assert(thread != nullptr);
-    return thread->isMarkedAsStatic();
-}
-
 uint32_t thread_flags_set(ThreadId thread_id, uint32_t flags) {
     auto hTask = (TaskHandle_t)thread_id;
     uint32_t rflags;

TactilityCore/Source/Thread.h

@@ -16,24 +16,23 @@ typedef TaskHandle_t ThreadId;
 class Thread {
 public:
 
-    typedef enum {
-        StateStopped,
-        StateStarting,
-        StateRunning,
-    } State;
+    enum class State{
+        Stopped,
+        Starting,
+        Running,
+    };
 
     /** ThreadPriority */
-    typedef enum {
-        PriorityNone = 0, /**< Uninitialized, choose system default */
-        PriorityIdle = 1,
-        PriorityLowest = 2,
-        PriorityLow = 3,
-        PriorityNormal = 4,
-        PriorityHigh = 5,
-        PriorityHigher = 6,
-        PriorityHighest = 7
-    } Priority;
-
+    enum class Priority : UBaseType_t {
+        None = 0U, /**< Uninitialized, choose system default */
+        Idle = 1U,
+        Lower = 2U,
+        Low = 3U,
+        Normal = 4U,
+        High = 5U,
+        Higher = 6U,
+        Critical = 7U
+    };
 
     /** ThreadCallback Your callback to run in new thread
      * @warning never use osThreadExit in Thread
@@ -55,41 +54,33 @@ public:
     typedef struct {
         Thread* thread;
         TaskHandle_t taskHandle;
-
         State state;
-
         Callback callback;
         void* callbackContext;
         int32_t callbackResult;
-
         StateCallback stateCallback;
         void* stateCallbackContext;
-
         std::string name;
-
         Priority priority;
-
-        // Keep all non-alignable byte types in one place,
-        // this ensures that the size of this structure is minimal
-        bool isStatic;
-
         configSTACK_DEPTH_TYPE stackSize;
+        portBASE_TYPE affinity;
     } Data;
 
     Thread();
 
     /** Allocate Thread, shortcut version
-     * @param[in] name
-     * @param[in] stack_size
+     * @param[in] name the name of the thread
+     * @param[in] stackSize in bytes
      * @param[in] callback
      * @param[in] callbackContext
-     * @return Thread*
+     * @param[in] affinity Which CPU core to pin this task to, -1 means unpinned (only works on ESP32)
      */
     Thread(
         const std::string& name,
         configSTACK_DEPTH_TYPE stackSize,
         Callback callback,
-        _Nullable void* callbackContext
+        _Nullable void* callbackContext,
+        portBASE_TYPE affinity = -1
     );
 
     ~Thread();
@@ -99,16 +90,6 @@ public:
      */
     void setName(const std::string& name);
 
-    /** Mark thread as service
-     * The service cannot be stopped or removed, and cannot exit from the thread body
-     */
-    void markAsStatic();
-
-    /** Check if thread is as service
-     * If true, the service cannot be stopped or removed, and cannot exit from the thread body
-     */
-    bool isMarkedAsStatic() const;
-
     /** Set Thread stack size
      * @param[in] stackSize stack size in bytes
      */
@@ -137,8 +118,7 @@ public:
      */
     State getState() const;
 
-    /** Start Thread
-     */
+    /** Start Thread */
     void start();
 
     /** Join Thread
@@ -150,10 +130,10 @@ public:
     /** Get FreeRTOS ThreadId for Thread instance
      * @return ThreadId or nullptr
      */
-    ThreadId getId();
+    ThreadId getId() const;
 
     /** @return thread return code */
-    int32_t getReturnCode();
+    int32_t getReturnCode() const;
 
 private:
 
@@ -161,9 +141,9 @@ private:
 };
 
 #define THREAD_PRIORITY_APP Thread::PriorityNormal
-#define THREAD_PRIORITY_SERVICE Thread::PriorityHigh
-#define THREAD_PRIORITY_RENDER Thread::PriorityHigher
-#define THREAD_PRIORITY_ISR (TT_CONFIG_THREAD_MAX_PRIORITIES - 1)
+#define THREAD_PRIORITY_SERVICE Thread::Priority::High
+#define THREAD_PRIORITY_RENDER Thread::Priority::Higher
+#define THREAD_PRIORITY_ISR Thread::Priority::Critical
 
 /** Set current thread priority
  * @param[in] priority ThreadPriority value
@@ -220,10 +200,4 @@ void thread_resume(ThreadId threadId);
  */
 bool thread_is_suspended(ThreadId threadId);
 
-/** Check if the thread was created with static memory
- * @param[in] threadId thread id
- * @return true if thread memory is static
- */
-bool thread_mark_is_static(ThreadId threadId);
-
 } // namespace

TactilityCore/Source/Timer.cpp

@@ -36,16 +36,16 @@ Timer::~Timer() {
     tt_check(xTimerDelete(timerHandle, portMAX_DELAY) == pdPASS);
 }
 
-bool Timer::start(uint32_t intervalTicks) {
+bool Timer::start(TickType_t interval) {
     tt_assert(!TT_IS_ISR());
-    tt_assert(intervalTicks < portMAX_DELAY);
-    return xTimerChangePeriod(timerHandle, intervalTicks, portMAX_DELAY) == pdPASS;
+    tt_assert(interval < portMAX_DELAY);
+    return xTimerChangePeriod(timerHandle, interval, portMAX_DELAY) == pdPASS;
 }
 
-bool Timer::restart(uint32_t intervalTicks) {
+bool Timer::restart(TickType_t interval) {
     tt_assert(!TT_IS_ISR());
-    tt_assert(intervalTicks < portMAX_DELAY);
-    return xTimerChangePeriod(timerHandle, intervalTicks, portMAX_DELAY) == pdPASS &&
+    tt_assert(interval < portMAX_DELAY);
+    return xTimerChangePeriod(timerHandle, interval, portMAX_DELAY) == pdPASS &&
         xTimerReset(timerHandle, portMAX_DELAY) == pdPASS;
 }
 
@@ -59,9 +59,9 @@ bool Timer::isRunning() {
     return xTimerIsTimerActive(timerHandle) == pdTRUE;
 }
 
-uint32_t Timer::getExpireTime() {
+TickType_t Timer::getExpireTime() {
     tt_assert(!TT_IS_ISR());
-    return (uint32_t)xTimerGetExpiryTime(timerHandle);
+    return xTimerGetExpiryTime(timerHandle);
 }
 
 bool Timer::setPendingCallback(PendingCallback callback, void* callbackContext, uint32_t callbackArg, TickType_t timeout) {
@@ -73,19 +73,13 @@ bool Timer::setPendingCallback(PendingCallback callback, void* callbackContext,
     }
 }
 
-void Timer::setThreadPriority(ThreadPriority priority)  {
+void Timer::setThreadPriority(Thread::Priority priority)  {
     tt_assert(!TT_IS_ISR());
 
     TaskHandle_t task_handle = xTimerGetTimerDaemonTaskHandle();
     tt_assert(task_handle); // Don't call this method before timer task start
 
-    if (priority == TimerThreadPriorityNormal) {
-        vTaskPrioritySet(task_handle, configTIMER_TASK_PRIORITY);
-    } else if (priority == TimerThreadPriorityElevated) {
-        vTaskPrioritySet(task_handle, configMAX_PRIORITIES - 1);
-    } else {
-        tt_crash("Unsupported timer priority");
-    }
+    vTaskPrioritySet(task_handle, static_cast<UBaseType_t>(priority));
 }
 
 } // namespace

TactilityCore/Source/Timer.h

@@ -3,6 +3,7 @@
 #include "CoreTypes.h"
 
 #include "RtosCompatTimers.h"
+#include "Thread.h"
 #include <memory>
 
 namespace tt {
@@ -34,17 +35,17 @@ public:
 
     /** Start timer
      * @warning This is asynchronous call, real operation will happen as soon as timer service process this request.
-     * @param[in] ticks The interval in ticks
+     * @param[in] interval The interval in ticks
      * @return success result
      */
-    bool start(uint32_t intervalTicks);
+    bool start(TickType_t interval);
 
     /** Restart timer with previous timeout value
      * @warning This is asynchronous call, real operation will happen as soon as timer service process this request.
-     * @param[in] ticks The interval in ticks
+     * @param[in] interval The interval in ticks
      * @return success result
      */
-    bool restart(uint32_t intervalTicks);
+    bool restart(TickType_t interval);
 
     /** Stop timer
      * @warning This is asynchronous call, real operation will happen as soon as timer service process this request.
@@ -61,7 +62,7 @@ public:
     /** Get timer expire time
      * @return expire tick
      */
-    uint32_t getExpireTime();
+    TickType_t getExpireTime();
 
     /**
      * Calls xTimerPendFunctionCall internally.
@@ -69,18 +70,19 @@ public:
      * @param[in] callbackContext the first function argument
      * @param[in] callbackArg the second function argument
      * @param[in] timeout the function timeout (must set to 0 in ISR mode)
+     * @return true on success
      */
     bool setPendingCallback(PendingCallback callback, void* callbackContext, uint32_t callbackArg, TickType_t timeout);
 
-    typedef enum {
-        TimerThreadPriorityNormal,   /**< Lower then other threads */
-        TimerThreadPriorityElevated, /**< Same as other threads */
-    } ThreadPriority;
+    enum class Priority{
+        Normal,   /**< Lower then other threads */
+        Elevated, /**< Same as other threads */
+    };
 
     /** Set Timer thread priority
      * @param[in] priority The priority
      */
-    void setThreadPriority(ThreadPriority priority);
+    void setThreadPriority(Thread::Priority priority);
 };
 
 } // namespace