From ff548d24815020e56cfedf3dbda23592021318b1 Mon Sep 17 00:00:00 2001 From: mikecarper Date: Sat, 17 Oct 2026 15:56:43 -0700 Subject: [PATCH] Expose read-only RX power-saving diagnostics through framed CLI --- .github/workflows/run-unit-tests.yml | 1 + examples/companion_radio/MyMesh.cpp | 130 +++++++++--------- examples/companion_radio/MyMesh.h | 2 + test/fixtures/companion_rxps_getters.cpp | 159 +++++++++++++++++++++++ test/test_companion_rxps_getters.py | 71 ++++++++++ 5 files changed, 292 insertions(+), 71 deletions(-) create mode 100644 test/fixtures/companion_rxps_getters.cpp create mode 100644 test/test_companion_rxps_getters.py diff --git a/.github/workflows/run-unit-tests.yml b/.github/workflows/run-unit-tests.yml index 4dd9c7ec9..0d0138dca 100644 --- a/.github/workflows/run-unit-tests.yml +++ b/.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 diff --git a/examples/companion_radio/MyMesh.cpp b/examples/companion_radio/MyMesh.cpp index ea1d9c883..bb09b2eb9 100644 --- a/examples/companion_radio/MyMesh.cpp +++ b/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]; diff --git a/examples/companion_radio/MyMesh.h b/examples/companion_radio/MyMesh.h index 3efbc84d3..eb8b19a1a 100644 --- a/examples/companion_radio/MyMesh.h +++ b/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; diff --git a/test/fixtures/companion_rxps_getters.cpp b/test/fixtures/companion_rxps_getters.cpp new file mode 100644 index 000000000..41423ba02 --- /dev/null +++ b/test/fixtures/companion_rxps_getters.cpp @@ -0,0 +1,159 @@ +#include +#include +#include +#include +#include +#include +#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(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(rx), + static_cast(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 guarded(capacity + 16, 0xA7); + char* reply = reinterpret_cast(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"); +} diff --git a/test/test_companion_rxps_getters.py b/test/test_companion_rxps_getters.py new file mode 100644 index 000000000..621aaf0c8 --- /dev/null +++ b/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()