Add mutex to chairAdjust object - fix potential crashes

common/chairAdjust.cpp

@@ -7,6 +7,7 @@ extern "C"
 #include "chairAdjust.hpp"
 
 
+#define MUTEX_TIMEOUT (10000 / portTICK_PERIOD_MS)
 
 //--- gloabl variables ---
 // strings for logging the rest state
@@ -21,12 +22,12 @@ static const char * TAG = "chair-adjustment";
 //=============================
 //======== constructor ========
 //=============================
-cControlledRest::cControlledRest(gpio_num_t gpio_up_f, gpio_num_t gpio_down_f, uint32_t travelDurationMs, const char * name_f, float defaultPosition):travelDuration(travelDurationMs){
+cControlledRest::cControlledRest(gpio_num_t gpio_up_f, gpio_num_t gpio_down_f, uint32_t travelDurationMs, const char * name_f, float defaultPosition): gpio_up(gpio_up_f), gpio_down(gpio_down_f), travelDuration(travelDurationMs){
     strcpy(name, name_f);
-    gpio_up = gpio_up_f;
-    gpio_down = gpio_down_f;
     positionNow = defaultPosition;
     positionTarget = positionNow;
+    // recursive mutex necessary, because handle() method calls setState() which both have the same mutex
+    mutex = xSemaphoreCreateRecursiveMutex();
     init();
 }
 
@@ -95,6 +96,9 @@ void cControlledRest::updatePosition(){
 //============================
 void cControlledRest::setState(restState_t targetState)
 {
+    // lock the mutex before accessing shared variables
+    if (xSemaphoreTakeRecursive(mutex, MUTEX_TIMEOUT) == pdTRUE)
+    {
         // TODO: drop this section?
         // check if actually changed
         if (targetState == state)
@@ -115,9 +119,9 @@ void cControlledRest::setState(restState_t targetState)
 
         // activate handle task when turning on (previous state is off)
         if (state == REST_OFF)
-        xTaskNotifyGive(taskHandle); //activate handle task that stops the rest-motor again
+            xTaskNotifyGive(taskHandle); // activate handle task that stops the rest-motor again
 
-    //apply new state
+        // apply new state
         ESP_LOGI(TAG, "[%s] switching from state '%s' to '%s'", name, restStateStr[state], restStateStr[targetState]);
         switch (targetState)
         {
@@ -135,18 +139,30 @@ void cControlledRest::setState(restState_t targetState)
             gpio_set_level(gpio_down, 0);
             gpio_set_level(gpio_up, 0);
             updatePosition();
-        positionTarget = positionNow; //disable resuming - no unexpected pos when incrementing
+            positionTarget = positionNow; // disable resuming - no unexpected pos when incrementing
             break;
         }
         state = targetState;
+
+        // Release the mutex
+        xSemaphoreGiveRecursive(mutex);
+    }
+    else
+    {
+        ESP_LOGE(TAG, "mutex timeout in setState() -> RESTART");
+        esp_restart();
+    }
 }
 
-
-
 //==========================
 //==== setTargetPercent ====
 //==========================
-void cControlledRest::setTargetPercent(float targetPercent){
+void cControlledRest::setTargetPercent(float targetPercent)
+{
+    // lock the mutex before accessing shared variables
+    if (xSemaphoreTakeRecursive(mutex, MUTEX_TIMEOUT) == pdTRUE)
+    {
+        positionTarget = targetPercent;
         float positionTargetPrev = positionTarget;
         positionTarget = targetPercent;
 
@@ -157,7 +173,7 @@ void cControlledRest::setTargetPercent(float targetPercent){
             positionTarget = 0;
 
         // ignore if unchanged
-    //if (positionTarget == positionTargetPrev){
+        // if (positionTarget == positionTargetPrev){
         //    ESP_LOGI(TAG, "[%s] Target position unchanged at %.2f%%", name, positionTarget);
         //    return;
         //}
@@ -173,17 +189,27 @@ void cControlledRest::setTargetPercent(float targetPercent){
             setState(REST_DOWN);
         else // already at exact position
             setState(REST_OFF);
+
+        // Release the mutex
+        xSemaphoreGiveRecursive(mutex);
+    }
+    else
+    {
+        ESP_LOGE(TAG, "mutex timeout while waiting in setTargetPercent -> RESTART");
+        esp_restart();
+    }
 }
 
-
-
 //======================
 //======= handle =======
 //======================
 // handle automatic stop when target position is reached, should be run repeatedly in a task
 #define TRAVEL_TIME_LIMIT_ADDITION_MS 2000 // traveling longer into limit compensates inaccuracies in time based position tracking
-void cControlledRest::handle(){
+void cControlledRest::handle()
-
+{
+    // lock the mutex before accessing shared variables
+    if (xSemaphoreTakeRecursive(mutex, MUTEX_TIMEOUT) == pdTRUE)
+    {
         // nothing to do when not running atm
         // TODO: turn on automatically when position != target?
         if (state == REST_OFF)
@@ -198,14 +224,22 @@ void cControlledRest::handle(){
             timeTarget += TRAVEL_TIME_LIMIT_ADDITION_MS;
 
         // target reached
-    if (timeRan >= timeTarget){
+        if (timeRan >= timeTarget)
+        {
             ESP_LOGW(TAG, "[%s] handle: reached target run-time (%dms/%dms) for position %.2f%% -> stopping", name, timeRan, timeTarget, positionTarget);
             setState(REST_OFF);
         }
+
+        // Release the mutex
+        xSemaphoreGiveRecursive(mutex);
+    }
+    else
+    {
+        ESP_LOGE(TAG, "mutex timeout while waiting in handle() -> RESTART");
+        esp_restart();
+    }
 }
 
-
-
 //============================
 //===== chairAdjust_task =====
 //============================

common/chairAdjust.hpp

@@ -3,6 +3,7 @@ extern "C"
 {
 #include "freertos/FreeRTOS.h"
 #include "freertos/task.h"
+#include "freertos/semphr.h"
 #include "driver/gpio.h"
 }
 
@@ -38,11 +39,14 @@ private:
     void init();
     void updatePosition();
 
+    SemaphoreHandle_t mutex;
+
     char name[32];
-    gpio_num_t gpio_up;
+    const gpio_num_t gpio_up;
-    gpio_num_t gpio_down;
+    const gpio_num_t gpio_down;
-    restState_t state;
     const uint32_t travelDuration = 12000;
+
+    restState_t state;
     uint32_t timestamp_lastPosUpdate = 0;
     float positionTarget = 0;
     float positionNow = 0;