Add various bugfixes

Author: hhvrc

include/http/HTTPRequestManager.h

@@ -79,6 +79,7 @@ namespace OpenShock::HTTP {
 
     T data;
     if (!jsonParser(response.code, json, data)) {
+      cJSON_Delete(json);
       return {RequestResult::ParseFailed, response.code, {}};
     }
 

src/CommandHandler.cpp

@@ -86,6 +86,7 @@ static void commandhandler_keepalivetask(void* arg)
       if (cmdRef.lastActivityTimestamp + KEEP_ALIVE_INTERVAL < now) {
         OS_LOGV(TAG, "Sending keep-alive for shocker %u", cmdRef.shockerId);
 
+        ScopedReadLock rfLock__(&s_rfTransmitterMutex);
         if (s_rfTransmitter == nullptr) {
           OS_LOGW(TAG, "RF Transmitter is not initialized, ignoring keep-alive");
           break;
@@ -229,6 +230,7 @@ bool CommandHandler::Init()
 
 bool CommandHandler::Ok()
 {
+  ScopedReadLock lock__(&s_rfTransmitterMutex);
   return s_rfTransmitter != nullptr;
 }
 
@@ -278,6 +280,7 @@ bool CommandHandler::SetKeepAliveEnabled(bool enabled)
 
 gpio_num_t CommandHandler::GetRfTxPin()
 {
+  ScopedReadLock lock__(&s_rfTransmitterMutex);
   if (s_rfTransmitter != nullptr) {
     return s_rfTransmitter->GetTxPin();
   }

src/EStopManager.cpp

@@ -33,9 +33,9 @@ static OpenShock::SimpleMutex s_estopMutex = {};
 static gpio_num_t s_estopPin               = GPIO_NUM_NC;
 static TaskHandle_t s_estopTask            = nullptr;
 
-static EStopState s_lastPublishedState = EStopState::Idle;
-static bool s_estopActive              = false;
-static int64_t s_estopActivatedAt      = 0;
+static EStopState s_lastPublishedState     = EStopState::Idle;
+static volatile bool s_estopActive         = false;
+static volatile int64_t s_estopActivatedAt = 0;
 
 static volatile bool s_externallyTriggered = false;
 

src/GatewayClient.cpp

@@ -155,8 +155,6 @@ void GatewayClient::_sendBootStatus()
 
 void GatewayClient::_handleEvent(WStype_t type, uint8_t* payload, std::size_t length)
 {
-  (void)payload;
-
   switch (type) {
     case WStype_DISCONNECTED:
       _setState(GatewayClientState::Disconnected);

src/GatewayConnectionManager.cpp

@@ -10,6 +10,9 @@ const char* const TAG = "GatewayConnectionManager";
 #include "http/JsonAPI.h"
 #include "Logging.h"
 
+#include "SimpleMutex.h"
+
+#include <atomic>
 #include <unordered_map>
 
 //
@@ -34,25 +37,29 @@ const uint8_t FLAG_LINKED = 1 << 1;
 
 const uint8_t LINK_CODE_LENGTH = 6;
 
-static uint8_t s_flags                                      = 0;
-static int64_t s_lastAuthFailure                            = 0;
-static int64_t s_lastConnectionAttempt                      = 0;
+static std::atomic<uint8_t> s_flags                 = 0;
+static std::atomic<int64_t> s_lastAuthFailure       = 0;
+static std::atomic<int64_t> s_lastConnectionAttempt = 0;
+static OpenShock::SimpleMutex s_clientMutex;
 static std::unique_ptr<OpenShock::GatewayClient> s_wsClient = nullptr;
 
 static void evh_gotIP(arduino_event_t* event)
 {
   (void)event;
 
-  s_flags |= FLAG_HAS_IP;
+  s_flags.fetch_or(FLAG_HAS_IP, std::memory_order_relaxed);
   OS_LOGD(TAG, "Got IP address");
 }
 
 static void evh_wiFiDisconnected(arduino_event_t* event)
 {
   (void)event;
 
-  s_flags    = FLAG_NONE;
-  s_wsClient = nullptr;
+  s_flags.store(FLAG_NONE, std::memory_order_relaxed);
+  {
+    OpenShock::ScopedLock lock__(&s_clientMutex);
+    s_wsClient = nullptr;
+  }
   OS_LOGD(TAG, "Lost IP address");
 }
 
@@ -79,6 +86,7 @@ bool GatewayConnectionManager::Init()
 
 bool GatewayConnectionManager::IsConnected()
 {
+  ScopedLock lock__(&s_clientMutex);
   if (s_wsClient == nullptr) {
     return false;
   }
@@ -88,15 +96,18 @@ bool GatewayConnectionManager::IsConnected()
 
 bool GatewayConnectionManager::IsLinked()
 {
-  return (s_flags & FLAG_LINKED) != 0;
+  return (s_flags.load(std::memory_order_relaxed) & FLAG_LINKED) != 0;
 }
 
 AccountLinkResultCode GatewayConnectionManager::Link(std::string_view linkCode)
 {
-  if ((s_flags & FLAG_HAS_IP) == 0) {
+  if ((s_flags.load(std::memory_order_relaxed) & FLAG_HAS_IP) == 0) {
     return AccountLinkResultCode::NoInternetConnection;
   }
-  s_wsClient = nullptr;
+  {
+    ScopedLock lock__(&s_clientMutex);
+    s_wsClient = nullptr;
+  }
 
   OS_LOGD(TAG, "Attempting to link to account using code %.*s", linkCode.length(), linkCode.data());
 
@@ -136,20 +147,24 @@ AccountLinkResultCode GatewayConnectionManager::Link(std::string_view linkCode)
     return AccountLinkResultCode::InternalError;
   }
 
-  s_flags |= FLAG_LINKED;
+  s_flags.fetch_or(FLAG_LINKED, std::memory_order_relaxed);
   OS_LOGD(TAG, "Successfully linked to account");
 
   return AccountLinkResultCode::Success;
 }
 void GatewayConnectionManager::UnLink()
 {
-  s_flags &= FLAG_HAS_IP;
-  s_wsClient = nullptr;
+  s_flags.fetch_and(static_cast<uint8_t>(~FLAG_LINKED), std::memory_order_relaxed);
+  {
+    ScopedLock lock__(&s_clientMutex);
+    s_wsClient = nullptr;
+  }
   Config::ClearBackendAuthToken();
 }
 
 bool GatewayConnectionManager::SendMessageTXT(std::string_view data)
 {
+  ScopedLock lock__(&s_clientMutex);
   if (s_wsClient == nullptr) {
     return false;
   }
@@ -159,6 +174,7 @@ bool GatewayConnectionManager::SendMessageTXT(std::string_view data)
 
 bool GatewayConnectionManager::SendMessageBIN(tcb::span<const uint8_t> data)
 {
+  ScopedLock lock__(&s_clientMutex);
   if (s_wsClient == nullptr) {
     return false;
   }
@@ -169,7 +185,7 @@ bool GatewayConnectionManager::SendMessageBIN(tcb::span<const uint8_t> data)
 bool FetchHubInfo(std::string authToken)
 {
   // TODO: this function is very slow, should be optimized!
-  if ((s_flags & FLAG_HAS_IP) == 0) {
+  if ((s_flags.load(std::memory_order_relaxed) & FLAG_HAS_IP) == 0) {
     return false;
   }
 
@@ -181,7 +197,7 @@ bool FetchHubInfo(std::string authToken)
 
   if (response.code == 401) {
     OS_LOGD(TAG, "Auth token is invalid, waiting 5 minutes before checking again");
-    s_lastAuthFailure = OpenShock::micros();
+    s_lastAuthFailure = OpenShock::millis();
     return false;
   }
 
@@ -205,7 +221,7 @@ bool FetchHubInfo(std::string authToken)
     OS_LOGI(TAG, "  [%s] rf=%u model=%u", shocker.id.c_str(), shocker.rfId, shocker.model);
   }
 
-  s_flags |= FLAG_LINKED;
+  s_flags.fetch_or(FLAG_LINKED, std::memory_order_relaxed);
 
   return true;
 }
@@ -245,7 +261,7 @@ bool StartConnectingToLCG()
 
   if (response.code == 401) {
     OS_LOGD(TAG, "Auth token is invalid, waiting 5 minutes before retrying");
-    s_lastAuthFailure = OpenShock::micros();
+    s_lastAuthFailure = OpenShock::millis();
     return false;
   }
 
@@ -270,9 +286,11 @@ bool StartConnectingToLCG()
 
 void GatewayConnectionManager::Update()
 {
+  ScopedLock lock__(&s_clientMutex);
+
   if (s_wsClient == nullptr) {
     // Can't connect to the API without WiFi or an auth token
-    if ((s_flags & FLAG_HAS_IP) == 0 || !Config::HasBackendAuthToken()) {
+    if ((s_flags.load(std::memory_order_relaxed) & FLAG_HAS_IP) == 0 || !Config::HasBackendAuthToken()) {
       return;
     }
 
@@ -287,7 +305,7 @@ void GatewayConnectionManager::Update()
       return;
     }
 
-    s_flags |= FLAG_LINKED;
+    s_flags.fetch_or(FLAG_LINKED, std::memory_order_relaxed);
     OS_LOGD(TAG, "Successfully verified auth token");
 
     s_wsClient = std::make_unique<GatewayClient>(authToken);

src/ReadWriteMutex.cpp

@@ -29,6 +29,7 @@ bool OpenShock::ReadWriteMutex::lockRead(TickType_t xTicksToWait)
 
   if (++m_readers == 1) {
     if (xSemaphoreTake(m_mutex, xTicksToWait) == pdFALSE) {
+      --m_readers;
       xSemaphoreGive(m_readSem);
       return false;
     }

src/VisualStateManager.cpp

@@ -11,6 +11,7 @@ const char* const TAG = "VisualStateManager";
 #include "PinPatternManager.h"
 #include "RGBPatternManager.h"
 
+#include <atomic>
 #include <memory>
 
 #ifndef OPENSHOCK_LED_GPIO
@@ -32,22 +33,22 @@ const uint64_t kWiFiScanningFlag                 = 1 << 7;
 // Bitmask of when the system is running normally
 const uint64_t kStatusOKMask = kWebSocketConnectedFlag | kHasIpAddressFlag | kWiFiConnectedFlag;
 
-static uint64_t s_stateFlags = 0;
+static std::atomic<uint64_t> s_stateFlags = 0;
 static std::shared_ptr<OpenShock::PinPatternManager> s_builtInLedManager;
 static std::shared_ptr<OpenShock::RGBPatternManager> s_RGBLedManager;
 
 inline void _setStateFlag(uint64_t flag, bool state)
 {
   if (state) {
-    s_stateFlags |= flag;
+    s_stateFlags.fetch_or(flag, std::memory_order_relaxed);
   } else {
-    s_stateFlags &= ~flag;
+    s_stateFlags.fetch_and(~flag, std::memory_order_relaxed);
   }
 }
 
 inline bool _isStateFlagSet(uint64_t flag)
 {
-  return s_stateFlags & flag;
+  return s_stateFlags.load(std::memory_order_relaxed) & flag;
 }
 
 using namespace OpenShock;
@@ -384,7 +385,7 @@ bool VisualStateManager::Init()
 
   err = esp_event_handler_register(WIFI_EVENT, ESP_EVENT_ANY_ID, _handleEspWiFiEvent, nullptr);
   if (err != ESP_OK) {
-    OS_LOGE(TAG, "Failed to register event handler for IP_EVENT: %s", esp_err_to_name(err));
+    OS_LOGE(TAG, "Failed to register event handler for WIFI_EVENT: %s", esp_err_to_name(err));
     return false;
   }
 

src/WebSocketDeFragger.cpp

@@ -91,6 +91,7 @@ void WebSocketDeFragger::start(uint8_t socketId, WebSocketMessageType type, cons
   auto it = m_messages.find(socketId);
   if (it != m_messages.end()) {
     it->second.data.assign(data, length);
+    it->second.type = type;
     return;
   }
 

src/captiveportal/Manager.cpp

@@ -17,13 +17,16 @@ const char* const TAG = "CaptivePortal";
 
 #include <esp_timer.h>
 
+#include "SimpleMutex.h"
+
 #include <memory>
 
 using namespace OpenShock;
 
-static bool s_alwaysEnabled                                             = false;
-static bool s_forceClosed                                               = false;
-static esp_timer_handle_t s_captivePortalUpdateLoopTimer                = nullptr;
+static volatile bool s_alwaysEnabled                     = false;
+static volatile bool s_forceClosed                       = false;
+static esp_timer_handle_t s_captivePortalUpdateLoopTimer = nullptr;
+static SimpleMutex s_instanceMutex;
 static std::unique_ptr<CaptivePortal::CaptivePortalInstance> s_instance = nullptr;
 
 static bool captiveportal_start()
@@ -83,6 +86,8 @@ static void captiveportal_updateloop(void*)
   bool commandHandlerOk = CommandHandler::Ok();
   bool shouldBeRunning  = (s_alwaysEnabled || !gatewayConnected || !commandHandlerOk) && !s_forceClosed;
 
+  ScopedLock lock__(&s_instanceMutex);
+
   if (s_instance == nullptr) {
     if (shouldBeRunning) {
       OS_LOGD(TAG, "Starting captive portal");
@@ -166,11 +171,13 @@ bool CaptivePortal::ForceClose(uint32_t timeoutMs)
 
 bool CaptivePortal::IsRunning()
 {
+  ScopedLock lock__(&s_instanceMutex);
   return s_instance != nullptr;
 }
 
 bool CaptivePortal::SendMessageTXT(uint8_t socketId, std::string_view data)
 {
+  ScopedLock lock__(&s_instanceMutex);
   if (s_instance == nullptr) return false;
 
   s_instance->sendMessageTXT(socketId, data);
@@ -179,6 +186,7 @@ bool CaptivePortal::SendMessageTXT(uint8_t socketId, std::string_view data)
 }
 bool CaptivePortal::SendMessageBIN(uint8_t socketId, tcb::span<const uint8_t> data)
 {
+  ScopedLock lock__(&s_instanceMutex);
   if (s_instance == nullptr) return false;
 
   s_instance->sendMessageBIN(socketId, data);
@@ -188,6 +196,7 @@ bool CaptivePortal::SendMessageBIN(uint8_t socketId, tcb::span<const uint8_t> da
 
 bool CaptivePortal::BroadcastMessageTXT(std::string_view data)
 {
+  ScopedLock lock__(&s_instanceMutex);
   if (s_instance == nullptr) return false;
 
   s_instance->broadcastMessageTXT(data);
@@ -196,6 +205,7 @@ bool CaptivePortal::BroadcastMessageTXT(std::string_view data)
 }
 bool CaptivePortal::BroadcastMessageBIN(tcb::span<const uint8_t> data)
 {
+  ScopedLock lock__(&s_instanceMutex);
   if (s_instance == nullptr) return false;
 
   s_instance->broadcastMessageBIN(data);

src/captiveportal/RFC8908Handler.cpp

@@ -85,7 +85,9 @@ void CaptivePortal::RFC8908Handler::handleRequest(AsyncWebServerRequest* request
     cJSON_AddStringToObject(doc, "user-portal-url", portalUrl.c_str());
     cJSON_AddStringToObject(doc, "venue-info-url", "https://openshock.org");
 
-    AsyncWebServerResponse* response = request->beginResponse(200, "application/captive+json", cJSON_Print(doc));
+    char* jsonStr                    = cJSON_Print(doc);
+    AsyncWebServerResponse* response = request->beginResponse(200, "application/captive+json", jsonStr);
+    cJSON_free(jsonStr);
 
     response->addHeader("Cache-Control", "private");
     request->send(response);