fix: review feedback — volatile cross-task globals, atomic mask replace

Code review on PR decentespresso#54 caught two real bugs in the deferred-action fix:

1. The pending-mask let opposing actions accumulate. If a client sent
   "display off" then "display on" before the main loop drained, both
   bits were set and processWsPendingCmds ran them in source order —
   final hardware ended up off while b_u8g2Sleep reported on. Add a
   wsReplacePending(set, clear) helper that atomically supersedes
   the opposing bit, and use it for the three mutually-exclusive
   pairs (display, low_power, sleep).

2. The new WS state globals (weightWebsocketNotifyInterval,
   b_websocketEventsEnabled, b_websocketLowPower/LedEnabled,
   i_websocketLedR/G/B, t_lastWebsocketStatusUpdate) and the pre-
   existing b_softSleep / b_u8g2Sleep / b_powerOff that are now
   written from the AsyncTCP task are missing `volatile`. On dual-
   core ESP32-S3 the compiler may cache stale values in a register
   on the loop core across the WS gate check. Mark them volatile.

Also fold in smaller doc/format cleanups from the same review:
  * Cast stopWatch.elapsed() (uint32_t) to (unsigned long) for the
    %lu format spec in sendWebsocketStatus / sendWebsocketStatusAll.
    Works today on ESP32 (both 32-bit) but technically a type
    mismatch.
  * Correct the wsQueuePending doc-comment (StopWatch is safe to
    mutate inline; only u8g2 and power-rail GPIOs need deferring).
  * Correct the processWsPendingCmds comment to note that
    b_powerOff is set here on purpose.
  * README: document the JSON rate forms accepted by the parser
    that weren't listed (rate_hz, hz, interval_ms).

Re-verified:
  * 60/60 WS feature regression PASS
  * 120s freeze stress: max weight gap 0.60s (was 4.18s pre-fix),
    0 HTTP failures, 0 slow, 590 commands sent

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Author: skialpine

README.md

@@ -63,11 +63,14 @@ rate 5k
 rate 10k
 ```
 
-or JSON:
+or JSON (any of these forms work):
 
 ```json
 { "command": "rate", "value": "10k" }
 { "rate": "10k" }
