Fix #55 for values

main/core/esp3d_values.cpp

@@ -20,6 +20,10 @@
 */
 #include "esp3d_values.h"
 
+#include <pthread.h>
+
+#include <algorithm>
+
 #include "esp3d_log.h"
 #include "esp3d_string.h"
 #include "esp3d_version.h"
@@ -29,7 +33,12 @@ ESP3DValues esp3dTftValues;
 // value of settings, storage values are all strings because it is easier to
 // display
 
-ESP3DValues::ESP3DValues() { intialize(); }
+ESP3DValues::ESP3DValues() {
+  intialize();
+  if (pthread_mutex_init(&_mutex, NULL) != 0) {
+    printf("\n mutex init failed\n");
+  }
+}
 
 void ESP3DValues::clear() {
   for (auto element = _values.begin(); element != _values.end(); ++element) {
@@ -40,7 +49,10 @@ void ESP3DValues::clear() {
   _values.clear();
 }
 
-ESP3DValues::~ESP3DValues() { clear(); }
+ESP3DValues::~ESP3DValues() {
+  clear();
+  pthread_mutex_destroy(&_mutex);
+}
 
 const ESP3DValuesDescription* ESP3DValues::get_description(
     ESP3DValuesIndex index) {
@@ -56,24 +68,62 @@ const char* ESP3DValues::get_string_value(ESP3DValuesIndex index) {
   return nullptr;
 }
 
+void ESP3DValues::handle() {
+  if (pthread_mutex_lock(&_mutex) == 0) {
+    // check if there is something to do
+    if (_updated_values_queue.size() > 0) {
+      uint8_t nb = 0;
+      // lets process 10 values max to avoid blocking the loop too long
+      while (!_updated_values_queue.empty() && nb < 10) {
+        ESP3DValuesData& element = _updated_values_queue.front();
+        // update value and call the callback function if any
+        element.description->value = element.value;
+        esp3d_log("Setting String value %s for %d", element.value.c_str(),
+                  (int)element.index);
+        if (element.description->callbackFn) {
+          element.description->callbackFn(element.index, element.value.c_str(),
+                         element.action);
+        }
+         // remove front element from queue
+      _updated_values_queue.pop_front();
+      nb++;
+      }
+    }
+  }
+  if (pthread_mutex_unlock(&_mutex) != 0) {
+    esp3d_log_e("Cannot unlock mutex");
+  }
+}
+
 bool ESP3DValues::set_string_value(ESP3DValuesIndex index, const char* value,
                                    ESP3DValuesCbAction action) {
+  bool result = false;
   if (_values.size() == 0) {
     // No values list set  - service is ignored
     return true;
   }
-  for (auto element = _values.begin(); element != _values.end(); ++element) {
-    if (element->index == index) {
-      element->value = value;
-      esp3d_log("Setting String value %s for %d", value, (int)index);
-      if (element->callbackFn) {
-        //FIXME:Instead of immediatly use the callback function
-        //we should use a queue and call it in the loop to be sure there is no race condition
-        element->callbackFn(element->index, value, action);
-      }
-      return true;
+  // use mutex to do successive calls and avoid any race condition
+  if (pthread_mutex_lock(&_mutex) == 0) {
+    // check if index is valid
+    auto it = std::find_if(_values.begin(), _values.end(),
+                           [index](const ESP3DValuesDescription& data) {
+                             return data.index == index;
+                           });
+    // is it found ?
+    if (it != _values.end()) {
+      // yes found it, push value in queue to be processed later
+      _updated_values_queue.emplace_back(ESP3DValuesData{
+          std::string(value ? value : ""), action, index, &(*it)});
+      result = true;
+    } else {
+      // not found - error
+      esp3d_log_w("Cannot set String value %s for %d", value, (int)index);
+    }
+    if (pthread_mutex_unlock(&_mutex) != 0) {
+      esp3d_log_e("Cannot unlock mutex");
     }
+  } else {
+    esp3d_log_e("Cannot lock mutex");
   }
-  esp3d_log_w("Cannot set String value %s for %d", value, (int)index);
-  return false;
+  return result;
 }

main/core/includes/esp3d_values.h

@@ -25,10 +25,12 @@
 #include <functional>
 #include <list>
 #include <string>
+//For the symbol used 
 #if ESP3D_DISPLAY_FEATURE
 #include "lvgl.h"
 #endif  // ESP3D_DISPLAY_FEATURE
 #include "esp3d_values_list.h"
+#include <pthread.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -60,12 +62,20 @@ struct ESP3DValuesDescription {
   callbackFunction_t callbackFn = nullptr;
 };
 
+struct ESP3DValuesData{
+  std::string value = "";
+  ESP3DValuesCbAction action = ESP3DValuesCbAction::Update;
+  ESP3DValuesIndex index = ESP3DValuesIndex::unknown_index;
+  ESP3DValuesDescription * description = nullptr;
+};
+
 class ESP3DValues final {
  public:
   ESP3DValues();
   ~ESP3DValues();
   bool intialize();
   void clear();
+  void handle();
   const ESP3DValuesDescription* get_description(ESP3DValuesIndex index);
   const char* get_string_value(ESP3DValuesIndex index);
   bool set_string_value(
@@ -74,6 +84,8 @@ class ESP3DValues final {
 
  private:
   std::list<ESP3DValuesDescription> _values;
+  std::list<ESP3DValuesData> _updated_values_queue;
+  pthread_mutex_t _mutex;
 };
 
 extern ESP3DValues esp3dTftValues;

main/core/includes/esp3d_version.h

@@ -23,7 +23,7 @@
 #ifdef __cplusplus
 extern "C" {
 #endif
-#define ESP3D_TFT_VERSION "1.0.0.a19"
+#define ESP3D_TFT_VERSION "1.0.0.a20"
 #define ESP3D_TFT_FW_URL "https://github.com/luc-github/ESP3D-TFT"
 
 #ifdef __cplusplus

main/display/esp3d_tft_ui.cpp

@@ -33,6 +33,7 @@
 #include "lvgl.h"
 #include "rendering/esp3d_rendering_client.h"
 #include "tasks_def.h"
+#include "esp3d_values.h"
 
 #define STACKDEPTH UI_STACK_DEPTH
 #define TASKPRIORITY UI_TASK_PRIORITY
@@ -85,6 +86,7 @@ static void guiTask(void *pvParameter) {
 
     /* Try to take the semaphore, call lvgl related function on success */
     if (pdTRUE == xSemaphoreTake(xGuiSemaphore, portMAX_DELAY)) {
+      esp3dTftValues.handle();
       lv_task_handler();
       xSemaphoreGive(xGuiSemaphore);
     }