diff --git a/docs/companion_radio_full.md b/docs/companion_radio_full.md index d6acc4f4d..0a20daa7a 100644 --- a/docs/companion_radio_full.md +++ b/docs/companion_radio_full.md @@ -488,8 +488,8 @@ three values, ESP-NOW + WiFi accepts `none|min`, LoRa + BLE accepts `min|max` for the inactive WiFi setting, and ESP-NOW + BLE requires `min`. Fresh Cascade builds select `min`, and an existing valid saved selection takes precedence. -On radios with RX duty-cycle support, WebConfig and the text terminal also -expose the persisted RXPS setting: +On radios with RX duty-cycle support, the app's device console, WebConfig, +and the text terminal expose the persisted RXPS setting: ```text get radio.rxps @@ -503,6 +503,11 @@ Fresh Cascade-profile Full Companion builds start with RXPS on at level 8 and a 16-symbol preamble. Changing it takes effect immediately and remains selected after reboot. +Older Companion images may reply `unknown command` to `set radio.rxps` in the +app console. Use the USB text terminal or WebConfig on those images, or update +the firmware. Radio 2 suspends RXPS while it is active; the saved RXPS setting +still applies when Radio 2 is disabled. + Companion firmware defaults device power saving to on. Version 1.17.1.2 also turns it on once when upgrading an older Companion preference file, including one written by the short-lived default-off regression. After that one-time diff --git a/examples/companion_radio/MyMesh.cpp b/examples/companion_radio/MyMesh.cpp index bdb467056..b16122bdc 100644 --- a/examples/companion_radio/MyMesh.cpp +++ b/examples/companion_radio/MyMesh.cpp @@ -4057,6 +4057,15 @@ bool MyMesh::handleLocalControlCommand(const char* command, char* reply, 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 (strncmp(command, "set radio.rxps", 14) == 0 + && (command[14] == 0 || command[14] == ' ' || command[14] == '\t')) { + const char* value = command + 14; + while (*value == ' ' || *value == '\t') ++value; + char result[160]; + applyAndSaveRxPowerSaving(value, result); + snprintf(reply, reply_size, "%s", result); + return true; + } if (mesh::companion::handleRadioDiagnosticsCommand( radio_driver, _radio_available, command, reply, reply_size, [this]() { radio_driver.resetStats(); @@ -10385,10 +10394,6 @@ void MyMesh::handleTerminalCommand(char* command, bool reader_prompt, char reply[160]; applyAndSavePowerSaving(config + 12, reply); terminalOutput().printf(" -> %s\r\n", reply); - } else if (strncmp(config, "radio.rxps ", 11) == 0) { - char reply[160]; - applyAndSaveRxPowerSaving(config + 11, reply); - terminalOutput().printf(" -> %s\r\n", reply); #if defined(ESP32) && defined(WIFI_SSID) } else if (strncmp(config, "wifi.powersave", 14) == 0 && (config[14] == 0 || config[14] == ' ' diff --git a/test/fixtures/companion_rxps_setters.cpp b/test/fixtures/companion_rxps_setters.cpp new file mode 100644 index 000000000..ce62f9eac --- /dev/null +++ b/test/fixtures/companion_rxps_setters.cpp @@ -0,0 +1,168 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +struct Prefs { + uint8_t rx_powersaving_enabled = 0, rx_ps_level = 8, rx_ps_preamble = 16, sf = 7; + uint32_t rx_ps_rx_us = 3000, rx_ps_sleep_us = 4000; + float bw = 125; +}; +static bool same(const Prefs& a, const Prefs& b) { + return a.rx_powersaving_enabled == b.rx_powersaving_enabled + && a.rx_ps_level == b.rx_ps_level && a.rx_ps_preamble == b.rx_ps_preamble + && a.rx_ps_rx_us == b.rx_ps_rx_us && a.rx_ps_sleep_us == b.rx_ps_sleep_us + && a.sf == b.sf && a.bw == b.bw; +} +struct Driver { + bool supported = true, enabled = false; + unsigned calls = 0; + uint32_t rx = 3000, sleep = 4000; + std::deque results; + bool supportsRxPowerSaving() const { return supported; } + bool setRxPowerSaving(bool enable, uint32_t receive, uint32_t rest) { + ++calls; + const bool accepted = results.empty() ? true : results.front(); + if (!results.empty()) results.pop_front(); + if (accepted) { enabled = enable; rx = receive; sleep = rest; } + return accepted; + } +} radio_driver; +struct BaseSerialInterface {}; +struct Serial { + BaseSerialInterface requester, other; + BaseSerialInterface* current = &requester; + BaseSerialInterface* reply_route = nullptr; + std::vector reply; + BaseSerialInterface* captureReplyRoute() { return current; } + size_t writeFrameToRoute(BaseSerialInterface* route, const uint8_t* data, size_t size) { + reply_route = route; + reply.assign(data, data + size); + return size; + } +}; +struct MyMesh { + Prefs _prefs, durable; + Serial serial; + Serial* _serial = &serial; + bool _radio_available = true, storage_accepts = true, saved_radio_apply_pending = false; + unsigned saves = 0; + uint32_t radio_apply_retry_at = 0; + uint8_t radio_apply_failures = 0; + uint8_t cmd_frame[180] = {}, out_frame[180] = {}; + char reply_buf[160] = {}; + MyMesh() { radio_driver = Driver(); } + bool savePrefs() { ++saves; if (!storage_accepts) return false; durable = _prefs; return true; } + bool applyAndSaveRxPowerSaving(const char*, char*); + bool handleLocalControlCommand(const char*, char*, size_t); + bool handleCommand(const char*, uint32_t, char*); + void frame(size_t len) { @FRAME@ } + void beginUsbLoggingReplyBarrier(BaseSerialInterface* route) { + assert(route == &serial.requester); + // Other transport activity must not steal the response after capture. + serial.current = &serial.other; + } + void endUsbLoggingReplyBarrier(bool queued) { assert(queued); } + void writeErrFrame(uint8_t) { assert(false && "Expected a textual CLI reply"); } + std::string run(const char* command, bool nul = true) { + const size_t size = strlen(command); + assert(size + 2 < sizeof(cmd_frame)); + cmd_frame[0] = mesh::companion::kRunCliCommand; + memcpy(cmd_frame + 1, command, size); + if (nul) cmd_frame[size + 1] = 0; + frame(size + 1 + unsigned(nul)); + assert(serial.reply_route == &serial.requester); + assert(!serial.reply.empty() && serial.reply[0] == RESP_CODE_CLI_REPLY); + return std::string(serial.reply.begin() + 1, serial.reply.end()); + } +}; +@METHODS@ + +static void configure(MyMesh& node, unsigned fault) { + if (fault == 1) radio_driver.supported = false; + if (fault == 2) radio_driver.results = {false}; + if (fault == 3) node.storage_accepts = false; +} +static bool error(const std::string& reply) { + return reply.find("Error") != std::string::npos || reply.find("ERROR") != std::string::npos; +} +int main() { + for (bool nul : {false, true}) { + for (const char* value : {"off", "on", "level 3 preamble 16", "level 8 preamble 32", "65625 60000"}) { + MyMesh node; + const std::string command = std::string(" A7| \tset radio.rxps \t") + value; + const std::string reply = node.run(command.c_str(), nul); + assert(reply.find("A7|OK - ") == 0 && node.saves == 1 && radio_driver.calls == 1); + assert(same(node._prefs, node.durable)); + assert(radio_driver.enabled == bool(node.durable.rx_powersaving_enabled)); + assert(radio_driver.rx == node.durable.rx_ps_rx_us && radio_driver.sleep == node.durable.rx_ps_sleep_us); + if (!strcmp(value, "on")) assert(node.durable.rx_ps_level == 3); + if (!strcmp(value, "off")) assert(!node.durable.rx_powersaving_enabled); + if (!strcmp(value, "level 3 preamble 16")) + assert(node.durable.rx_ps_level == 3 && node.durable.rx_ps_preamble == 16); + if (!strcmp(value, "65625 60000")) + assert(node.durable.rx_ps_level == 0 && node.durable.rx_ps_rx_us == 65625 && node.durable.rx_ps_sleep_us == 60000); + } + } + for (const char* command : {"set radio.rxps", "set radio.rxps on extra", "set radio.rxps level 0", + "set radio.rxps level 3 preamble 17", "set radio.rxps 1000 -1"}) { + MyMesh node; + const std::string reply = node.run(command); + assert(error(reply) && reply.find("Unknown") == std::string::npos); + assert(node.saves == 0 && radio_driver.calls == 0 && same(node._prefs, Prefs())); + } + for (unsigned fault : {1u, 2u, 3u}) { + MyMesh node; + configure(node, fault); + const std::string reply = node.run("B2|set radio.rxps level 3 preamble 16"); + assert(reply.find("B2|") == 0 && error(reply) && reply.find("Unknown") == std::string::npos); + assert(same(node._prefs, Prefs()) && same(node.durable, Prefs())); + assert(node.saves == unsigned(fault == 3)); + assert(radio_driver.calls == (fault == 1 ? 0u : fault == 2 ? 1u : 2u)); + assert(!radio_driver.enabled && radio_driver.rx == 3000 && radio_driver.sleep == 4000); + } + { + MyMesh node; + node.storage_accepts = false; + radio_driver.results = {true, false}; + assert(error(node.run("set radio.rxps level 3 preamble 16"))); + assert(node.saved_radio_apply_pending && same(node._prefs, Prefs()) && same(node.durable, Prefs())); + } + // Every shared-route response remains bounded, including errors from the + // original fixed-size setter. A zero-capacity call must have no side effects. + for (unsigned fault : {0u, 1u, 2u, 3u, 4u}) { + const char* command = fault == 4 ? "set radio.rxps level 3 preamble 17" + : "set radio.rxps level 3 preamble 16"; + MyMesh reference; + configure(reference, fault); + char full[160]; + assert(reference.handleLocalControlCommand(command, full, sizeof(full))); + const std::string expected = full; + for (size_t capacity = 0; capacity <= 180; ++capacity) { + MyMesh node; + configure(node, fault); + std::vector bytes(capacity + 16, 0xA7); + char* reply = reinterpret_cast(bytes.data() + 8); + assert(node.handleLocalControlCommand(command, reply, capacity) == (capacity != 0)); + if (capacity) { + const size_t count = std::min(expected.size(), capacity - 1); + assert(strlen(reply) == count && !memcmp(reply, expected.data(), count)); + } else assert(node.saves == 0 && radio_driver.calls == 0); + for (size_t i = 0; i < 8; ++i) assert(bytes[i] == 0xA7); + for (size_t i = 8 + capacity; i < bytes.size(); ++i) assert(bytes[i] == 0xA7); + } + } + for (const char* command : {"set radio.rxpsx on", "set radio.rxps.status on", "set radio.rxps.rfrx_disabled on"}) { + MyMesh node; + const std::string reply = node.run(command); + assert(reply == "Unknown command" && node.saves == 0 && radio_driver.calls == 0); + } + puts("RXPS framed setter and bounded shared-route contracts passed"); +} diff --git a/test/test_companion_rxps_getters.py b/test/test_companion_rxps_getters.py index 351dbd629..712792bb4 100644 --- a/test/test_companion_rxps_getters.py +++ b/test/test_companion_rxps_getters.py @@ -241,6 +241,58 @@ int main() { self.assertNotIn("ota_", helper) self.assertNotIn("_prefs.rx_ps_rx_us =", helper) + def test_framed_setters_share_checked_persistence_and_bounded_local_replies(self): + source = (ROOT / "examples/companion_radio/MyMesh.cpp").read_text(encoding="ascii") + local = extract_braced(source, "bool MyMesh::handleLocalControlCommand(") + setter = extract_braced(local, 'if (strncmp(command, "set radio.rxps", 14) == 0') + self.assertIn("applyAndSaveRxPowerSaving(value, result)", setter) + # Keep the actual entry validation and command recognition. Unrelated + # hardware/transport handlers need no boundaries in this RXPS fixture. + entry = local[:local.index("#if defined(ESP32_PLATFORM)")] + entry += setter + "\n return false;\n}\n" + shared = extract_braced(source, "bool MyMesh::handleCommand(") + adapter = shared[:shared.index("#if defined(ESP32_PLATFORM)")] + registration = " if (handleLocalControlCommand(command, reply, reply_capacity)) return true;" + self.assertIn(registration, shared) + adapter += " (void)sender_timestamp;\n" + registration + "\n return false;\n}\n" + frame = extract_braced(source, "} else if (mesh::companion::isRunCliFrame(") + frame = frame.replace("} else if", "if", 1) + methods = "\n".join(( + extract_braced(source, "bool MyMesh::applyAndSaveRxPowerSaving("), entry, adapter, + )) + constants = "\n".join(line for line in source.splitlines() + if line.startswith(("#define RESP_CODE_CLI_REPLY ", + "#define ERR_CODE_ILLEGAL_ARG "))) + fixture = (ROOT / "test/fixtures/companion_rxps_setters.cpp").read_text(encoding="ascii") + fixture = constants + "\n" + fixture.replace("@METHODS@", methods).replace("@FRAME@", frame) + compiler = os.environ.get("CXX") or shutil.which("g++") or shutil.which("clang++") + self.assertIsNotNone(compiler, "RXPS routing needs a host C++17 compiler") + sanitizers = [] if os.name == "nt" else [ + "-fsanitize=address,undefined", "-fno-sanitize-recover=all", "-fno-pie", "-no-pie"] + with tempfile.TemporaryDirectory(prefix="meshcore-rxps-setter-routes-") as directory: + work = Path(directory) + cpp, binary = work / "routes.cpp", work / "routes" + for removed_route in (False, True): + with self.subTest(removed_route=removed_route): + # The old firmware had the parser and save helper but + # omitted this dispatch registration. It must fail here. + cpp.write_text(fixture.replace(setter, "", 1) if removed_route else fixture, + encoding="ascii") + built = subprocess.run([ + compiler, "-std=c++17", "-Wall", "-Wextra", "-Werror", *sanitizers, + "-DCOMPANION_FEATURE_TEMP_RADIO=0", "-I", str(ROOT / "src"), + "-isystem", str(ROOT / "test/mocks"), str(cpp), + str(ROOT / "src/helpers/radiolib/RXPowerSaving.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) + if removed_route: + self.assertNotEqual(ran.returncode, 0, "Missing RXPS dispatch was not detected") + self.assertIn("Assertion", ran.stderr) + else: + self.assertEqual(ran.returncode, 0, ran.stdout + ran.stderr) + self.assertIn("RXPS framed setter and bounded shared-route contracts passed", ran.stdout) + if __name__ == "__main__": unittest.main()