+{ "rate_hz": 10 }
+{ "hz": 10 }
+{ "interval_ms": 100 }
 ```
 
 The supported rates are 2 Hz, 5 Hz, and 10 Hz. The firmware sends back a

include/parameter.h

@@ -17,14 +17,17 @@ const unsigned long WEBSOCKET_5HZ_NOTIFY_INTERVAL_MS = 200;
 const unsigned long WEBSOCKET_10HZ_NOTIFY_INTERVAL_MS = 100;
 const unsigned long WEBSOCKET_DEFAULT_NOTIFY_INTERVAL_MS = WEBSOCKET_2HZ_NOTIFY_INTERVAL_MS;
 const unsigned long WEBSOCKET_STATUS_NOTIFY_INTERVAL_MS = 5000;
-unsigned long weightWebsocketNotifyInterval = WEBSOCKET_DEFAULT_NOTIFY_INTERVAL_MS;
-bool b_websocketEventsEnabled = false;
-bool b_websocketLowPowerEnabled = false;
-bool b_websocketLedEnabled = false;
-uint8_t i_websocketLedR = 0;
-uint8_t i_websocketLedG = 0;
-uint8_t i_websocketLedB = 0;
-unsigned long t_lastWebsocketStatusUpdate = 0;
+// volatile: written from the AsyncTCP task (WS event callback) and read
+// from the main loop. Without volatile, the compiler may keep these cached
+// in a register across the loop's WS gate check on the other core.
+volatile unsigned long weightWebsocketNotifyInterval = WEBSOCKET_DEFAULT_NOTIFY_INTERVAL_MS;
+volatile bool b_websocketEventsEnabled = false;
+volatile bool b_websocketLowPowerEnabled = false;
+volatile bool b_websocketLedEnabled = false;
+volatile uint8_t i_websocketLedR = 0;
+volatile uint8_t i_websocketLedG = 0;
+volatile uint8_t i_websocketLedB = 0;
+volatile unsigned long t_lastWebsocketStatusUpdate = 0;
 
 // Websocket pending-command mask. Set on the AsyncTCP task by the WS event
 // callback; drained on the main loop. Defers hardware-touching ops (u8g2,
@@ -65,7 +68,8 @@ bool b_debug = false;
 
 unsigned long t_batteryIcon = 0;
 bool b_showBatteryIcon = true;
-bool b_softSleep = false;
+// volatile: now also written from the AsyncTCP task (WS soft-sleep command).
+volatile bool b_softSleep = false;
 #if defined(ACC_MPU6050) || defined(ACC_BMA400)
 bool b_gyroEnabled = true;
 #endif
@@ -91,7 +95,8 @@ bool b_chargingOLED = true;
 bool b_heartBeatIcon = false; //debug ble heart icon
 unsigned long t_shutdownFailBle = 0;  //for popping up shut down fail due to ble is connected.
 bool b_shutdownFailBle = false;
-bool b_u8g2Sleep = true;
+// volatile: now also written from the AsyncTCP task (WS display/sleep commands).
+volatile bool b_u8g2Sleep = true;
 unsigned long t_bootTare = 0;
 bool b_bootTare = false;
 int i_bootTareDelay = 1000;
@@ -106,7 +111,9 @@ bool b_tareByBle = false;
 portMUX_TYPE remoteTareMux = portMUX_INITIALIZER_UNLOCKED;
 unsigned long t_tareStatus = 0;  //tare done time stamp
 unsigned long t_power_off;       //关机倒计时
-bool b_powerOff = false;
+// volatile: set in processWsPendingCmds (main loop) and read in loop's top
+// guard; main loop also writes it from other paths. Keep volatile defensively.
+volatile bool b_powerOff = false;
 #if defined(ACC_MPU6050) || defined(ACC_BMA400)
 unsigned long t_power_off_gyro = 0;  //侧放关机倒计时
 #endif

src/hds.ino

@@ -485,24 +485,37 @@ bool parseWebsocketRgb(String input, uint8_t &r, uint8_t &g, uint8_t &b) {
 
 // Queue a pending hardware action so the main loop performs it on its own
 // task. The WS event callback runs on the AsyncTCP task and must not touch
-// u8g2, stopWatch, or power-rail GPIOs directly.
+// u8g2 (I2C bus) or peripheral power-rail GPIOs directly. StopWatch and
+// plain bool/uint32 flag writes are safe to do inline in the callback.
 inline void wsQueuePending(uint32_t bits) {
   portENTER_CRITICAL(&wsPendingMux);
   wsPendingMask |= bits;
   portEXIT_CRITICAL(&wsPendingMux);
 }
 
+// Atomically set `setBits` and clear `clearBits` in the pending mask. Used
+// for mutually-exclusive action pairs (DISPLAY_ON/OFF, SLEEP_ON/OFF, ...) so
+// that if a previous opposing action is still queued, it's superseded by
+// the new one rather than running both in source order.
+inline void wsReplacePending(uint32_t setBits, uint32_t clearBits) {
+  portENTER_CRITICAL(&wsPendingMux);
+  wsPendingMask = (wsPendingMask & ~clearBits) | setBits;
+  portEXIT_CRITICAL(&wsPendingMux);
+}
+
 void processWsPendingCmds() {
   portENTER_CRITICAL(&wsPendingMux);
   uint32_t mask = wsPendingMask;
   wsPendingMask = 0;
   portEXIT_CRITICAL(&wsPendingMux);
   if (mask == 0) return;
 
-  // Hardware ops only — touching u8g2 (I2C) or peripheral power rails from
-  // the AsyncTCP task can race the main loop. State flags (b_u8g2Sleep,
-  // b_softSleep, etc.) are already updated synchronously in the WS callback
-  // so status frames stay accurate.
+  // u8g2 (I2C) and peripheral power-rail writes happen here because they
+  // would race the main loop if called from the AsyncTCP task. State flags
+  // touched by the user-visible status (b_u8g2Sleep, b_softSleep, ...) are
+  // updated synchronously in the WS callback so the response is accurate;
+  // b_powerOff is set here so it sequences after the centralized shutdown
+  // chain in shut_down_now_nobeep().
   if (mask & WSP_DISPLAY_ON)  { u8g2.setPowerSave(0); }
   if (mask & WSP_DISPLAY_OFF) { u8g2.setPowerSave(1); }
   if (mask & WSP_LOWPWR_ON)   { u8g2.setContrast(0); }
@@ -571,7 +584,7 @@ void sendWebsocketStatus(AsyncWebSocketClient *client, const char *status) {
                  f_batteryVoltage,
                  websocketIsCharging() ? "true" : "false",
                  stopWatch.isRunning() ? "true" : "false",
-                 stopWatch.elapsed(),
+                 (unsigned long)stopWatch.elapsed(),
                  b_u8g2Sleep ? "false" : "true",
                  b_websocketLowPowerEnabled ? "true" : "false",
                  b_softSleep ? "true" : "false",
@@ -595,7 +608,7 @@ void sendWebsocketStatusAll(const char *status) {
                       f_batteryVoltage,
                       websocketIsCharging() ? "true" : "false",
                       stopWatch.isRunning() ? "true" : "false",
-                      stopWatch.elapsed(),
+                      (unsigned long)stopWatch.elapsed(),
                       b_u8g2Sleep ? "false" : "true",
                       b_websocketLowPowerEnabled ? "true" : "false",
                       b_softSleep ? "true" : "false",
@@ -738,14 +751,14 @@ bool handleWebsocketControlCommand(AsyncWebSocketClient *client, String command,
     if (action == "on") {
       Serial.println("Websocket LED/display on detected.");
       b_u8g2Sleep = false;
-      wsQueuePending(WSP_DISPLAY_ON);
+      wsReplacePending(WSP_DISPLAY_ON, WSP_DISPLAY_OFF);
       sendWebsocketStatus(client, "ok");
       return true;
     }
     if (action == "off") {
       Serial.println("Websocket display off detected.");
       b_u8g2Sleep = true;
-      wsQueuePending(WSP_DISPLAY_OFF);
+      wsReplacePending(WSP_DISPLAY_OFF, WSP_DISPLAY_ON);
       sendWebsocketStatus(client, "ok");
       return true;
     }
@@ -757,14 +770,14 @@ bool handleWebsocketControlCommand(AsyncWebSocketClient *client, String command,
     if (action == "on") {
       Serial.println("Websocket low power mode on detected.");
       b_websocketLowPowerEnabled = true;
-      wsQueuePending(WSP_LOWPWR_ON);
+      wsReplacePending(WSP_LOWPWR_ON, WSP_LOWPWR_OFF);
       sendWebsocketStatus(client, "ok");
       return true;
     }
     if (action == "off") {
       Serial.println("Websocket low power mode off detected.");
       b_websocketLowPowerEnabled = false;
-      wsQueuePending(WSP_LOWPWR_OFF);
+      wsReplacePending(WSP_LOWPWR_OFF, WSP_LOWPWR_ON);
       sendWebsocketStatus(client, "ok");
       return true;
     }
@@ -778,15 +791,15 @@ bool handleWebsocketControlCommand(AsyncWebSocketClient *client, String command,
       // Set state flags synchronously so status reflects the requested mode.
       // u8g2 + GPIO power-rail writes are deferred to the main loop.
       b_softSleep = true;
-      wsQueuePending(WSP_SLEEP_ON);
+      wsReplacePending(WSP_SLEEP_ON, WSP_SLEEP_OFF);
       sendWebsocketStatus(client, "ok");
       return true;
     }
     if (action == "off" || action == "wake") {
       Serial.println("Websocket soft sleep off detected.");
       b_softSleep = false;
       b_u8g2Sleep = false;
-      wsQueuePending(WSP_SLEEP_OFF);
+      wsReplacePending(WSP_SLEEP_OFF, WSP_SLEEP_ON);
       sendWebsocketStatus(client, "ok");
       return true;
     }