Expose read-only RX power-saving diagnostics through framed CLI

.github/workflows/run-unit-tests.yml

@@ -334,6 +334,7 @@ jobs:
           python3 -B test/test_power_saving_diagnostics.py -v
           python3 -B test/test_power_saving_role_diagnostics.py -v
           python3 -B test/test_companion_power_diagnostics.py -v
+          python3 -B test/test_companion_rxps_getters.py -v
           python3 -B test/test_nrf52_sleep_cycle.py -v
           python3 -B test/test_nrf52_sleep_events.py -v
           python3 -B test/test_nrf52_profile_idle.py -v

examples/companion_radio/MyMesh.cpp

@@ -3846,6 +3846,7 @@ bool MyMesh::handleLocalControlCommand(const char* command, char* reply,
 #endif
   if (handleCompanionBluetoothCommand(command, reply, reply_size)) return true;
   if (handleCompanionWirelessCommand(command, reply, reply_size)) return true;
+  if (handleRxPowerSavingGetCommand(command, reply, reply_size)) return true;
 
   if (strcmp(command, "get powersaving") == 0 || strcmp(command, "powersaving") == 0) {
     snprintf(reply, reply_size, "> %s", _prefs.powersaving_enabled ? "on" : "off");
@@ -5214,17 +5215,6 @@ void MyMesh::execAdminCommand(char* cmd, char* reply) {
     return;
   }
   if (handleCommand(cmd, 0, reply)) return;
-  if (strcmp(cmd, "get radio.rxps") == 0 || strcmp(cmd, "get radio.rxps.config") == 0) {
-    if (!radio_driver.supportsRxPowerSaving()) {
-      strcpy(reply, "Error: RX power saving is unsupported on this radio");
-    } else {
-      snprintf(reply, 160, "> %s,level=%u,preamble=%u,rx=%lu,sleep=%lu",
-               _prefs.rx_powersaving_enabled ? "on" : "off",
-               (unsigned)_prefs.rx_ps_level, (unsigned)_prefs.rx_ps_preamble,
-               (unsigned long)_prefs.rx_ps_rx_us, (unsigned long)_prefs.rx_ps_sleep_us);
-    }
-    return;
-  }
   // The settings form already implements the Companion's MQTT and remaining
   // configuration setters. Its batch hooks persist/restart MQTT once per batch.
   execCommand(cmd, reply);
@@ -8796,6 +8786,56 @@ bool MyMesh::applyAndSaveWiFiPowerSaving(const char* value, char* reply,
 }
 #endif
 
+bool MyMesh::handleRxPowerSavingGetCommand(const char* command, char* reply,
+                                           size_t reply_size) const {
+  if (!command || !reply || reply_size == 0) return false;
+  const bool config = strcmp(command, "get radio.rxps.config") == 0;
+  if (!config && strcmp(command, "get radio.rxps") != 0) return false;
+  if (!radio_driver.supportsRxPowerSaving()) {
+    snprintf(reply, reply_size, "Error: RX power saving is unsupported on this radio");
+    return true;
+  }
+
+  uint32_t rx_us = _prefs.rx_ps_rx_us;
+  uint32_t sleep_us = _prefs.rx_ps_sleep_us;
+  uint8_t effective_level = 0;
+  uint8_t effective_preamble = 0;
+  if (!config && _prefs.rx_ps_level != 0) {
+    uint8_t sf = _prefs.sf;
+    float bw = _prefs.bw;
+#if COMPANION_FEATURE_TEMP_RADIO
+    if (isTempRadioActive()) {
+      sf = _temp_radio_sf;
+      bw = _temp_radio_bw;
+    }
+#endif
+    // Calculate into locals only. A diagnostic must not change saved timings,
+    // apply radio parameters, or create a pending preference write.
+    recalcRxPowerSavingFromLevel(_prefs.rx_ps_level, sf, bw,
+        _prefs.rx_ps_preamble, &rx_us, &sleep_us,
+        &effective_level, &effective_preamble);
+  }
+  snprintf(reply, reply_size, "> %s,level=%u,preamble=%u,rx=%lu,sleep=%lu%s",
+           _prefs.rx_powersaving_enabled ? "on" : "off",
+           (unsigned)_prefs.rx_ps_level, (unsigned)_prefs.rx_ps_preamble,
+           (unsigned long)rx_us, (unsigned long)sleep_us,
+           !config && radio_driver.isRxPowerSavingContinuousFallback()
+               ? ",mode=continuous-fast" : "");
+  size_t used = strlen(reply);
+  if (!config && effective_level != 0 && effective_level != _prefs.rx_ps_level
+      && used < reply_size - 1) {
+    snprintf(reply + used, reply_size - used, ",effective-level=%u",
+             (unsigned)effective_level);
+  }
+  used = strlen(reply);
+  if (!config && effective_preamble != 0 && effective_preamble != _prefs.rx_ps_preamble
+      && used < reply_size - 1) {
+    snprintf(reply + used, reply_size - used, ",effective-preamble=%u",
+             (unsigned)effective_preamble);
+  }
+  return true;
+}
+
 bool MyMesh::applyAndSaveRxPowerSaving(const char* value, char* reply) {
   if (!radio_driver.supportsRxPowerSaving()) {
     strcpy(reply, "Error: RX power saving unsupported");
@@ -9753,6 +9793,14 @@ void MyMesh::handleTerminalCommand(char* command, bool reader_prompt,
   // Match the repeater ASCII reply envelope so unmodified serial packet
   // bridges can query this role. Framed/BLE replies retain their payloads.
   char local_reply[160];
+  if (handleRxPowerSavingGetCommand(command, local_reply, sizeof(local_reply))) {
+    if (local_reply[0] == '>') {
+      terminalOutput().printf("  %s %s\r\n", command + 4, local_reply + 2);
+    } else {
+      terminalOutput().printf("  ERROR: %s\r\n", local_reply + 7);
+    }
+    return;
+  }
   if (handleCompanionWirelessCommand(command, local_reply, sizeof(local_reply),
                                      _terminal_mode ? CompanionWirelessSource::Usb : CompanionWirelessSource::Network)
       || handleCompanionBluetoothCommand(command, local_reply, sizeof(local_reply),
@@ -10012,66 +10060,6 @@ void MyMesh::handleTerminalCommand(char* command, bool reader_prompt,
     char reply[160];
     applyAndSavePowerSaving(command + 12, reply);
     terminalOutput().printf("  -> %s\r\n", reply);
-  } else if (strcmp(command, "get radio.rxps.config") == 0) {
-    if (!radio_driver.supportsRxPowerSaving()) {
-      terminalOutput().print("  ERROR: RX power saving is unsupported on this radio\r\n");
-    } else {
-      terminalOutput().printf(
-          "  radio.rxps.config %s,level=%u,preamble=%u,rx=%lu,sleep=%lu\r\n",
-          _prefs.rx_powersaving_enabled ? "on" : "off",
-          (unsigned)_prefs.rx_ps_level,
-          (unsigned)_prefs.rx_ps_preamble,
-          (unsigned long)_prefs.rx_ps_rx_us,
-          (unsigned long)_prefs.rx_ps_sleep_us);
-    }
-  } else if (strcmp(command, "get radio.rxps") == 0) {
-    if (!radio_driver.supportsRxPowerSaving()) {
-      terminalOutput().print("  ERROR: RX power saving is unsupported on this radio\r\n");
-    } else {
-      uint8_t effective_level = 0;
-      uint8_t effective_preamble = 0;
-      uint32_t effective_rx_us = _prefs.rx_ps_rx_us;
-      uint32_t effective_sleep_us = _prefs.rx_ps_sleep_us;
-      uint8_t active_sf = _prefs.sf;
-      float active_bw = _prefs.bw;
-#if COMPANION_FEATURE_TEMP_RADIO
-      if (isTempRadioActive()) {
-        active_sf = _temp_radio_sf;
-        active_bw = _temp_radio_bw;
-      }
-#endif
-      if (_prefs.rx_ps_level != 0) {
-        recalcRxPowerSavingFromLevel(
-            _prefs.rx_ps_level, active_sf, active_bw,
-            _prefs.rx_ps_preamble, &effective_rx_us, &effective_sleep_us,
-            &effective_level, &effective_preamble);
-      }
-      char effective_suffix[48] = {};
-      const bool adjusted_level = effective_level != 0
-          && effective_level != _prefs.rx_ps_level;
-      const bool adjusted_preamble = effective_preamble != 0
-          && effective_preamble != _prefs.rx_ps_preamble;
-      if (adjusted_level && adjusted_preamble) {
-        snprintf(effective_suffix, sizeof(effective_suffix),
-                 ",effective-level=%u,effective-preamble=%u",
-                 (unsigned)effective_level, (unsigned)effective_preamble);
-      } else if (adjusted_level) {
-        snprintf(effective_suffix, sizeof(effective_suffix),
-                 ",effective-level=%u", (unsigned)effective_level);
-      } else if (adjusted_preamble) {
-        snprintf(effective_suffix, sizeof(effective_suffix),
-                 ",effective-preamble=%u", (unsigned)effective_preamble);
-      }
-      terminalOutput().printf("  radio.rxps %s,level=%u,preamble=%u,rx=%lu,sleep=%lu%s%s\r\n",
-                    _prefs.rx_powersaving_enabled ? "on" : "off",
-                    (unsigned)_prefs.rx_ps_level,
-                    (unsigned)_prefs.rx_ps_preamble,
-                    (unsigned long)effective_rx_us,
-                    (unsigned long)effective_sleep_us,
-                    radio_driver.isRxPowerSavingContinuousFallback()
-                        ? ",mode=continuous-fast" : "",
-                    effective_suffix);
-    }
 #if defined(ESP32) && defined(WIFI_SSID)
   } else if (strcmp(command, "get wifi.powersave") == 0) {
     char reply[160];

examples/companion_radio/MyMesh.h

@@ -665,6 +665,8 @@ private:
   void formatEspNowChannel(char* reply, size_t reply_size) const;
 #endif
   bool applyAndSavePowerSaving(const char* value, char* reply);
+  bool handleRxPowerSavingGetCommand(const char* command, char* reply,
+                                      size_t reply_size) const;
   bool applyAndSaveRxPowerSaving(const char* value, char* reply);
   void appendRxPowerSavingAdjustmentNote(char* reply, size_t reply_size,
                                          uint8_t sf, float bw) const;

test/fixtures/companion_rxps_getters.cpp

@@ -0,0 +1,159 @@
+#include <assert.h>
+#include <stdarg.h>
+#include <stdio.h>
+#include <string.h>
+#include <string>
+#include <vector>
+#include "helpers/radiolib/RXPowerSaving.h"
+
+struct Radio {
+  bool supported = true;
+  bool fallback = false;
+  unsigned fallback_queries = 0;
+  bool supportsRxPowerSaving() const { return supported; }
+  bool isRxPowerSavingContinuousFallback() { ++fallback_queries; return fallback; }
+  // No mutating radio APIs exist in this fixture: a write cannot compile.
+} radio_driver;
+
+struct Prefs {
+  uint8_t rx_powersaving_enabled = 1;
+  uint8_t rx_ps_level = 8;
+  uint8_t rx_ps_preamble = 16;
+  uint32_t rx_ps_rx_us = 11111;
+  uint32_t rx_ps_sleep_us = 22222;
+  uint8_t sf = 7;
+  float bw = 62.5;
+};
+
+struct Output {
+  std::string value;
+  void printf(const char* format, ...) {
+    char result[512];
+    va_list args;
+    va_start(args, format);
+    const int count = vsnprintf(result, sizeof(result), format, args);
+    va_end(args);
+    assert(count >= 0 && static_cast<size_t>(count) < sizeof(result));
+    value += result;
+  }
+};
+
+struct MyMesh {
+  Prefs _prefs;
+  bool temporary = false;
+  uint8_t _temp_radio_sf = 5;
+  float _temp_radio_bw = 500;
+  Output output;
+  bool isTempRadioActive() const { return temporary; }
+  bool handleRxPowerSavingGetCommand(const char*, char*, size_t) const;
+  bool handleLocalControlCommand(const char*, char*, size_t);
+  Output& terminalOutput() { return output; }
+  void ascii(const char* command) {
+    char local_reply[160];
+#include "production_ascii.inc"
+    assert(false && "Fixture expects one recognized ASCII RXPS getter");
+  }
+};
+
+#include "production_getters.inc"
+
+static std::string expected(const MyMesh& mesh, bool config) {
+  if (!radio_driver.supported) return "Error: RX power saving is unsupported on this radio";
+  const Prefs& prefs = mesh._prefs;
+  uint32_t rx = prefs.rx_ps_rx_us, sleep = prefs.rx_ps_sleep_us;
+  uint8_t level = 0, preamble = 0;
+  if (!config && prefs.rx_ps_level != 0) {
+    uint8_t sf = prefs.sf;
+    float bw = prefs.bw;
+#if COMPANION_FEATURE_TEMP_RADIO
+    if (mesh.temporary) { sf = mesh._temp_radio_sf; bw = mesh._temp_radio_bw; }
+#endif
+    recalcRxPowerSavingFromLevel(prefs.rx_ps_level, sf, bw, prefs.rx_ps_preamble,
+                                &rx, &sleep, &level, &preamble);
+  }
+  char result[256];
+  snprintf(result, sizeof(result), "> %s,level=%u,preamble=%u,rx=%lu,sleep=%lu%s",
+           prefs.rx_powersaving_enabled ? "on" : "off", unsigned(prefs.rx_ps_level),
+           unsigned(prefs.rx_ps_preamble), static_cast<unsigned long>(rx),
+           static_cast<unsigned long>(sleep),
+           !config && radio_driver.fallback ? ",mode=continuous-fast" : "");
+  std::string out = result;
+  if (!config && level && level != prefs.rx_ps_level) out += ",effective-level=" + std::to_string(level);
+  if (!config && preamble && preamble != prefs.rx_ps_preamble) out += ",effective-preamble=" + std::to_string(preamble);
+  return out;
+}
+
+static void check(MyMesh& mesh, bool config, bool all_capacities) {
+  const char* command = config ? "get radio.rxps.config" : "get radio.rxps";
+  const std::string wanted = expected(mesh, config);
+  const Prefs saved = mesh._prefs;
+  const unsigned before = radio_driver.fallback_queries;
+  for (size_t capacity = all_capacities ? 1 : 160; capacity <= 180; ++capacity) {
+    std::vector<unsigned char> guarded(capacity + 16, 0xA7);
+    char* reply = reinterpret_cast<char*>(guarded.data() + 8);
+    assert(mesh.handleLocalControlCommand(command, reply, capacity));
+    const size_t count = wanted.size() < capacity - 1 ? wanted.size() : capacity - 1;
+    assert(strlen(reply) == count && !memcmp(reply, wanted.data(), count));
+    for (size_t i = 0; i < 8; ++i) assert(guarded[i] == 0xA7);
+    for (size_t i = 8 + capacity; i < guarded.size(); ++i) assert(guarded[i] == 0xA7);
+    assert(memcmp(&mesh._prefs, &saved, sizeof(saved)) == 0);
+  }
+  if (config || !radio_driver.supported) assert(radio_driver.fallback_queries == before);
+  mesh.output.value.clear();
+  mesh.ascii(command);
+  const std::string ascii = radio_driver.supported
+      ? "  " + std::string(command + 4) + " " + wanted.substr(2) + "\r\n"
+      : "  ERROR: RX power saving is unsupported on this radio\r\n";
+  assert(mesh.output.value == ascii);
+  assert(memcmp(&mesh._prefs, &saved, sizeof(saved)) == 0);
+}
+
+int main() {
+  MyMesh mesh;
+  for (uint8_t enabled = 0; enabled <= 1; ++enabled) {
+    for (uint8_t level = 0; level <= 10; ++level) {
+      for (uint8_t preamble : {uint8_t(0), uint8_t(16), uint8_t(32)}) {
+        for (bool temporary : {false, true}) {
+          mesh._prefs.rx_powersaving_enabled = enabled;
+          mesh._prefs.rx_ps_level = level;
+          mesh._prefs.rx_ps_preamble = preamble;
+          mesh.temporary = temporary;
+          radio_driver.fallback = temporary;
+          check(mesh, false, level == 8);
+          check(mesh, true, level == 8);
+        }
+      }
+    }
+  }
+  // Config must return actual stored timings, never a recalculated pair. The
+  // effective getter follows TempRadio only when compiled and currently active.
+  mesh._prefs = Prefs();
+  mesh.temporary = false;
+  const std::string normal = expected(mesh, false);
+  mesh.temporary = true;
+  const std::string temporary = expected(mesh, false);
+#if COMPANION_FEATURE_TEMP_RADIO
+  assert(normal != temporary);
+#else
+  assert(normal == temporary);
+#endif
+  check(mesh, false, true);
+  check(mesh, true, true);
+  radio_driver.supported = false;
+  check(mesh, false, true);
+  check(mesh, true, true);
+  radio_driver.supported = true;
+  char reply[160];
+  memset(reply, 'X', sizeof(reply));
+  assert(!mesh.handleRxPowerSavingGetCommand(nullptr, reply, sizeof(reply)));
+  assert(!mesh.handleRxPowerSavingGetCommand("get radio.rxps", nullptr, sizeof(reply)));
+  assert(!mesh.handleRxPowerSavingGetCommand("get radio.rxps", reply, 0));
+  for (const char* command : {"", "get radio.rxps ", "get radio.rxps.extra", "get radio.rxps.configx",
+       "get radio.rxps.config extra", "set radio.rxps on", "GET radio.rxps"}) {
+    assert(!mesh.handleLocalControlCommand(command, reply, sizeof(reply)));
+    for (char byte : reply) assert(byte == 'X');
+  }
+  assert(mesh.handleLocalControlCommand("   get radio.rxps.config", reply, sizeof(reply)));
+  assert(std::string(reply) == expected(mesh, true));
+  puts("RXPS getter contracts passed");
+}

test/test_companion_rxps_getters.py

@@ -0,0 +1,71 @@
+#!/usr/bin/env python3
+"""Exercise the production shared RXPS getters and ASCII adapter under sanitizers."""
+
+from pathlib import Path
+import os
+import subprocess
+import tempfile
+import unittest
+
+from test_replay_reset_integration import extract_braced
+
+
+ROOT = Path(__file__).resolve().parents[1]
+
+
+class CompanionRxpsGetterTests(unittest.TestCase):
+    def test_real_getters_are_bounded_readonly_and_match_active_primary(self):
+        source = (ROOT / "examples/companion_radio/MyMesh.cpp").read_text(encoding="ascii")
+        getter = extract_braced(source, "bool MyMesh::handleRxPowerSavingGetCommand(")
+        local = extract_braced(source, "bool MyMesh::handleLocalControlCommand(")
+        signature = "  if (handleRxPowerSavingGetCommand(command, reply, reply_size)) return true;"
+        self.assertIn(signature, local)
+        # Keep the real local entry validation/space handling and registration;
+        # unrelated setters/transport helpers are not part of this fixture.
+        entry = local[:local.index("  if (mesh::powerdiag::handleCommand")]
+        entry += signature + "\n  return false;\n}\n"
+        terminal = extract_braced(source, "void MyMesh::handleTerminalCommand(")
+        ascii_adapter = extract_braced(terminal, "if (handleRxPowerSavingGetCommand(command, local_reply, sizeof(local_reply)))")
+        with tempfile.TemporaryDirectory(prefix="meshcore-companion-rxps-get-") as directory:
+            work = Path(directory)
+            (work / "production_getters.inc").write_text(getter + "\n" + entry, encoding="ascii")
+            (work / "production_ascii.inc").write_text(ascii_adapter, encoding="ascii")
+            for platform in ("NRF52_PLATFORM", "ESP32_PLATFORM", "STM32_PLATFORM"):
+                for temporary in (0, 1):
+                    with self.subTest(platform=platform, temporary=temporary):
+                        binary = work / (platform + str(temporary))
+                        built = subprocess.run([
+                            os.environ.get("CXX", "c++"), "-std=c++17", "-Wall", "-Wextra", "-Werror",
+                            "-fsanitize=address,undefined", "-fno-sanitize-recover=all", "-fno-pie", "-no-pie",
+                            "-D" + platform + "=1", "-DCOMPANION_FEATURE_TEMP_RADIO=" + str(temporary),
+                            "-I", str(ROOT / "src"), "-isystem", str(ROOT / "test/mocks"), "-I", str(work),
+                            str(ROOT / "test/fixtures/companion_rxps_getters.cpp"), "-o", str(binary),
+                        ], capture_output=True, text=True, timeout=60)
+                        self.assertEqual(built.returncode, 0, built.stdout + built.stderr)
+                        ran = subprocess.run([str(binary)], capture_output=True, text=True, timeout=20)
+                        self.assertEqual(ran.returncode, 0, ran.stdout + ran.stderr)
+                        self.assertIn("RXPS getter contracts passed", ran.stdout)
+
+    def test_framed_admin_and_ascii_share_the_same_getter(self):
+        source = (ROOT / "examples/companion_radio/MyMesh.cpp").read_text(encoding="ascii")
+        local = extract_braced(source, "bool MyMesh::handleLocalControlCommand(")
+        self.assertIn("handleRxPowerSavingGetCommand(command, reply, reply_size)", local)
+        shared = extract_braced(source, "bool MyMesh::handleCommand(")
+        self.assertIn("handleLocalControlCommand(command, reply, reply_capacity)", shared)
+        framed = source[source.index("} else if (mesh::companion::isRunCliFrame("):]
+        self.assertIn("handled = handleCommand(text, 0, reply_buf);", framed)
+        admin = extract_braced(source, "void MyMesh::execAdminCommand(")
+        self.assertIn("if (handleCommand(cmd, 0, reply)) return;", admin)
+        self.assertNotIn("get radio.rxps", admin)
+        terminal = extract_braced(source, "void MyMesh::handleTerminalCommand(")
+        self.assertLess(terminal.index("handleRxPowerSavingGetCommand("), terminal.index("handleLocalControlCommand("))
+        self.assertNotIn("recalcRxPowerSavingFromLevel", terminal)
+        helper = extract_braced(source, "bool MyMesh::handleRxPowerSavingGetCommand(")
+        self.assertNotIn("savePrefs", helper)
+        self.assertNotIn("setRxPowerSaving", helper)
+        self.assertNotIn("ota_", helper)
+        self.assertNotIn("_prefs.rx_ps_rx_us =", helper)
+
+
+if __name__ == "__main__":
+    unittest.main()