From e70f4f9030ab444d24de59c7c17f51c81720a120 Mon Sep 17 00:00:00 2001 From: mikecarper Date: Tue, 15 Sep 2026 14:32:40 -0700 Subject: [PATCH] Fix OTA resume, CLI identity, radio persistence, and network sessions Distinguish OTA read failures from missing blocks and select unique raw-flash checkpoints by MID and target. Keep deferred CLI requests and replies bound to immutable command identity and full authenticated keys. Gate radio mutations on their exact reply delivery, recover coding-rate restoration, and roll back failed Companion preference writes. Preserve accepted low duty cycles across reboot. Retain partial network writes, cancel displaced client operations, and reject malformed or stalled framed input. Add regression and CI coverage, and size-optimize the RAK4631 Ethernet build without dropping features. --- .github/workflows/run-unit-tests.yml | 6 + build.sh | 3 + docs/cli_commands.md | 13 + docs/companion_radio_full.md | 20 + docs/ota_protocol.md | 6 +- docs/ota_user_guide.md | 4 + docs/radio_profiles.md | 17 +- examples/companion_radio/MyMesh.cpp | 255 +++++++---- examples/companion_radio/MyMesh.h | 11 + examples/companion_radio/main.cpp | 11 + examples/simple_repeater/MyMesh.cpp | 140 ++++-- examples/simple_repeater/MyMesh.h | 6 + src/Dispatcher.cpp | 48 +- src/Dispatcher.h | 7 +- src/helpers/CommonCLI.cpp | 6 +- src/helpers/DeferredCliCommand.h | 38 +- src/helpers/RadioProfileCLI.cpp | 134 +++++- src/helpers/RadioProfileCLI.h | 23 +- src/helpers/RemoteCliReplyCache.h | 9 +- src/helpers/TempRadioReplyBarrier.h | 4 + src/helpers/esp32/SerialWifiInterface.cpp | 114 ++--- src/helpers/esp32/SerialWifiInterface.h | 8 +- .../RAK13800/RAK13800EthernetInterface.cpp | 23 +- .../RAK13800/RAK13800EthernetInterface.h | 3 + .../ethernet/SerialEthernetInterface.cpp | 99 ++++- .../ethernet/SerialEthernetInterface.h | 24 +- .../ethernet/ch390/CH390EthernetInterface.cpp | 24 +- .../ethernet/ch390/CH390EthernetInterface.h | 3 + src/helpers/ota/OtaManager.cpp | 38 +- src/helpers/ota/OtaManager.h | 4 +- src/helpers/ota/OtaStore.h | 9 + src/helpers/ota/OtaStoreFlashEsp32.cpp | 85 ++-- src/helpers/ota/OtaStoreFlashEsp32.h | 9 +- src/helpers/ota/OtaStoreFlashNrf52.cpp | 73 +-- src/helpers/ota/OtaStoreFlashNrf52.h | 1 + src/helpers/radiolib/RadioLibWrappers.cpp | 9 + src/helpers/radiolib/RadioLibWrappers.h | 1 + .../test_ota_store_flash_nrf52_hybrid.cpp | 75 +++- test/fixtures/ota_store_resume/test.cpp | 363 ++++++++++++++- .../radio_profiles/reply_mutation_test.cpp | 173 ++++++++ .../remote_cli_command_identity/test.cpp | 415 ++++++++++++++++++ .../test_replay_reset_integration.cpp | 22 +- .../serial_ethernet_sessions/test.cpp | 251 +++++++++++ .../serial_wifi_sessions/mocks/WiFi.h | 2 + test/fixtures/serial_wifi_sessions/test.cpp | 75 +++- test/test_build_size_detection.sh | 33 ++ test/test_companion_prefs_transactions.py | 325 ++++++++++++++ ...est_companion_primary_radio_persistence.py | 247 +++++++++++ ...st_companion_radio_settings_transaction.py | 4 +- test/test_companion_wifi_session.py | 54 ++- .../test_deferred_cli_command.cpp | 82 +++- test/test_indicator_exclusive_transport.py | 3 + test/test_local_cli_access.py | 8 +- test/test_ota_store_flash_nrf52_hybrid.py | 10 +- test/test_ota_store_resume.py | 4 +- test/test_radio_profile_scan.py | 19 +- test/test_radio_reply_mutation.py | 200 +++++++++ test/test_remote_cli_command_identity.py | 73 +++ .../test_remote_cli_reply_cache.cpp | 21 + .../test_repeater_radio_timing_integration.py | 19 + test/test_replay_reset_integration.py | 4 + test/test_serial_ethernet_sessions.py | 79 ++++ test/test_serial_wifi_sessions.py | 11 + ...test_temp_radio_reply_delivery_contract.py | 13 +- 64 files changed, 3504 insertions(+), 369 deletions(-) create mode 100644 test/fixtures/radio_profiles/reply_mutation_test.cpp create mode 100644 test/fixtures/remote_cli_command_identity/test.cpp create mode 100644 test/fixtures/serial_ethernet_sessions/test.cpp create mode 100644 test/test_companion_prefs_transactions.py create mode 100644 test/test_companion_primary_radio_persistence.py create mode 100644 test/test_radio_reply_mutation.py create mode 100644 test/test_remote_cli_command_identity.py create mode 100644 test/test_serial_ethernet_sessions.py diff --git a/.github/workflows/run-unit-tests.yml b/.github/workflows/run-unit-tests.yml index 6c7c70608..465d24fb6 100644 --- a/.github/workflows/run-unit-tests.yml +++ b/.github/workflows/run-unit-tests.yml @@ -83,6 +83,8 @@ jobs: python3 -B test/test_companion_tx_routing.py -v python3 -B test/test_companion_preferences_transaction.py -v python3 -B test/test_companion_radio_settings_transaction.py -v + python3 -B test/test_companion_prefs_transactions.py -v + python3 -B test/test_companion_primary_radio_persistence.py -v python3 -B test/test_contact_cache.py -v - name: Verify receive recovery and pinned radio status transport @@ -104,6 +106,7 @@ jobs: run: | python3 -B test/test_carrier_wave.py python3 -B test/test_radio_profile_cli.py + python3 -B test/test_radio_reply_mutation.py python3 -B test/test_radio_profile_scan.py python3 -B test/test_repeater_radio_timing_integration.py python3 -B test/test_temp_radio_reply_delivery_contract.py @@ -156,6 +159,7 @@ jobs: run: | python3 -B test/test_wireless_control.py -v python3 -B test/test_serial_wifi_sessions.py -v + python3 -B test/test_serial_ethernet_sessions.py -v python3 -B test/test_companion_wifi_session.py -v python3 -B test/test_espnow_radio_lifecycle.py -v @@ -191,6 +195,8 @@ jobs: python3 -B test/test_path_hash_cli.py python3 -B test/test_cli_settings_contract.py python3 -B test/test_local_cli_access.py + python3 -B test/test_remote_cli_command_identity.py -v + python3 -B test/test_replay_reset_integration.py -v - name: Verify firmware memory budgets and nRF52 Bluetooth startup run: | diff --git a/build.sh b/build.sh index 62c7b91a2..3d399aa94 100755 --- a/build.sh +++ b/build.sh @@ -3325,6 +3325,7 @@ apply_nrf52_size_profile() { fi if ! is_repeater_role_target "$env_name" \ && ! is_nrf52_companion_radio_full_target "$env_name" \ + && [ "$env_name" != "RAK_4631_companion_radio_ethernet" ] \ && { ! is_lora_ota_build "$env_name" \ || ! is_lora_ota_only_target "$env_name"; }; then return 0 @@ -3338,6 +3339,8 @@ apply_nrf52_size_profile() { # and Full Companion source images, especially their diagnostic profile. # Full-sensor repeaters also need this after adding the second radio profile; # changing optimization preserves their sensors and protocol features. + # The legacy RAK4631 Ethernet Companion also needs -Os to fit its declared + # application limit; keep its sensors, transports, OTA and layout unchanged. append_platformio_build_unflags "-Ofast" export PLATFORMIO_BUILD_FLAGS="${PLATFORMIO_BUILD_FLAGS} -Os" diff --git a/docs/cli_commands.md b/docs/cli_commands.md index fe3d76ab7..88569147b 100644 --- a/docs/cli_commands.md +++ b/docs/cli_commands.md @@ -24,6 +24,15 @@ The first word of a command is case-insensitive, so `set`, `Set`, and `SET` are equivalent, as are `get`, `Get`, and the other command verbs. The case of arguments such as node names, passwords, and keys is left unchanged. +Repeater retries carrying the same logical request ID and original command +bytes reuse the cached result, even when command parsing trims whitespace or +splits arguments in place. Changing the timestamp alone does not execute the +operation again. Normal authentication and stale-request checks still apply. +Deferred commands and host-service replies remain bound to the full authenticated +public key, even if permission changes move ACL entries. Removing or revoking +the requester cancels pending execution; a running `setperm` can still acknowledge +its own completed change to the original sender. + Use the site search or your browser's Find command with everyday wording such as **tx retries**, **retry attempts**, **serial logging**, or **tx power**. **Search terms** below are alternative wording to help find a command, not @@ -1216,6 +1225,10 @@ settings use a safe effective power without replacing the saved preference. **Notes:** - `tempradio` is not saved to preferences and clears on reboot. +- On repeaters, remote `tempradio` and `normalradio` wait for their exact reply + copies to finish, with at least one transmitted. This also covers the `set` + alias, case-insensitive verbs, and client correlation prefixes. Failed replies + cancel an unconfirmed temporary handoff; hard lease expiry still takes priority. - `normalradio` cancels pending and active temporary-radio windows, then restores the saved radio tuple after its CLI reply has drained on the current channel. Permanent `radioat` entries are not removed. diff --git a/docs/companion_radio_full.md b/docs/companion_radio_full.md index b1ca8d55e..5a6adbb89 100644 --- a/docs/companion_radio_full.md +++ b/docs/companion_radio_full.md @@ -17,6 +17,11 @@ The full Companion is deliberately not a LoRa OTA destination: it has no firmware staging store, refuses `ota install`, and never advertises its own firmware as an mOTA image. +Preference changes through the Companion protocol, WebConfig, or USB terminal +report a storage error if saving fails, and restore their previous values. +Settings that also change GPS or radio power-saving state restore that state; +a busy radio rollback remains pending for recovery instead of reporting success. + | Capability | ESP32 full | nRF52 full | | --- | --- | --- | | USB Binary Companion | Yes | Yes | @@ -399,10 +404,25 @@ message transmissions are not canceled. Queued frames are retained, not a resumable command session, and IP matching is not authentication. Turning WiFi off clears both queues. Stored message history is unaffected. +Ethernet also retains complete queued frames for the same IP. Unlike WiFi, it +has no spare backlog buffer: a different IP cancels the old owner's pending +operations and discards its queued frames before the new socket is exposed. +This keeps Ethernet's existing four-frame memory limit. On both transports, a +live same-IP replacement preserves pending operations; an observed disconnect, +disabled transport, or rejected input cancels them. Already queued frames can +still be replayed on a same-IP reconnect, but canceled operations are not resumed. + Partial TCP writes retain the unsent bytes without interleaving later frames. After a reconnect, an incomplete frame restarts from its header; the new TCP connection must not receive only the old connection's trailing bytes. +Binary WiFi/Ethernet frames must declare 1–176 payload bytes. Illegal lengths +close the connection immediately, without waiting for the announced body or +executing a truncated command. An incomplete binary header or payload has a +five-second deadline once reception starts; reconnect to send a fresh frame. +Ethernet raw-line terminal input has no typing deadline, but an overlong line +also closes the connection instead of executing its prefix. + USB Binary output is queued as complete length-prefixed frames. Temporary CDC or UART backpressure pauses the contact stream; a frame may drain through a smaller hardware FIFO in ordered chunks, but its remainder is retained and no diff --git a/docs/ota_protocol.md b/docs/ota_protocol.md index b1a0413d4..4e6961603 100644 --- a/docs/ota_protocol.md +++ b/docs/ota_protocol.md @@ -399,7 +399,7 @@ There is no separate availability structure. **Block `i` is present <=> `leaves[ reopenable store. A hybrid nRF52 transfer is the deliberate exception: its SRAM-backed payload suffix is volatile, so the application refuses to adopt that staged header instead of rebuilding partial progress. -**Resume (`OtaManager::resumeStaged` + `OtaStore::checkpoint`/`reopen`):** an interrupted fetch resumes from +**Resume (`OtaManager::resumeStaged` + `OtaStore::checkpoint`/`reopenFor`):** an interrupted fetch resumes from the staged container after a reboot - re-parse the stored manifest, recompute geometry, count present blocks, continue fetching the holes (or jump straight to COMPLETE). The checkpoint cadence (persist progress every N committed blocks) is runtime-tunable (`ota config checkpoint `, 0 = only finalized containers @@ -410,6 +410,10 @@ package and keeps target `0` as a MID-only wildcard. Stores keep `leaves[]` in R auto-GC, preserving resumable progress. The debug/operator equivalent is `ota dev resume `; after a reboot the MID is mandatory, while a no-argument form may only reuse a still-active session MID. It never uses the `nullptr` automatic-adoption path, so a malformed MID or no active MID fails closed. +Raw ESP32/nRF52 staging scans every valid header before adoption: address order is not creation order. +Explicit resume selects a unique matching MID/target; automatic resume refuses multiple valid headers +without erasing them. A leaf-marker read error is never treated as a missing block: reconnectable storage +pauses and re-verifies after reconnection, while other storage reports failure and permits an explicit retry. Hybrid nRF52 staging cannot enter this resume path after an application restart, even if its flash prefix still contains metadata; the complete logical container must be fetched again. diff --git a/docs/ota_user_guide.md b/docs/ota_user_guide.md index 4dd074eb9..d061aeffe 100644 --- a/docs/ota_user_guide.md +++ b/docs/ota_user_guide.md @@ -465,6 +465,10 @@ target to match this node and its version to be newer than the running valid End For bring-up/debugging, `ota dev resume ` performs the same explicit MID-bound re-adoption without starting a new network fetch. After reboot it requires the MID; bare `ota dev resume` is accepted only while an active/requested session MID still exists, and malformed or missing identifiers are rejected. +If raw flash contains multiple valid staged headers, automatic resume leaves them untouched rather than +guessing which is newest. Specify the intended MID to select its unique checkpoint; ambiguous matches are +rejected. Temporary storage read failures no longer consume a download slot indefinitely: host storage +pauses for reconnection, while non-reconnectable storage reports a storage error and can be retried. Recommended for most people: leave both **off** and update by hand. Use `autoinstall trusted` only once you've added the signer's key (next section) and you trust them to push updates unattended. Automatic diff --git a/docs/radio_profiles.md b/docs/radio_profiles.md index 62fe9fece..b55523366 100644 --- a/docs/radio_profiles.md +++ b/docs/radio_profiles.md @@ -59,8 +59,21 @@ on the primary profile by default. Use an OTA-capable build and the usual OTA setup on every participating node; a temporary profile does not add OTA support to firmware built without it. -New settings take effect after a short reply allowance. Temporary periods and -schedules live in RAM and disappear on reboot. Timer expiry also uses monotonic +Local settings take effect after a short reply allowance. On repeaters, immediate +remote `radio2`/`tempradio2` changes (including `off`) wait until their exact reply +copies finish, with at least one successfully transmitted. A reply that cannot +be queued or whose copies all fail cancels the change. Queued replies have a +five-minute deadline; an already-transmitting copy may finish under the normal +radio watchdog. This confirms transmission, not reception by the remote client. + +Permanent remote changes are prepared in an uncommitted file before replying; +the saved configuration is replaced only after reply transmission. If that +replacement fails, the old channel remains active and the commit retries. +Local `get radio2.status` reports this pending state. Roles without the tracked remote +reply path require local USB for these immediate secondary-profile changes. + +Temporary periods and schedules live in RAM and disappear on reboot. Waiting +for a reply does not extend a temporary lease. Timer expiry also uses monotonic time, so setting the clock backwards cannot extend a temporary session. ## Repeater, room-server, and sensor replies diff --git a/examples/companion_radio/MyMesh.cpp b/examples/companion_radio/MyMesh.cpp index eb47ee888..448f9788d 100644 --- a/examples/companion_radio/MyMesh.cpp +++ b/examples/companion_radio/MyMesh.cpp @@ -1847,7 +1847,10 @@ void MyMesh::begin(bool has_display, bool radio_available) { // sanitise bad pref values _prefs.rx_delay_base = constrain(_prefs.rx_delay_base, 0, 20.0f); - _prefs.airtime_factor = constrain(_prefs.airtime_factor, 0, 9.0f); + if (!isfinite(_prefs.airtime_factor)) _prefs.airtime_factor = 1.0f; + // `set dutycycle 1` stores factor 99; the legacy `set af` input limit is + // narrower, but must not truncate an accepted duty cycle on restart. + _prefs.airtime_factor = constrain(_prefs.airtime_factor, 0, 99.0f); _prefs.freq = constrain(_prefs.freq, 150.0f, 2500.0f); _prefs.bw = constrain(_prefs.bw, 7.8f, 500.0f); _prefs.sf = constrain(_prefs.sf, 5, 12); @@ -2470,9 +2473,12 @@ bool MyMesh::handleLocalControlCommand(const char* command, char* reply, if (!sensors.setSettingValue("gps", enabled ? "1" : "0")) { snprintf(reply, reply_size, "Error: GPS unavailable"); } else { - _prefs.gps_enabled = enabled ? 1 : 0; - snprintf(reply, reply_size, savePrefs() ? "OK - GPS %s (saved)" : "Error: GPS %s but save failed", - enabled ? "on" : "off"); + if (savePreference(_prefs.gps_enabled, static_cast(enabled))) { + snprintf(reply, reply_size, "OK - GPS %s (saved)", enabled ? "on" : "off"); + } else { + applyGpsPrefs(); + snprintf(reply, reply_size, "Error: GPS could not be saved"); + } } return true; } @@ -3725,11 +3731,11 @@ void MyMesh::execCommand(char* cmd, char* reply) { const char* value = split + 1; if (strcmp(key, "name") == 0) { - if (!value[0] || !wcCopyValue(_prefs.node_name, sizeof(_prefs.node_name), value)) { + char name[sizeof(_prefs.node_name)]; + if (!value[0] || !wcCopyValue(name, sizeof(name), value)) { strcpy(reply, "Error: name must be 1-31 characters"); } else { - savePrefs(); - strcpy(reply, "OK"); + strcpy(reply, saveAdvertName(name) ? "OK" : "Error: name could not be saved"); } return; } @@ -3762,9 +3768,9 @@ void MyMesh::execCommand(char* cmd, char* reply) { strcpy(reply, latitude ? "Error: latitude must be -90 to 90" : "Error: longitude must be -180 to 180"); } else { - if (latitude) sensors.node_lat = parsed; else sensors.node_lon = parsed; - savePrefs(); - strcpy(reply, "OK"); + strcpy(reply, saveAdvertLocation(latitude ? parsed : sensors.node_lat, + latitude ? sensors.node_lon : parsed) + ? "OK" : "Error: location could not be saved"); } return; } @@ -3776,6 +3782,10 @@ void MyMesh::execCommand(char* cmd, char* reply) { || bw < 7.0f || bw > 500.0f || sf < 5 || sf > 12 || cr < 5 || cr > 8) { strcpy(reply, "Error: radio must be freq,bw,sf,cr"); } else { + const float previous_freq = _prefs.freq, previous_bw = _prefs.bw; + const uint8_t previous_sf = _prefs.sf, previous_cr = _prefs.cr; + const uint32_t previous_rx_us = _prefs.rx_ps_rx_us; + const uint32_t previous_sleep_us = _prefs.rx_ps_sleep_us; _prefs.freq = freq; _prefs.bw = bw; _prefs.sf = sf; @@ -3783,8 +3793,17 @@ void MyMesh::execCommand(char* cmd, char* reply) { recalcRxPowerSavingFromLevel(_prefs.rx_ps_level, _prefs.sf, _prefs.bw, _prefs.rx_ps_preamble, &_prefs.rx_ps_rx_us, &_prefs.rx_ps_sleep_us); - savePrefs(); - strcpy(reply, "OK - reboot required"); + if (savePrefs()) { + strcpy(reply, "OK - reboot required"); + } else { + _prefs.freq = previous_freq; + _prefs.bw = previous_bw; + _prefs.sf = previous_sf; + _prefs.cr = previous_cr; + _prefs.rx_ps_rx_us = previous_rx_us; + _prefs.rx_ps_sleep_us = previous_sleep_us; + strcpy(reply, "Error: radio could not be saved"); + } } return; } @@ -3808,9 +3827,9 @@ void MyMesh::execCommand(char* cmd, char* reply) { strcpy(reply, is_af ? "Error: airtime factor must be 0-9" : "Error: RX delay must be 0-20"); } else { - if (is_af) _prefs.airtime_factor = parsed; else _prefs.rx_delay_base = parsed; - savePrefs(); - strcpy(reply, "OK"); + strcpy(reply, savePreference(is_af ? _prefs.airtime_factor : _prefs.rx_delay_base, + static_cast(parsed)) + ? "OK" : "Error: setting could not be saved"); } return; } @@ -3829,8 +3848,10 @@ void MyMesh::execCommand(char* cmd, char* reply) { return; } } else { - _prefs.client_repeat = enabled; - savePrefs(); + if (!savePreference(_prefs.client_repeat, static_cast(enabled))) { + strcpy(reply, "Error: repeat could not be saved"); + return; + } } strcpy(reply, "OK"); } @@ -4508,12 +4529,13 @@ void MyMesh::handleCmdFrame(size_t len) { } } } else if (cmd_frame[0] == CMD_SET_ADVERT_NAME && len >= 2) { + char name[sizeof(_prefs.node_name)]; int nlen = len - 1; - if (nlen > sizeof(_prefs.node_name) - 1) nlen = sizeof(_prefs.node_name) - 1; // max len - memcpy(_prefs.node_name, &cmd_frame[1], nlen); - _prefs.node_name[nlen] = 0; // null terminator - savePrefs(); - writeOKFrame(); + if (nlen > sizeof(name) - 1) nlen = sizeof(name) - 1; // max len + memcpy(name, &cmd_frame[1], nlen); + name[nlen] = 0; + if (saveAdvertName(name)) writeOKFrame(); + else writeErrFrame(ERR_CODE_FILE_IO_ERROR); } else if (cmd_frame[0] == CMD_SET_ADVERT_LATLON && len >= 9) { int32_t lat, lon, alt = 0; memcpy(&lat, &cmd_frame[1], 4); @@ -4522,10 +4544,9 @@ void MyMesh::handleCmdFrame(size_t len) { memcpy(&alt, &cmd_frame[9], 4); // for FUTURE support } if (lat <= 90 * 1E6 && lat >= -90 * 1E6 && lon <= 180 * 1E6 && lon >= -180 * 1E6) { - sensors.node_lat = ((double)lat) / 1000000.0; - sensors.node_lon = ((double)lon) / 1000000.0; - savePrefs(); - writeOKFrame(); + if (saveAdvertLocation(((double)lat) / 1000000.0, + ((double)lon) / 1000000.0)) writeOKFrame(); + else writeErrFrame(ERR_CODE_FILE_IO_ERROR); } else { writeErrFrame(ERR_CODE_ILLEGAL_ARG); // invalid geo coordinate } @@ -4855,10 +4876,16 @@ void MyMesh::handleCmdFrame(size_t len) { i += 4; memcpy(&af, &cmd_frame[i], 4); i += 4; + const float previous_rx = _prefs.rx_delay_base, previous_af = _prefs.airtime_factor; _prefs.rx_delay_base = ((float)rx) / 1000.0f; _prefs.airtime_factor = ((float)af) / 1000.0f; - savePrefs(); - writeOKFrame(); + if (savePrefs()) { + writeOKFrame(); + } else { + _prefs.rx_delay_base = previous_rx; + _prefs.airtime_factor = previous_af; + writeErrFrame(ERR_CODE_FILE_IO_ERROR); + } } else if (cmd_frame[0] == CMD_GET_TUNING_PARAMS) { uint32_t rx = _prefs.rx_delay_base * 1000, af = _prefs.airtime_factor * 1000; int i = 0; @@ -4871,6 +4898,12 @@ void MyMesh::handleCmdFrame(size_t len) { writeErrFrame(ERR_CODE_ILLEGAL_ARG); return; } + const uint8_t previous_manual = _prefs.manual_add_contacts; + const uint8_t previous_base = _prefs.telemetry_mode_base; + const uint8_t previous_loc = _prefs.telemetry_mode_loc; + const uint8_t previous_env = _prefs.telemetry_mode_env; + const uint8_t previous_policy = _prefs.advert_loc_policy; + const uint8_t previous_acks = _prefs.multi_acks; _prefs.manual_add_contacts = cmd_frame[1]; if (len >= 3) { _prefs.telemetry_mode_base = cmd_frame[2] & 0x03; // v5+ @@ -4884,16 +4917,24 @@ void MyMesh::handleCmdFrame(size_t len) { } } } - updateGpsTelemetryPolicy(); - savePrefs(); - writeOKFrame(); + if (savePrefs()) { + updateGpsTelemetryPolicy(); + writeOKFrame(); + } else { + _prefs.manual_add_contacts = previous_manual; + _prefs.telemetry_mode_base = previous_base; + _prefs.telemetry_mode_loc = previous_loc; + _prefs.telemetry_mode_env = previous_env; + _prefs.advert_loc_policy = previous_policy; + _prefs.multi_acks = previous_acks; + writeErrFrame(ERR_CODE_FILE_IO_ERROR); + } } else if (cmd_frame[0] == CMD_SET_PATH_HASH_MODE && len >= 3 && cmd_frame[1] == 0) { if (cmd_frame[2] >= 3) { writeErrFrame(ERR_CODE_ILLEGAL_ARG); } else { - _prefs.path_hash_mode = cmd_frame[2]; - savePrefs(); - writeOKFrame(); + if (savePreference(_prefs.path_hash_mode, cmd_frame[2])) writeOKFrame(); + else writeErrFrame(ERR_CODE_FILE_IO_ERROR); } } else if (cmd_frame[0] == CMD_REBOOT && len >= 7 && memcmp(&cmd_frame[1], "reboot", 6) == 0) { if (!flushContactsBeforeReboot()) { @@ -5308,9 +5349,8 @@ void MyMesh::handleCmdFrame(size_t len) { // ensure pin is zero, or a valid 6 digit pin if (pin == 0 || (pin >= 100000 && pin <= 999999)) { - _prefs.ble_pin = pin; - savePrefs(); - writeOKFrame(); + if (savePreference(_prefs.ble_pin, pin)) writeOKFrame(); + else writeErrFrame(ERR_CODE_FILE_IO_ERROR); } else { writeErrFrame(ERR_CODE_ILLEGAL_ARG); } @@ -5339,12 +5379,19 @@ void MyMesh::handleCmdFrame(size_t len) { #if ENV_INCLUDE_GPS == 1 // Update node preferences for GPS settings if (strcmp(sp, "gps") == 0) { - _prefs.gps_enabled = (np[0] == '1') ? 1 : 0; - savePrefs(); + if (!savePreference(_prefs.gps_enabled, static_cast(np[0] == '1'))) { + applyGpsPrefs(); + writeErrFrame(ERR_CODE_FILE_IO_ERROR); + return; + } } else if (strcmp(sp, "gps_interval") == 0) { uint32_t interval_seconds = atoi(np); - _prefs.gps_interval = constrain(interval_seconds, 0, 86400); - savePrefs(); + if (!savePreference(_prefs.gps_interval, + static_cast(constrain(interval_seconds, 0, 86400)))) { + applyGpsPrefs(); + writeErrFrame(ERR_CODE_FILE_IO_ERROR); + return; + } } #endif writeOKFrame(); @@ -5642,6 +5689,10 @@ void MyMesh::handleCmdFrame(size_t len) { send_unscoped = true; writeOKFrame(); } else if (cmd_frame[0] == CMD_SET_DEFAULT_FLOOD_SCOPE && len >= 1) { + char previous_name[sizeof(_prefs.default_scope_name)]; + uint8_t previous_key[sizeof(_prefs.default_scope_key)]; + memcpy(previous_name, _prefs.default_scope_name, sizeof(previous_name)); + memcpy(previous_key, _prefs.default_scope_key, sizeof(previous_key)); if (len >= 1+31+16) { const void* terminator = memchr(&cmd_frame[1], 0, 31); size_t n = terminator == NULL ? 31 : (const uint8_t*)terminator - &cmd_frame[1]; @@ -5649,16 +5700,20 @@ void MyMesh::handleCmdFrame(size_t len) { memcpy(_prefs.default_scope_name, &cmd_frame[1], n); _prefs.default_scope_name[n] = 0; memcpy(_prefs.default_scope_key, &cmd_frame[1+31], 16); - savePrefs(); - writeOKFrame(); } else { writeErrFrame(ERR_CODE_ILLEGAL_ARG); + return; } } else { memset(_prefs.default_scope_name, 0, sizeof(_prefs.default_scope_name)); // set default scope to null memset(_prefs.default_scope_key, 0, sizeof(_prefs.default_scope_key)); - savePrefs(); + } + if (savePrefs()) { writeOKFrame(); + } else { + memcpy(_prefs.default_scope_name, previous_name, sizeof(previous_name)); + memcpy(_prefs.default_scope_key, previous_key, sizeof(previous_key)); + writeErrFrame(ERR_CODE_FILE_IO_ERROR); } } else if (cmd_frame[0] == CMD_GET_DEFAULT_FLOOD_SCOPE) { out_frame[0] = RESP_CODE_DEFAULT_FLOOD_SCOPE; @@ -5682,12 +5737,19 @@ void MyMesh::handleCmdFrame(size_t len) { writeErrFrame(ERR_CODE_ILLEGAL_ARG); return; } + const uint8_t previous_config = _prefs.autoadd_config; + const uint8_t previous_max_hops = _prefs.autoadd_max_hops; _prefs.autoadd_config = cmd_frame[1]; if (len >= 3) { _prefs.autoadd_max_hops = min(cmd_frame[2], (uint8_t)64); } - savePrefs(); - writeOKFrame(); + if (savePrefs()) { + writeOKFrame(); + } else { + _prefs.autoadd_config = previous_config; + _prefs.autoadd_max_hops = previous_max_hops; + writeErrFrame(ERR_CODE_FILE_IO_ERROR); + } } else if (cmd_frame[0] == CMD_GET_AUTOADD_CONFIG) { int i = 0; out_frame[i++] = RESP_CODE_AUTOADD_CONFIG; @@ -6316,6 +6378,25 @@ bool MyMesh::handleExtraFsHilCommand(const char* command, char* reply, #endif #endif +bool MyMesh::saveAdvertName(const char* name) { + char previous[sizeof(_prefs.node_name)]; + memcpy(previous, _prefs.node_name, sizeof(previous)); + StrHelper::strncpy(_prefs.node_name, name, sizeof(_prefs.node_name)); + if (savePrefs()) return true; + memcpy(_prefs.node_name, previous, sizeof(previous)); + return false; +} + +bool MyMesh::saveAdvertLocation(double latitude, double longitude) { + const double previous_lat = sensors.node_lat, previous_lon = sensors.node_lon; + sensors.node_lat = latitude; + sensors.node_lon = longitude; + if (savePrefs()) return true; + sensors.node_lat = previous_lat; + sensors.node_lon = previous_lon; + return false; +} + MyMesh::RadioSettingResult MyMesh::applyAndSaveTxPower(int8_t power) { const int8_t previous = _prefs.tx_power_dbm; if (_radio_available && !radio_driver.setTxPower(power)) { @@ -6804,12 +6885,11 @@ bool MyMesh::applyAndSavePowerSaving(const char* value, char* reply) { return false; } - _prefs.powersaving_enabled = enabled ? 1 : 0; - sensors.setPowerSavingEnabled(enabled); - if (!savePrefs()) { - strcpy(reply, "Error: power saving changed for this boot but save failed"); + if (!savePreference(_prefs.powersaving_enabled, static_cast(enabled))) { + strcpy(reply, "Error: power saving could not be saved"); return false; } + sensors.setPowerSavingEnabled(enabled); snprintf(reply, 160, "OK - powersaving %s", enabled ? "on" : "off"); return true; } @@ -6970,12 +7050,30 @@ bool MyMesh::applyAndSaveRxPowerSaving(const char* value, char* reply) { return false; } + const uint8_t previous_enabled = _prefs.rx_powersaving_enabled; + const uint8_t previous_level = _prefs.rx_ps_level; + const uint8_t previous_preamble = _prefs.rx_ps_preamble; + const uint32_t previous_rx = _prefs.rx_ps_rx_us, previous_sleep = _prefs.rx_ps_sleep_us; _prefs.rx_powersaving_enabled = enabled; _prefs.rx_ps_rx_us = rx_us; _prefs.rx_ps_sleep_us = sleep_us; _prefs.rx_ps_level = level; _prefs.rx_ps_preamble = preamble; - savePrefs(); + if (!savePrefs()) { + _prefs.rx_powersaving_enabled = previous_enabled; + _prefs.rx_ps_rx_us = previous_rx; + _prefs.rx_ps_sleep_us = previous_sleep; + _prefs.rx_ps_level = previous_level; + _prefs.rx_ps_preamble = previous_preamble; + if (_radio_available + && !radio_driver.setRxPowerSaving(previous_enabled != 0, previous_rx, previous_sleep)) { + saved_radio_apply_pending = true; + radio_apply_retry_at = 0; + radio_apply_failures = 0; + } + strcpy(reply, "Error: RX power saving could not be saved"); + return false; + } snprintf(reply, 160, "OK - %s,%lu,%lu", enabled ? "on" : "off", (unsigned long)rx_us, (unsigned long)sleep_us); @@ -8045,23 +8143,20 @@ void MyMesh::handleTerminalCommand(char* command) { || parsed < 0.0f || parsed > 9.0f) { terminalOutput().print(" ERROR: airtime factor must be 0-9\r\n"); } else { - _prefs.airtime_factor = parsed; - savePrefs(); - terminalOutput().print(" OK\r\n"); + terminalOutput().print(savePreference(_prefs.airtime_factor, parsed) + ? " OK\r\n" : " ERROR: airtime factor could not be saved\r\n"); } } else if (strncmp(config, "name ", 5) == 0 && config[5] != 0) { - StrHelper::strncpy(_prefs.node_name, config + 5, sizeof(_prefs.node_name)); - savePrefs(); - terminalOutput().print(" OK\r\n"); + terminalOutput().print(saveAdvertName(config + 5) + ? " OK\r\n" : " ERROR: name could not be saved\r\n"); } else if (strncmp(config, "lat ", 4) == 0) { float parsed = 0.0f; if (!mesh::cli::parseDecimalStrict(config + 4, parsed) || parsed < -90.0f || parsed > 90.0f) { terminalOutput().print(" ERROR: latitude must be -90 to 90\r\n"); } else { - sensors.node_lat = parsed; - savePrefs(); - terminalOutput().print(" OK\r\n"); + terminalOutput().print(saveAdvertLocation(parsed, sensors.node_lon) + ? " OK\r\n" : " ERROR: latitude could not be saved\r\n"); } } else if (strncmp(config, "lon ", 4) == 0) { float parsed = 0.0f; @@ -8069,9 +8164,8 @@ void MyMesh::handleTerminalCommand(char* command) { || parsed < -180.0f || parsed > 180.0f) { terminalOutput().print(" ERROR: longitude must be -180 to 180\r\n"); } else { - sensors.node_lon = parsed; - savePrefs(); - terminalOutput().print(" OK\r\n"); + terminalOutput().print(saveAdvertLocation(sensors.node_lat, parsed) + ? " OK\r\n" : " ERROR: longitude could not be saved\r\n"); } } else if (strncmp(config, "tx ", 3) == 0) { handleCommand(command, 0, local_reply); @@ -8392,11 +8486,12 @@ bool MyMesh::handleCommand(const char* command, uint32_t sender_timestamp, while (*command == ' ' || *command == '\t') command++; } - if (_radio_profiles.handle(command, reply, reply_capacity)) return true; + if (_radio_profiles.handle(command, reply, reply_capacity, sender_timestamp != 0)) return true; if (!strncmp(command, "set tempradio ", 14)) command += 4; if (!strcmp(command, "get tempradio")) command += 4; char profile_command[140]; const uint16_t previous_primary_preamble = _radio_profiles.primaryPreamble(); + uint16_t requested_primary_preamble = previous_primary_preamble; if (!strncmp(command, "set radio ", 10)) { char legacy[120]; uint16_t preamble = 0; @@ -8408,9 +8503,7 @@ bool MyMesh::handleCommand(const char* command, uint32_t sender_timestamp, if (!_radio_profiles.acceptsPrimary(f, bw, sf, cr, preamble)) { snprintf(reply, reply_capacity, "Error: radio params unsupported"); return true; } - if (!_radio_profiles.savePrimaryPreamble(preamble)) { - snprintf(reply, reply_capacity, "Error: preamble could not be saved"); return true; - } + requested_primary_preamble = preamble; snprintf(profile_command, sizeof(profile_command), "set radio %s", legacy); command = profile_command; } @@ -8488,14 +8581,19 @@ bool MyMesh::handleCommand(const char* command, uint32_t sender_timestamp, command, sender_timestamp, reply)) { return false; } - if (_prefs.getRadioPrefs()->isDirty()) { + if (_prefs.getRadioPrefs()->isDirty() && strncmp(reply, "OK", 2) == 0) { if (strncmp(command, "set radio ", 10) == 0) { recalcRxPowerSavingFromLevel( _prefs.rx_ps_level, _prefs.sf, _prefs.bw, _prefs.rx_ps_preamble, &_prefs.rx_ps_rx_us, &_prefs.rx_ps_sleep_us); } - if (!savePrefs()) { + // Both validators must accept before either persistent image changes. + // In particular, the shared prefs parser has a tighter BW tolerance. + const bool primary_radio_command = strncmp(command, "set radio ", 10) == 0; + const bool preamble_saved = !primary_radio_command + || _radio_profiles.savePrimaryPreamble(requested_primary_preamble); + if (!preamble_saved || !savePrefs()) { _prefs.freq = previous_freq; _prefs.bw = previous_bw; _prefs.sf = previous_sf; @@ -8507,7 +8605,9 @@ bool MyMesh::handleCommand(const char* command, uint32_t sender_timestamp, _prefs.rx_ps_rx_us = previous_rx_us; _prefs.rx_ps_sleep_us = previous_sleep_us; _prefs.clearDirty(); - if (strncmp(command, "set radio ", 10) == 0 + if (!preamble_saved) { + strcpy(reply, "Error: preamble could not be saved"); + } else if (primary_radio_command && !_radio_profiles.savePrimaryPreamble(previous_primary_preamble)) { strcpy(reply, "Error: radio save failed; preamble rollback failed"); } else { @@ -8536,15 +8636,7 @@ bool MyMesh::handleCommand(const char* command, uint32_t sender_timestamp, if (!AdvertDataParser::isValidName(name)) { strcpy(reply, "Error, bad chars"); } else { - char previous[sizeof(_prefs.node_name)]; - memcpy(previous, _prefs.node_name, sizeof(previous)); - StrHelper::strncpy(_prefs.node_name, name, sizeof(_prefs.node_name)); - if (!savePrefs()) { - memcpy(_prefs.node_name, previous, sizeof(_prefs.node_name)); - strcpy(reply, "Error: name changed but save failed"); - } else { - strcpy(reply, "OK"); - } + strcpy(reply, saveAdvertName(name) ? "OK" : "Error: name could not be saved"); } return true; } @@ -8893,7 +8985,10 @@ void MyMesh::loop() { // once the receive/response path is idle. radio_driver.setRxBoostedGainMode(_prefs.rx_boosted_gain); if (applySavedRadioParams() - && radio_driver.setTxPower(_prefs.tx_power_dbm)) { + && radio_driver.setTxPower(_prefs.tx_power_dbm) + && (!radio_driver.supportsRxPowerSaving() + || radio_driver.setRxPowerSaving(_prefs.rx_powersaving_enabled != 0, + _prefs.rx_ps_rx_us, _prefs.rx_ps_sleep_us))) { saved_radio_apply_pending = false; radio_apply_retry_at = 0; radio_apply_failures = 0; diff --git a/examples/companion_radio/MyMesh.h b/examples/companion_radio/MyMesh.h index 649c2f6bb..08898b0da 100644 --- a/examples/companion_radio/MyMesh.h +++ b/examples/companion_radio/MyMesh.h @@ -360,6 +360,17 @@ public: bool hasPendingWork() const; private: + // Only snapshot the changed value: CompanionNodePrefs owns self-referencing + // runtime adapters and must not be copied as a transaction snapshot. + template bool savePreference(T& preference, T value) { + const T previous = preference; + preference = value; + if (savePrefs()) return true; + preference = previous; + return false; + } + bool saveAdvertName(const char* name); + bool saveAdvertLocation(double latitude, double longitude); void writeOKFrame(BaseSerialInterface* route = nullptr); void writeErrFrame(uint8_t err_code, BaseSerialInterface* route = nullptr); diff --git a/examples/companion_radio/main.cpp b/examples/companion_radio/main.cpp index d3a9e6669..95a15ebe5 100644 --- a/examples/companion_radio/main.cpp +++ b/examples/companion_radio/main.cpp @@ -472,6 +472,16 @@ static void serviceCompanionPowerSaving(bool force = false) { } #endif +#if defined(ETHERNET_ENABLED) +static void cancelCompanionEthernetSession(void*) { + if (interface_manager.isReplyRouteFor(ðernet_interface)) { + the_mesh.cancelSerialResponseStream(); + } + the_mesh.cancelSerialOperationsForRoute(ðernet_interface); + interface_manager.forgetReplyRouteForDisconnected(ðernet_interface); +} +#endif + #if defined(ENABLE_USB_INTERFACE) static char usb_terminal_line[MAX_TRANS_UNIT * 2 + 32]; static size_t usb_terminal_line_len = 0; @@ -2900,6 +2910,7 @@ void setup() { // add ethernet interface #if defined(ETHERNET_ENABLED) + ethernet_interface.setSessionChangedCallback(cancelCompanionEthernetSession, nullptr); ethernet_interface.begin(); interface_manager.addInterface(InterfaceType::Ethernet, ðernet_interface); #endif diff --git a/examples/simple_repeater/MyMesh.cpp b/examples/simple_repeater/MyMesh.cpp index 4da6ecd94..54fd7b034 100644 --- a/examples/simple_repeater/MyMesh.cpp +++ b/examples/simple_repeater/MyMesh.cpp @@ -2830,12 +2830,13 @@ void MyMesh::onPeerDataRecv(mesh::Packet *packet, uint8_t type, int sender_idx, uint32_t command_fingerprint = mesh::RemoteCliReplyCache::fingerprint(command, command_len); const char* cached_response = NULL; + bool cached_authoritative_reply = false; // A cached challenge response can outlive its disclosure window or name // a token from an earlier attempt. Re-evaluate preparation against the // live nonce policy instead; confirmation results remain cacheable. const bool cached_retry = !replay_prepare && remote_cli_reply_cache.lookup( client->id.pub_key, request_id, command_fingerprint, - &cached_response); + &cached_response, &cached_authoritative_reply); // An old exact match may only replay its stored response. Any stale // mismatch remains blocked by the normal timestamp replay guard. @@ -2868,10 +2869,7 @@ void MyMesh::onPeerDataRecv(mesh::Packet *packet, uint8_t type, int sender_idx, && region_map.getTransportKeysFor(*recv_pkt_region, &reply_scope, 1) > 0; if (cached_retry) { - const bool cached_temp_radio_success = - strncmp(command, "tempradio ", 10) == 0 - && strncmp(cached_response, "OK - temp params for ", 21) == 0; - if (cached_temp_radio_success) { + if (cached_authoritative_reply) { // The original acknowledgement is the only packet allowed to // release its radio-mutation barrier. Replaying this success would // create another untracked copy, including after a TX failure @@ -2884,7 +2882,7 @@ void MyMesh::onPeerDataRecv(mesh::Packet *packet, uint8_t type, int sender_idx, sender_timestamp, cached_response, reply_scoped ? &reply_scope : NULL); } - } else if (deferred_cli_command.matches(i, request_id, command, + } else if (deferred_cli_command.matches(client->id.pub_key, request_id, command, command_len)) { // The original request is already queued. Let it produce the one // authoritative result instead of turning an in-flight retry into a @@ -2892,7 +2890,7 @@ void MyMesh::onPeerDataRecv(mesh::Packet *packet, uint8_t type, int sender_idx, MESH_DEBUG_PRINTLN("onPeerDataRecv: remote CLI request is already pending"); } else if (repeated_timestamp) { MESH_DEBUG_PRINTLN("onPeerDataRecv: duplicate remote CLI request has no cached reply"); - } else if (!deferred_cli_command.enqueue(i, sender_timestamp, + } else if (!deferred_cli_command.enqueue(client->id.pub_key, sender_timestamp, packet->getPathHashSize(), secret, command, command_len, request_id, packet->radio_profile, @@ -3019,10 +3017,9 @@ bool MyMesh::completeHostCliRequest(const char* service_reply) { return false; } - const int client_index = deferred_cli_command.client_index; - if (client_index < 0 || client_index >= acl.getNumClients()) { - MESH_DEBUG_PRINTLN("completeHostCliRequest: invalid client idx: %d", - client_index); + const int client_index = deferred_cli_command.findClientIndex(acl); + if (client_index < 0 || !acl.getClientByIdx(client_index)->isAdmin()) { + MESH_DEBUG_PRINTLN("completeHostCliRequest: requester removed or no longer admin"); clearDeferredCliCommand(); return false; } @@ -3037,9 +3034,7 @@ bool MyMesh::completeHostCliRequest(const char* service_reply) { deferred_cli_command.radio_generation); const uint32_t command_fingerprint = - mesh::RemoteCliReplyCache::fingerprint( - deferred_cli_command.command, - strlen(deferred_cli_command.command)); + deferred_cli_command.command_fingerprint; remote_cli_reply_cache.remember(client->id.pub_key, deferred_cli_command.request_id, command_fingerprint, reply); @@ -3098,15 +3093,24 @@ void __attribute__((noinline)) MyMesh::processDeferredCliCommand() { ReceiveProfileScope radio_scope(*this, deferred_cli_command.radio_profile, deferred_cli_command.radio_generation); - const int client_index = deferred_cli_command.client_index; - if (client_index < 0 || client_index >= acl.getNumClients()) { - MESH_DEBUG_PRINTLN("processDeferredCliCommand: invalid client idx: %d", client_index); + const int client_index = deferred_cli_command.findClientIndex(acl); + if (client_index < 0) { + MESH_DEBUG_PRINTLN("processDeferredCliCommand: requester no longer exists"); + clearDeferredCliCommand(); + return; + } + ClientInfo* client = acl.getClientByIdx(client_index); + if (!(client->isAdmin() || client->isRegionMgr() || client->isFilterMgr())) { clearDeferredCliCommand(); return; } #if MESH_ENABLE_HOST_CLI if (host_cli_waiting) { + if (!client->isAdmin()) { + clearDeferredCliCommand(); + return; + } if (millisHasNowPassed(host_cli_deadline)) { completeHostCliRequest("Err - host service timeout"); return; @@ -3140,10 +3144,28 @@ void __attribute__((noinline)) MyMesh::processDeferredCliCommand() { } #endif - ClientInfo* client = acl.getClientByIdx(client_index); char* reply = (char*)&reply_data[5]; reply[0] = 0; + if (temp_radio_reply_barrier.waiting() || _cli.radioProfiles().hasReplyMutation()) { + // A second command must not replace the callback ownership of an + // unconfirmed mutation. Local/USB recovery remains available. + strcpy(reply, "Err - radio acknowledgement/storage commit pending; retry later"); + remote_cli_reply_cache.remember(client->id.pub_key, deferred_cli_command.request_id, + deferred_cli_command.command_fingerprint, reply); + sendRemoteCliReply(client, deferred_cli_command.secret, deferred_cli_command.path_hash_size, + deferred_cli_command.sender_timestamp, reply, + deferred_cli_reply_scoped ? &deferred_cli_reply_scope : NULL); + clearDeferredCliCommand(); + return; + } + const uint32_t primary_mutation_before = primary_radio_mutation_generation; + const uint32_t secondary_mutation_before = _cli.radioProfiles().replyMutationGeneration(); + // setperm may compact the ACL, including removal of this sender. Keep the + // authenticated pre-command destination/path for its final acknowledgement. + // Command permission checks still use the live ACL entry below. + ClientInfo reply_client = *client; + #if MESH_ENABLE_HOST_CLI mesh::HostCliBridge::RequestView host_request; const mesh::HostCliBridge::ParseResult host_result = @@ -3197,39 +3219,43 @@ void __attribute__((noinline)) MyMesh::processDeferredCliCommand() { } } else { #endif + _cli.radioProfiles().beginReplyCommand(); handleCommand(deferred_cli_command.sender_timestamp, client, deferred_cli_command.command, reply, client_index, deferred_cli_command.path_hash_size); + _cli.radioProfiles().endReplyCommand(); #if MESH_ENABLE_HOST_CLI } #endif const uint32_t command_fingerprint = - mesh::RemoteCliReplyCache::fingerprint( - deferred_cli_command.command, - strlen(deferred_cli_command.command)); - const bool arms_temp_radio = - strncmp(deferred_cli_command.command, "tempradio ", 10) == 0 - && strncmp(reply, "OK - temp params for ", 21) == 0; - remote_cli_reply_cache.remember(client->id.pub_key, + deferred_cli_command.command_fingerprint; + // Detect the operation actually accepted by the parsed command, rather than + // spelling/capitalization, optional `set`, whitespace, or request prefixes. + const bool arms_primary_radio = primary_radio_mutation_generation != primary_mutation_before; + const bool arms_secondary_radio = _cli.radioProfiles().replyMutationGeneration() != secondary_mutation_before; + const bool arms_temp_radio = arms_primary_radio || arms_secondary_radio; + remote_cli_reply_cache.remember(deferred_cli_command.client_pub_key, deferred_cli_command.request_id, - command_fingerprint, reply); + command_fingerprint, reply, arms_temp_radio); mesh::Packet* queued_reply = NULL; const bool reply_queued = sendRemoteCliReply( - client, deferred_cli_command.secret, + &reply_client, deferred_cli_command.secret, deferred_cli_command.path_hash_size, deferred_cli_command.sender_timestamp, reply, deferred_cli_reply_scoped ? &deferred_cli_reply_scope : NULL, arms_temp_radio ? &queued_reply : NULL); if (arms_temp_radio) { + radio_reply_secondary = arms_secondary_radio; if (reply_queued && queued_reply != NULL) { // A fixed delay can expire while either copy is parked behind queue // work. Wait for both to drain and at least one to physically transmit. temp_radio_reply_barrier.arm(queued_reply); + radio_reply_deadline = millis() + 300000UL; } else { // A command which cannot queue its acknowledgement must never strand // the administrator on an unconfirmed tuple. - scheduleNormalRadio(); + finishRadioReply(false); } } clearDeferredCliCommand(); @@ -3958,7 +3984,7 @@ bool MyMesh::sendRepeatersFloodText(const char* text, const TransportKey* scope, void MyMesh::onSendComplete(mesh::Packet* packet) { mesh::Mesh::onSendComplete(packet); - temp_radio_reply_barrier.complete(packet); + if (temp_radio_reply_barrier.complete(packet) && !temp_radio_reply_barrier.waiting()) finishRadioReply(true); if (packet == pending_battery_alert_packet) { pending_battery_alert_packet = NULL; battery_alert_sent = true; @@ -3978,13 +4004,42 @@ void MyMesh::onSendFail(mesh::Packet* packet) { // Failure of every acknowledgement copy cancels the unconfirmed // handoff. A later cached-command retry may replay the reply, but it does // not re-run the mutation. - scheduleNormalRadio(); + finishRadioReply(false); + } else if (!temp_radio_reply_barrier.waiting() && temp_radio_reply_barrier.succeeded()) { + finishRadioReply(true); } if (packet == pending_battery_alert_packet) { pending_battery_alert_packet = NULL; } } +void MyMesh::finishRadioReply(bool delivered) { + temp_radio_reply_barrier.clear(); + radio_reply_deadline = 0; + if (radio_reply_secondary) { + _cli.radioProfiles().finishReplyMutation(delivered); + } else if (!delivered && primary_radio_mutation_starts_temp) { + scheduleNormalRadio(); + } + radio_reply_secondary = false; +} + +void MyMesh::serviceRadioReplyDeadline() { + if (!temp_radio_reply_barrier.waiting() + || (int32_t)((uint32_t)millis() - radio_reply_deadline) < 0) return; + // Retire parked copies through their ordinary failure hooks. An on-air + // copy completes (or hits Dispatcher's airtime watchdog); it cannot retry. + if (temp_radio_reply_barrier.contains(getOutboundInFlight())) { + cancelOutboundRadioRetry(getOutboundInFlight()); + } + for (int i = _mgr->getOutboundTotal() - 1; i >= 0; --i) { + if (!temp_radio_reply_barrier.contains(_mgr->getOutboundByIdx(i))) continue; + auto* packet = _mgr->removeOutboundByIdx(i); + onSendFail(packet); + releasePacket(packet); + } +} + void MyMesh::checkBatteryAlert() { if (!_prefs.battery_alert_enabled) { return; @@ -4125,6 +4180,8 @@ bool MyMesh::applySavedRadioParams() { } void MyMesh::queueSavedRadioApply() { + ++primary_radio_mutation_generation; + primary_radio_mutation_starts_temp = false; saved_radio_apply_pending = true; scheduled_radio_retry_at = 0; scheduled_radio_retry_failures = 0; @@ -4584,7 +4641,12 @@ void MyMesh::processScheduledRadioSettings() { // already active: a permanently queued acknowledgement must not preserve // that old tuple beyond the newly accepted hard lease. The late reply can // still leave on the restored channel, but it can no longer cause a switch. - temp_radio_reply_barrier.clear(); + if (!radio_reply_secondary) { + // Expiring a scheduled lease must not cancel other future temporary + // entries. The scheduler below retires only the expired entry itself. + temp_radio_reply_barrier.clear(); + radio_reply_deadline = 0; + } } // A radio-apply backoff must not turn into a lease extension. Bypass it // only for an expired temporary window; ordinary starts and permanent @@ -4827,6 +4889,12 @@ uint32_t MyMesh::getPowerSaveSleepSeconds(uint32_t max_secs) const { } void MyMesh::applyTempRadioParams(float freq, float bw, uint8_t sf, uint8_t cr, int timeout_mins, uint16_t preamble) { + ++primary_radio_mutation_generation; + primary_radio_mutation_starts_temp = true; + if (_cli.radioProfiles().hasReplyMutation()) { + _cli.radioProfiles().finishReplyMutation(false); + radio_reply_secondary = false; + } // A newer TempRadio command supersedes the reply barrier belonging to the // old schedule. processDeferredCliCommand() arms the new exact reply after // it has been composed and successfully queued. @@ -4880,6 +4948,11 @@ void MyMesh::applyTempRadioParams(float freq, float bw, uint8_t sf, uint8_t cr, } bool MyMesh::scheduleNormalRadio() { + ++primary_radio_mutation_generation; + if (_cli.radioProfiles().hasReplyMutation()) { + _cli.radioProfiles().finishReplyMutation(false); + radio_reply_secondary = false; + } // Cancel every pending/active temporary entry, but leave permanent radioat // changes intact. The saved apply waits for the CLI reply to leave the // outbound queue before changing modulation parameters. @@ -11205,9 +11278,7 @@ bool MyMesh::handleReplayResetCommand(ClientInfo* sender, const char* command, // for the affected identity. Do not execute that stale mailbox afterwards. // The executing LoRa reset owns the mailbox until its reply is sent. if (usb_origin && deferred_cli_command.pending) { - const int index = deferred_cli_command.client_index; - if (all || (index >= 0 && index < acl.getNumClients() - && memcmp(acl.getClientByIdx(index)->id.pub_key, target, PUB_KEY_SIZE) == 0)) { + if (all || memcmp(deferred_cli_command.client_pub_key, target, PUB_KEY_SIZE) == 0) { clearDeferredCliCommand(); } } @@ -12141,6 +12212,7 @@ void MyMesh::loop() { servicePendingSerialOutput(); #endif _cli.loop(); + serviceRadioReplyDeadline(); processDeferredCliCommand(); servicePostMeshLoop(); #if defined(ENABLE_OTA) && OTA_DYNAMIC_CONTEXT diff --git a/examples/simple_repeater/MyMesh.h b/examples/simple_repeater/MyMesh.h index f6f30bf83..5a4304169 100644 --- a/examples/simple_repeater/MyMesh.h +++ b/examples/simple_repeater/MyMesh.h @@ -337,6 +337,12 @@ class MyMesh : public mesh::Mesh, public CommonCLICallbacks mesh::ReplayResetNonce replay_reset_nonce; bool replay_clock_set = false; mesh::TempRadioReplyBarrier temp_radio_reply_barrier; + uint32_t primary_radio_mutation_generation = 0; + bool primary_radio_mutation_starts_temp = false; + uint32_t radio_reply_deadline = 0; + bool radio_reply_secondary = false; + void finishRadioReply(bool delivered); + void serviceRadioReplyDeadline(); TransportKey deferred_cli_reply_scope; bool deferred_cli_reply_scoped; #if MESH_ENABLE_HOST_CLI diff --git a/src/Dispatcher.cpp b/src/Dispatcher.cpp index c783eb20a..bca46518c 100644 --- a/src/Dispatcher.cpp +++ b/src/Dispatcher.cpp @@ -124,15 +124,40 @@ void Dispatcher::updateTxBudget() { } } -void Dispatcher::restoreOutboundTxOverrides() { - if (outbound_restore_cr != 0) { - _radio->setCodingRate(outbound_restore_cr); +bool Dispatcher::restoreOutboundTxOverrides() { + if (!outbound_restore_cr) return true; + const uint32_t now = _ms->getMillis(); + if (outbound_restore_retry_at && (int32_t)(now - outbound_restore_retry_at) < 0) return false; + // A scanner or an intervening settings change may have selected another + // profile since the failed restore. Restore its configured CR, not a stale + // default from the preceding packet. + const uint8_t cr = _radio->profiles() + ? _radio->profiles()->params(_radio->receiveProfile()).cr : getDefaultTxCodingRate(); + const auto result = _radio->tryRestoreCodingRate(cr >= 4 && cr <= 8 ? cr : outbound_restore_cr); + if (result == RadioParamApplyResult::APPLIED) { outbound_restore_cr = 0; + outbound_restore_retry_at = outbound_restore_recovery_at = 0; + return true; } + outbound_restore_retry_at = now + 250; + if (result == RadioParamApplyResult::BUSY) { + // A legitimate RX/preamble/duty-cycle sleep is not a failed modulation + // write. Let receive service drain it without forcing a radio reset. + outbound_restore_recovery_at = 0; + return false; + } + if (!outbound_restore_recovery_at) outbound_restore_recovery_at = now + RADIO_PREPARE_BUSY_GRACE_MS; + else if ((int32_t)(now - outbound_restore_recovery_at) >= 0) { + _err_flags |= ERR_EVENT_RADIO_WATCHDOG; + _radio->recoverRadio(true); + outbound_restore_recovery_at = now + RADIO_PREPARE_BUSY_GRACE_MS; + } + return false; } bool Dispatcher::startOutboundTransmit() { if (outbound == NULL) return false; + if (!restoreOutboundTxOverrides()) return false; if (!isPacketRadioCurrent(outbound) || _radio->prepareTransmitProfile(outbound->radio_profile, outbound->radio_reply && outbound->radio_reply_force) != RadioParamApplyResult::APPLIED) return false; @@ -286,6 +311,12 @@ bool Dispatcher::getNextQueueWakeDelay(uint32_t& delay_millis) const { return true; } + if (outbound_restore_cr) { + const int32_t restore_delay = (int32_t)(outbound_restore_retry_at - now); + shortest_delay = restore_delay > 0 ? (uint32_t)restore_delay : 0; + found = true; + } + uint32_t scheduled_for; if (_mgr->getNextOutboundTime(now, scheduled_for)) { int32_t signed_queue_delay = (int32_t)(scheduled_for - now); @@ -294,7 +325,7 @@ bool Dispatcher::getNextQueueWakeDelay(uint32_t& delay_millis) const { if (signed_tx_delay > 0 && (uint32_t)signed_tx_delay > outbound_delay) { outbound_delay = (uint32_t)signed_tx_delay; } - shortest_delay = outbound_delay; + if (!found || outbound_delay < shortest_delay) shortest_delay = outbound_delay; found = true; } @@ -384,6 +415,12 @@ void Dispatcher::loop() { return; } if (!millisHasNowPassed(outbound_radio_retry_at)) return; + if (!restoreOutboundTxOverrides()) { + // Release a failed retained retry; restoration itself remains pending + // while normal RX/watchdog service and future queued work continue. + failOutboundTransmit(); + return; + } const auto prepared = _radio->prepareTransmitProfile(outbound->radio_profile, outbound->radio_reply && outbound->radio_reply_force); if (prepared == RadioParamApplyResult::BUSY) { @@ -799,6 +836,9 @@ bool Dispatcher::isTransmitChannelReady(const Packet* packet, uint32_t& retry_de void Dispatcher::checkSend() { if (_radio->isCarrierWaveActive()) return; + // Keep queued packets intact while recovering a failed per-packet CR + // override. RX processing and ordinary watchdogs continue in loop(). + if (!restoreOutboundTxOverrides()) return; const uint32_t now = _ms->getMillis(); if (ota_tx_airtime && now - ota_tx_finished_at >= ota::packetQuietTime(ota_tx_airtime, getOtaSpeedFactor())) { ota_tx_airtime = 0; diff --git a/src/Dispatcher.h b/src/Dispatcher.h index 3b4d605df..4c1a513df 100644 --- a/src/Dispatcher.h +++ b/src/Dispatcher.h @@ -97,6 +97,9 @@ public: * \returns true if the radio accepted the coding rate. */ virtual bool setCodingRate(uint8_t cr) { return false; } + virtual RadioParamApplyResult tryRestoreCodingRate(uint8_t cr) { + return setCodingRate(cr) ? RadioParamApplyResult::APPLIED : RadioParamApplyResult::FAILED; + } /** * \returns true if the previous 'startSendRaw()' completed successfully. @@ -322,7 +325,9 @@ class Dispatcher { bool scheduleOutboundRadioRetry(); bool isTransmitChannelReady(const Packet* packet, uint32_t& retry_delay); void failOutboundTransmit(); - void restoreOutboundTxOverrides(); + bool restoreOutboundTxOverrides(); + uint32_t outbound_restore_retry_at = 0; + uint32_t outbound_restore_recovery_at = 0; void updateTxBudget(); protected: diff --git a/src/helpers/CommonCLI.cpp b/src/helpers/CommonCLI.cpp index 25b99b90f..323523c73 100644 --- a/src/helpers/CommonCLI.cpp +++ b/src/helpers/CommonCLI.cpp @@ -1300,7 +1300,9 @@ void CommonCLI::loadPrefsInt(FILESYSTEM* fs, const char* filename) { _prefs->rx_delay_base = constrain(_prefs->rx_delay_base, 0, 20.0f); _prefs->tx_delay_factor = constrain(_prefs->tx_delay_factor, 0, 2.0f); _prefs->direct_tx_delay_factor = constrain(_prefs->direct_tx_delay_factor, 0, 2.0f); - _prefs->airtime_factor = constrain(_prefs->airtime_factor, 0, 9.0f); + // Duty cycles down to 1% are accepted and persist factor 99, even though + // the legacy `set af` command retains its narrower input range. + _prefs->airtime_factor = constrain(_prefs->airtime_factor, 0, 99.0f); _prefs->freq = constrain(_prefs->freq, 150.0f, 2500.0f); _prefs->bw = isValidLoRaBandwidth(_prefs->bw) ? _prefs->bw : defaultLoRaBandwidth(); _prefs->sf = constrain(_prefs->sf, 5, 12); @@ -2470,7 +2472,7 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, char* command, char* re mesh::cli::normalizeCommandVerb(command); if (mesh::wireless::control().handle(command, reply, 160, millis(), _callbacks->wirelessCommandSource(sender_timestamp))) return; - if (_radio_profiles.handle(command, reply)) return; + if (_radio_profiles.handle(command, reply, 160, sender_timestamp != 0)) return; #if defined(ENABLE_OTA) if (mesh::ota::handleSpeedCommand(command, reply, 160)) return; #endif diff --git a/src/helpers/DeferredCliCommand.h b/src/helpers/DeferredCliCommand.h index e6258c6ea..3763278c6 100644 --- a/src/helpers/DeferredCliCommand.h +++ b/src/helpers/DeferredCliCommand.h @@ -1,6 +1,7 @@ #pragma once #include +#include "RemoteCliReplyCache.h" #include #include #include @@ -14,9 +15,14 @@ namespace mesh { // perform filesystem or cryptographic work. struct DeferredCliCommand { bool pending; - int client_index; + // ACL entries can move or be reused while a request is pending or executing. + // Bind its owner to the complete authenticated key, never an array slot. + uint8_t client_pub_key[PUB_KEY_SIZE]; uint32_t sender_timestamp; uint32_t request_id; + // Command handlers may normalize or tokenize command[] in place. Keep the + // authenticated wire identity separately so retries find the stored result. + uint32_t command_fingerprint; uint8_t path_hash_size; uint8_t radio_profile = 0; uint32_t radio_generation = 0; @@ -24,26 +30,28 @@ struct DeferredCliCommand { char command[MAX_PACKET_PAYLOAD + 1]; DeferredCliCommand() - : pending(false), client_index(-1), sender_timestamp(0), request_id(0), - path_hash_size(1) { + : pending(false), sender_timestamp(0), request_id(0), + command_fingerprint(0), path_hash_size(1) { + memset(client_pub_key, 0, sizeof(client_pub_key)); memset(secret, 0, sizeof(secret)); command[0] = 0; } - bool enqueue(int new_client_index, uint32_t new_sender_timestamp, + bool enqueue(const uint8_t* new_client_pub_key, uint32_t new_sender_timestamp, uint8_t new_path_hash_size, const uint8_t* new_secret, const char* new_command, size_t command_len, uint32_t new_request_id = 0, uint8_t new_radio_profile = 0, uint32_t new_radio_generation = 0) { - if (pending || new_secret == NULL || new_command == NULL + if (pending || new_client_pub_key == NULL || new_secret == NULL || new_command == NULL || command_len >= sizeof(command)) { return false; } - client_index = new_client_index; + memcpy(client_pub_key, new_client_pub_key, sizeof(client_pub_key)); sender_timestamp = new_sender_timestamp; request_id = new_request_id != 0 ? new_request_id : new_sender_timestamp; + command_fingerprint = RemoteCliReplyCache::fingerprint(new_command, command_len); path_hash_size = new_path_hash_size; radio_profile = new_radio_profile; radio_generation = new_radio_generation; @@ -54,21 +62,31 @@ struct DeferredCliCommand { return true; } - bool matches(int other_client_index, uint32_t other_request_id, + bool matches(const uint8_t* other_client_pub_key, uint32_t other_request_id, const char* other_command, size_t other_command_len) const { - return pending && other_command != NULL - && client_index == other_client_index + return pending && other_client_pub_key != NULL && other_command != NULL + && memcmp(client_pub_key, other_client_pub_key, sizeof(client_pub_key)) == 0 && request_id == other_request_id && other_command_len < sizeof(command) && command[other_command_len] == 0 && memcmp(command, other_command, other_command_len) == 0; } + template int findClientIndex(const ACL& acl) const { + if (!pending) return -1; + for (int i = 0; i < acl.getNumClients(); ++i) { + if (memcmp(client_pub_key, acl.getClientByIdx(i)->id.pub_key, + sizeof(client_pub_key)) == 0) return i; + } + return -1; + } + void clear() { pending = false; - client_index = -1; + memset(client_pub_key, 0, sizeof(client_pub_key)); sender_timestamp = 0; request_id = 0; + command_fingerprint = 0; path_hash_size = 1; radio_profile = 0; radio_generation = 0; diff --git a/src/helpers/RadioProfileCLI.cpp b/src/helpers/RadioProfileCLI.cpp index b73acadc6..b76454a2e 100644 --- a/src/helpers/RadioProfileCLI.cpp +++ b/src/helpers/RadioProfileCLI.cpp @@ -12,6 +12,9 @@ constexpr size_t ImageSize = 24; const char* const ImagePath = "/radio_profiles"; const char* const BackupPath = "/radio_profiles.bak"; const char* const TempPath = "/radio_profiles.tmp"; +// Never considered a boot-time committed image. Losing power before the +// acknowledgement leaves the previous saved radio untouched. +const char* const ReplyPath = "/radio_profiles.reply"; uint32_t checksum(const uint8_t* p, size_t n) { uint32_t crc = 0xffffffffU; while (n--) { @@ -91,6 +94,11 @@ bool RadioProfileCLI::writeImage(const char* path, const uint8_t* bytes, size_t } bool RadioProfileCLI::save(const RadioProfileConfig& config, uint16_t preamble, RadioCrossMode cross) { + return prepareSavedImage(TempPath, config, preamble, cross) && commitSavedImage(TempPath); +} + +bool RadioProfileCLI::prepareSavedImage(const char* path, const RadioProfileConfig& config, + uint16_t preamble, RadioCrossMode cross) { if (!fs_ || hold_) return false; uint8_t image[ImageSize] = {'R', '2', 1, (uint8_t)config.mode}; memcpy(image + 4, &config.params.freq, 4); @@ -102,12 +110,16 @@ bool RadioProfileCLI::save(const RadioProfileConfig& config, uint16_t preamble, image[19] = reply_setting_; const uint32_t crc = checksum(image, ImageSize - 4); memcpy(image + ImageSize - 4, &crc, 4); - if (!writeImage(TempPath, image, sizeof(image))) return false; + return writeImage(path, image, sizeof(image)); +} + +bool RadioProfileCLI::commitSavedImage(const char* path) { + if (!fs_ || hold_) return false; if (fs_->exists(ImagePath)) { if (fs_->exists(BackupPath) && !fs_->remove(BackupPath)) return false; if (!fs_->rename(ImagePath, BackupPath)) return false; } - if (!fs_->rename(TempPath, ImagePath)) { + if (!fs_->rename(path, ImagePath)) { if (fs_->exists(BackupPath) && !fs_->rename(BackupPath, ImagePath)) hold_ = true; return false; } @@ -159,6 +171,7 @@ bool RadioProfileCLI::savePrimaryPreamble(uint16_t symbols) { if (symbols && (symbols < 8 || symbols > RadioProfiles::MaxPreamble)) return false; if (!radio_ || !radio_->profiles()) return symbols == 0; if (symbols == primary_preamble_) return true; + if (hasReplyMutation()) return false; if (!save(saved_, symbols, cross_)) return false; primary_preamble_ = symbols; return true; @@ -184,11 +197,73 @@ void RadioProfileCLI::publish() { radio_->profiles()->setSecondary(temp_active_ ? temporary_ : saved_, temp_active_); } +bool RadioProfileCLI::stageRemoteMutation(RemoteMutation kind, const RadioProfileConfig& config, + uint32_t duration_ms, uint8_t delete_mask) { + if (hasReplyMutation()) return false; + remote_mutation_ = kind; + ++remote_generation_; + remote_config_ = config; + remote_duration_ms_ = duration_ms; + remote_start_ms_ = millis() + 2000; + remote_delete_mask_ = delete_mask; + remote_delivered_ = false; + remote_commit_retry_ms_ = 0; + return true; +} + +bool RadioProfileCLI::finishReplyMutation(bool delivered) { + if (delivered) { remote_delivered_ = hasReplyMutation(); return true; } + const auto kind = remote_mutation_; + remote_mutation_ = RemoteMutation::None; + remote_delivered_ = false; + // This file was never committed, so cancellation needs no risky rollback of + // the only saved image. Failure to remove scratch cannot activate it at boot. + return (kind != RemoteMutation::Saved && kind != RemoteMutation::Off) + || !fs_ || !fs_->exists(ReplyPath) || fs_->remove(ReplyPath); +} + +bool RadioProfileCLI::applyReplyMutation() { + const auto kind = remote_mutation_; + if ((kind == RemoteMutation::Saved || kind == RemoteMutation::Off) + && !commitSavedImage(ReplyPath)) return false; + remote_mutation_ = RemoteMutation::None; + remote_delivered_ = false; + if (kind == RemoteMutation::None) return true; + if (kind == RemoteMutation::Temporary) { + pending_temporary_ = remote_config_; + pending_duration_ms_ = remote_duration_ms_; + temp_start_ms_ = remote_start_ms_; // waiting never extends the hard lease + temp_pending_ = true; + } else if (kind == RemoteMutation::Saved) { + saved_ = remote_config_; + publish_pending_ = true; publish_after_ms_ = millis(); + } else if (kind == RemoteMutation::DeleteTemp) { + for (unsigned i = 0; i < 4; ++i) if (remote_delete_mask_ & (1U << i)) { + if (schedules_[4+i].started) { temp_active_ = false; temp_remaining_ms_ = 0; } + schedules_[4+i] = {}; + } + } else { + if (kind == RemoteMutation::Off) { saved_ = {}; schedule_retry_ms_ = 0; } + temp_pending_ = temp_active_ = false; temp_remaining_ms_ = 0; + for (auto& s : schedules_) if (kind == RemoteMutation::Off || s.temporary) s = {}; + } + // loop() owns publishing, after all completion/failure callbacks have + // returned and Dispatcher no longer owns either response packet. + return true; +} + void RadioProfileCLI::loop() { if (!radio_ || !radio_->profiles()) return; const uint32_t now = millis(), elapsed = now - last_ms_; last_ms_ = now; - if (temp_pending_ && (int32_t)(now - temp_start_ms_) >= 0) { + remote_commit_retry_ms_ = elapsed >= remote_commit_retry_ms_ ? 0 : remote_commit_retry_ms_ - elapsed; + if (remote_delivered_ && !remote_commit_retry_ms_ && !applyReplyMutation()) { + // Keep the accepted operation pending and the old channel live if the + // post-ACK atomic rename fails. Never report/apparently apply an unsaved + // tuple, and avoid a hot storage retry loop. + remote_commit_retry_ms_ = 1000; + } + if (!hasReplyMutation() && temp_pending_ && (int32_t)(now - temp_start_ms_) >= 0) { temporary_ = pending_temporary_; const uint32_t late = now - temp_start_ms_; temp_remaining_ms_ = late < pending_duration_ms_ ? pending_duration_ms_ - late : 0; @@ -207,7 +282,7 @@ void RadioProfileCLI::loop() { } } schedule_retry_ms_ = elapsed >= schedule_retry_ms_ ? 0 : schedule_retry_ms_ - elapsed; - while (!schedule_retry_ms_) { + while (!schedule_retry_ms_ && !hasReplyMutation()) { Schedule* next = nullptr; for (auto& s : schedules_) { if (s.active && !s.temporary && epoch >= s.start && (!next || s.start < next->start)) next = &s; @@ -222,7 +297,7 @@ void RadioProfileCLI::loop() { saved_ = next->config; *next = {}; } for (auto& s : schedules_) { - if (!s.active || !s.temporary || s.started || epoch < s.start || temp_active_ || temp_pending_) continue; + if (!s.active || !s.temporary || s.started || epoch < s.start || temp_active_ || temp_pending_ || hasReplyMutation()) continue; temporary_ = s.config; temp_remaining_ms_ = remainingMillis(s.end_ms, now); const uint32_t epoch_left = (s.end - epoch) * 1000UL; @@ -303,7 +378,7 @@ void RadioProfileCLI::formatConfig(char* reply, size_t capacity, const RadioProf appendChirpWarning(reply, capacity, preview); } -bool RadioProfileCLI::handle(const char* command, char* reply, size_t capacity) { +bool RadioProfileCLI::handle(const char* command, char* reply, size_t capacity, bool remote_origin) { if (handleCarrierWaveCommand(radio_, command, reply, capacity)) return true; const char* text = command; enum { Get, Set, Delete } verb = Get; @@ -328,6 +403,13 @@ bool RadioProfileCLI::handle(const char* command, char* reply, size_t capacity) if (!base && !temporary && !scheduled && !crossing && !replies && !status && !scan && !timing) return false; if (!radio_ || !radio_->profiles()) { snprintf(reply, capacity, "Error: radio profiles unsupported"); return true; } if (command == text && *args && verb == Get && (temporary || base)) verb = Set; + if (remote_origin && !remote_command_ + && (((base || temporary) && verb == Set) || (scheduled_temp && verb == Delete))) { + snprintf(reply, capacity, "Error: this role requires local USB for radio2 changes"); return true; + } + if (hasReplyMutation() && verb != Get) { + snprintf(reply, capacity, "Error: radio acknowledgement pending"); return true; + } if (replies) { const auto* p = radio_->profiles(); if (verb == Get && !*args) { @@ -401,6 +483,11 @@ bool RadioProfileCLI::handle(const char* command, char* reply, size_t capacity) } if (status) { if (verb != Get || *args) { snprintf(reply, capacity, "Error: radio2.status is read-only"); return true; } + if (hasReplyMutation()) { + snprintf(reply, capacity, "> radio2 change pending %s; previous channel retained", + remote_delivered_ ? "storage commit (retrying)" : "reply transmission"); + return true; + } const auto& p = *radio_->profiles(); snprintf(reply, capacity, "> %s; RX=%lu,%lu TX=%lu,%lu switches=%lu errors=%lu max=%luus preamble=%u,%u", modeName(p.secondary.mode), (unsigned long)p.rx_packets[0], (unsigned long)p.rx_packets[1], @@ -416,9 +503,16 @@ bool RadioProfileCLI::handle(const char* command, char* reply, size_t capacity) return true; } if ((base || temporary) && verb == Set && !strcmp(args, "off")) { - if (base && !save({}, primary_preamble_, cross_)) { + if (base && !(remote_command_ ? prepareSavedImage(ReplyPath, {}, primary_preamble_, cross_) + : save({}, primary_preamble_, cross_))) { snprintf(reply, capacity, "Error: settings could not be saved"); return true; } + if (remote_command_) { + stageRemoteMutation(base ? RemoteMutation::Off : RemoteMutation::TempOff); + snprintf(reply, capacity, "OK - %s after reply", !base && saved_.mode != RadioProfileMode::Off + ? "saved radio2 restored" : "single radio"); + return true; + } if (base) { saved_ = {}; schedule_retry_ms_ = 0; } temp_pending_ = temp_active_ = false; temp_remaining_ms_ = 0; for (auto& s : schedules_) if (base || s.temporary) s = {}; @@ -433,6 +527,10 @@ bool RadioProfileCLI::handle(const char* command, char* reply, size_t capacity) } const unsigned first = scheduled_temp ? 4 : 0; if (verb == Delete) { + if (remote_command_ && scheduled_temp) { + stageRemoteMutation(RemoteMutation::DeleteTemp, {}, 0, index ? 1U << (index - 1) : 15); + snprintf(reply, capacity, "OK - schedule cleared after reply"); return true; + } if (!scheduled_temp) schedule_retry_ms_ = 0; for (unsigned i = 0; i < 4; ++i) if (!index || index == i + 1) { if (schedules_[first+i].started) { @@ -517,17 +615,25 @@ bool RadioProfileCLI::handle(const char* command, char* reply, size_t capacity) for (const auto& s : schedules_) if (s.active && s.temporary) { snprintf(reply, capacity, "Error: clear tempradioat2 schedules first"); return true; } - pending_temporary_ = config; pending_duration_ms_ = minutes * 60000UL; - temp_start_ms_ = millis() + 2000; temp_pending_ = true; + if (remote_command_) stageRemoteMutation(RemoteMutation::Temporary, config, minutes * 60000UL); + else { + pending_temporary_ = config; pending_duration_ms_ = minutes * 60000UL; + temp_start_ms_ = millis() + 2000; temp_pending_ = true; + } // An existing session remains active until the new request can take over. snprintf(reply, capacity, "OK - tempradio2 %lud%luh%lum (%lu min); preamble=%u", (unsigned long)(minutes/1440), (unsigned long)(minutes/60%24), (unsigned long)(minutes%60), (unsigned long)minutes, preview.preamble(1, 32)); - } else if (save(config, primary_preamble_, cross_)) { - saved_ = config; - publish_pending_ = true; publish_after_ms_ = millis() + 2000; - publish(); - snprintf(reply, capacity, "OK - radio2 %s; preamble=%u", modeName(config.mode), preview.preamble(1, 32)); + } else if (remote_command_ ? prepareSavedImage(ReplyPath, config, primary_preamble_, cross_) + : save(config, primary_preamble_, cross_)) { + if (remote_command_) stageRemoteMutation(RemoteMutation::Saved, config); + else { + saved_ = config; + publish_pending_ = true; publish_after_ms_ = millis() + 2000; + publish(); + } + snprintf(reply, capacity, "OK - radio2 %s%s; preamble=%u", modeName(config.mode), + remote_command_ ? " pending after reply" : "", preview.preamble(1, 32)); } else snprintf(reply, capacity, "Error: settings could not be saved"); if (!strncmp(reply, "OK", 2)) appendChirpWarning(reply, capacity, preview); return true; diff --git a/src/helpers/RadioProfileCLI.h b/src/helpers/RadioProfileCLI.h index df6299434..af7cce574 100644 --- a/src/helpers/RadioProfileCLI.h +++ b/src/helpers/RadioProfileCLI.h @@ -37,7 +37,21 @@ class RadioProfileCLI { bool temp_pending_ = false, temp_active_ = false; bool hold_ = false; bool publish_pending_ = false; + enum class RemoteMutation : uint8_t { None, Saved, Temporary, Off, TempOff, DeleteTemp }; + RemoteMutation remote_mutation_ = RemoteMutation::None; + RadioProfileConfig remote_config_; + uint32_t remote_duration_ms_ = 0, remote_start_ms_ = 0; + uint32_t remote_commit_retry_ms_ = 0; + uint32_t remote_generation_ = 0; + uint8_t remote_delete_mask_ = 0; + bool remote_command_ = false, remote_delivered_ = false; + bool stageRemoteMutation(RemoteMutation kind, const RadioProfileConfig& config = {}, + uint32_t duration_ms = 0, uint8_t delete_mask = 0); bool save(const RadioProfileConfig& config, uint16_t preamble, RadioCrossMode cross); + bool prepareSavedImage(const char* path, const RadioProfileConfig& config, + uint16_t preamble, RadioCrossMode cross); + bool commitSavedImage(const char* path); + bool applyReplyMutation(); bool readImage(const char* path, uint8_t* bytes, size_t size); bool writeImage(const char* path, const uint8_t* bytes, size_t size); void publish(); @@ -46,7 +60,14 @@ class RadioProfileCLI { public: void begin(FILESYSTEM* fs, Radio* radio, RTCClock* rtc, bool infrastructure_replies = false); void loop(); - bool handle(const char* command, char* reply, size_t capacity = 160); + bool handle(const char* command, char* reply, size_t capacity = 160, bool remote_origin = false); + // Remote infrastructure commands must keep both RX-origin generations alive + // until their exact response packets finish, not merely wait a fixed delay. + void beginReplyCommand() { remote_command_ = true; } + void endReplyCommand() { remote_command_ = false; } + bool hasReplyMutation() const { return remote_mutation_ != RemoteMutation::None; } + uint32_t replyMutationGeneration() const { return remote_generation_; } + bool finishReplyMutation(bool delivered); uint16_t primaryPreamble() const { return primary_preamble_; } bool savePrimaryPreamble(uint16_t symbols); bool acceptsPrimary(float freq, float bw, uint8_t sf, uint8_t cr, uint16_t preamble) const; diff --git a/src/helpers/RemoteCliReplyCache.h b/src/helpers/RemoteCliReplyCache.h index 41f82e7f1..c01defef7 100644 --- a/src/helpers/RemoteCliReplyCache.h +++ b/src/helpers/RemoteCliReplyCache.h @@ -36,7 +36,8 @@ public: } bool remember(const uint8_t* sender_pub_key, uint32_t request_timestamp, - uint32_t command_fingerprint, const char* response) { + uint32_t command_fingerprint, const char* response, + bool authoritative_reply = false) { if (sender_pub_key == NULL || response == NULL) return false; Entry* target = NULL; @@ -66,18 +67,21 @@ public: memcpy(target->response, response, response_len); target->response[response_len] = 0; target->valid = true; + target->authoritative_reply = authoritative_reply; return true; } bool lookup(const uint8_t* sender_pub_key, uint32_t request_timestamp, uint32_t command_fingerprint, - const char** response = NULL) const { + const char** response = NULL, bool* authoritative_reply = NULL) const { + if (authoritative_reply) *authoritative_reply = false; for (size_t i = 0; i < ENTRY_COUNT; ++i) { if (!entryMatches(entries_[i], sender_pub_key, request_timestamp, command_fingerprint)) { continue; } if (response != NULL) *response = entries_[i].response; + if (authoritative_reply) *authoritative_reply = entries_[i].authoritative_reply; return true; } return false; @@ -106,6 +110,7 @@ public: private: struct Entry { bool valid; + bool authoritative_reply; uint8_t sender_pub_key[PUB_KEY_SIZE]; uint32_t request_timestamp; uint32_t command_fingerprint; diff --git a/src/helpers/TempRadioReplyBarrier.h b/src/helpers/TempRadioReplyBarrier.h index 3963cb073..a3f47dfcf 100644 --- a/src/helpers/TempRadioReplyBarrier.h +++ b/src/helpers/TempRadioReplyBarrier.h @@ -36,6 +36,10 @@ public: void clear() { _packet = _copy = nullptr; _armed = _succeeded = false; } bool waiting() const { return _armed && (_packet || _copy); } + bool succeeded() const { return _armed && _succeeded; } + bool contains(const void* packet) const { + return _armed && packet && (packet == _packet || packet == _copy); + } bool complete(const void* packet) { if (!remove(packet)) return false; diff --git a/src/helpers/esp32/SerialWifiInterface.cpp b/src/helpers/esp32/SerialWifiInterface.cpp index 6792cc0f9..a6a48ae75 100644 --- a/src/helpers/esp32/SerialWifiInterface.cpp +++ b/src/helpers/esp32/SerialWifiInterface.cpp @@ -2,6 +2,8 @@ #include "../CompanionFrameQueue.h" #include +static constexpr uint32_t WIFI_FRAME_TIMEOUT_MS = 5000; + void SerialWifiInterface::clearBuffers() { held_queue_len = send_queue_len = 0; send_offset = 0; @@ -19,7 +21,23 @@ void SerialWifiInterface::expireHeldQueue(uint32_t now) { } void SerialWifiInterface::loop() { - expireHeldQueue((uint32_t)millis()); + const uint32_t now = (uint32_t)millis(); + expireHeldQueue(now); + if (deviceConnected && (!client.connected() + || (receive_pending && (uint32_t)(now - receive_started) >= WIFI_FRAME_TIMEOUT_MS))) { + disconnectClient(true); + } +} + +void SerialWifiInterface::disconnectClient(bool cancel_session) { + const bool was_connected = deviceConnected; + deviceConnected = false; + if (client) client.stop(); + resetReceivedFrameHeader(); + send_offset = 0; // A reconnect must replay the whole frame, never its suffix. + if (cancel_session && was_connected && session_changed != nullptr) { + session_changed(session_context); + } } void SerialWifiInterface::selectClient(const IPAddress& ip, uint32_t now) { @@ -28,8 +46,6 @@ void SerialWifiInterface::selectClient(const IPAddress& ip, uint32_t now) { // IP. An incomplete outgoing frame is retained whole, not just its suffix. send_offset = 0; if (queue_has_ip && queue_ip != ip) { - if (session_changed != nullptr) session_changed(session_context); - if (held_queue_len > 0 && held_ip == ip) { // The previous owner returned: restore its queue and park the displaced // active queue in the same fixed storage. Only one old IP is retained. @@ -83,10 +99,7 @@ void SerialWifiInterface::enable() { void SerialWifiInterface::disable() { _isEnabled = false; - deviceConnected = false; - if (client) client.stop(); - if (queue_has_ip && session_changed != nullptr) session_changed(session_context); - resetReceivedFrameHeader(); + disconnectClient(true); clearBuffers(); } @@ -127,10 +140,12 @@ bool SerialWifiInterface::hasReceivedFrameHeader() { void SerialWifiInterface::resetReceivedFrameHeader() { received_frame_header.type = 0; received_frame_header.length = 0; + receive_pending = false; + receive_started = 0; } size_t SerialWifiInterface::checkRecvFrame(uint8_t dest[]) { - expireHeldQueue((uint32_t)millis()); + loop(); if (!_isEnabled) return 0; // check if new client connected auto newClient = server.available(); @@ -138,8 +153,7 @@ size_t SerialWifiInterface::checkRecvFrame(uint8_t dest[]) { const IPAddress new_ip = newClient.remoteIP(); // disconnect existing client - deviceConnected = false; - client.stop(); + disconnectClient(!queue_has_ip || queue_ip != new_ip); // Partition queues and cancel the displaced session before exposing the // new connection to response producers or consuming its first command. @@ -160,7 +174,7 @@ size_t SerialWifiInterface::checkRecvFrame(uint8_t dest[]) { } } else { if (deviceConnected) { - deviceConnected = false; + disconnectClient(true); WIFI_DEBUG_PRINTLN("Disconnected"); } } @@ -189,67 +203,35 @@ size_t SerialWifiInterface::checkRecvFrame(uint8_t dest[]) { memset(&send_queue[send_queue_len], 0, sizeof(send_queue[send_queue_len])); } else { - // check if we are waiting for a frame header - if(!hasReceivedFrameHeader()){ - - // make sure we have received enough bytes for a frame header - // 3 bytes frame header = (1 byte frame type) + (2 bytes frame length as unsigned 16-bit little endian) - int frame_header_length = 3; - if(client.available() >= frame_header_length){ - - // read frame header - client.readBytes(&received_frame_header.type, 1); - client.readBytes((uint8_t*)&received_frame_header.length, 2); - - } - + if (!receive_pending && client.available() > 0) { + receive_pending = true; + receive_started = (uint32_t)millis(); } - - // check if we have received a frame header - if(hasReceivedFrameHeader()){ - - // make sure we have received enough bytes for the required frame length - int available = client.available(); - int frame_type = received_frame_header.type; - int frame_length = received_frame_header.length; - if(frame_length > available){ - WIFI_DEBUG_PRINTLN("Waiting for %d more bytes", frame_length - available); + if (!hasReceivedFrameHeader()) { + if (client.available() < 3) return 0; + uint8_t header[3]; + if (client.readBytes(header, sizeof(header)) != sizeof(header)) { + disconnectClient(true); return 0; } - - // skip frames that are larger than MAX_FRAME_SIZE - if(frame_length > MAX_FRAME_SIZE){ - WIFI_DEBUG_PRINTLN("Skipping frame: length=%d is larger than MAX_FRAME_SIZE=%d", frame_length, MAX_FRAME_SIZE); - while(frame_length > 0){ - uint8_t skip[1]; - int skipped = client.read(skip, 1); - frame_length -= skipped; - } - resetReceivedFrameHeader(); + received_frame_header.type = header[0]; + received_frame_header.length = uint16_t(header[1]) | (uint16_t(header[2]) << 8); + // Validate before waiting for any payload. Closing an invalid stream + // avoids unbounded discard loops and interpreting its tail as commands. + if (received_frame_header.type != '<' || received_frame_header.length == 0 + || received_frame_header.length > MAX_FRAME_SIZE) { + disconnectClient(true); return 0; } - - // skip frames that are not expected type - // '<' is 0x3c which indicates a frame sent from app to radio - if(frame_type != '<'){ - WIFI_DEBUG_PRINTLN("Skipping frame: type=0x%x is unexpected", frame_type); - while(frame_length > 0){ - uint8_t skip[1]; - int skipped = client.read(skip, 1); - frame_length -= skipped; - } - resetReceivedFrameHeader(); - return 0; - } - - // read frame data to provided buffer - client.readBytes(dest, frame_length); - - // ready for next frame - resetReceivedFrameHeader(); - return frame_length; - } + const size_t frame_length = received_frame_header.length; + if (client.available() < (int)frame_length) return 0; + if (dest == nullptr || client.readBytes(dest, frame_length) != frame_length) { + disconnectClient(true); + return 0; // Never dispatch a truncated command, even after a short read. + } + resetReceivedFrameHeader(); + return frame_length; } } diff --git a/src/helpers/esp32/SerialWifiInterface.h b/src/helpers/esp32/SerialWifiInterface.h index bbae92473..a7a86f9ea 100644 --- a/src/helpers/esp32/SerialWifiInterface.h +++ b/src/helpers/esp32/SerialWifiInterface.h @@ -36,12 +36,15 @@ class SerialWifiInterface : public BaseSerialInterface { bool queue_has_ip = false; uint32_t held_since = 0; size_t send_offset = 0; + uint32_t receive_started = 0; + bool receive_pending = false; void (*session_changed)(void*) = nullptr; void* session_context = nullptr; void clearBuffers(); void expireHeldQueue(uint32_t now); void selectClient(const IPAddress& ip, uint32_t now); + void disconnectClient(bool cancel_session); protected: @@ -57,8 +60,9 @@ public: void begin(int port); void end(); - // Called synchronously while disconnected, before a different IP can send - // commands or receive replies. The owner can cancel route-bound operations. + // Called synchronously while disconnected, on teardown or before a different + // IP can send commands/receive replies. Same-IP live replacement preserves + // the existing owner; actual disconnects cancel its pending operations. void setSessionChangedCallback(void (*callback)(void*), void* context) { session_changed = callback; session_context = context; diff --git a/src/helpers/ethernet/RAK13800/RAK13800EthernetInterface.cpp b/src/helpers/ethernet/RAK13800/RAK13800EthernetInterface.cpp index 127053f17..51455ecb6 100644 --- a/src/helpers/ethernet/RAK13800/RAK13800EthernetInterface.cpp +++ b/src/helpers/ethernet/RAK13800/RAK13800EthernetInterface.cpp @@ -84,28 +84,35 @@ size_t RAK13800EthernetInterface::write(const uint8_t *buf, size_t size) { } bool RAK13800EthernetInterface::isConnected() const { - return _isConnected; + return isEnabled() && _isConnected; +} + +void RAK13800EthernetInterface::disconnectClient() { + _isConnected = false; + client.stop(); } void RAK13800EthernetInterface::loop() { Ethernet.maintain(); - const bool wasConnected = _isConnected; + if (!isEnabled()) return; + if (_isConnected && !client.connected()) { + _isConnected = false; + onClientDisconnected(); + } auto newClient = server.accept(); if (newClient) { IPAddress remoteIp = newClient.remoteIP(); uint16_t remotePort = newClient.remotePort(); ETHERNET_DEBUG_PRINTLN("New client accepted %u.%u.%u.%u:%u", remoteIp[0], remoteIp[1], remoteIp[2], remoteIp[3], remotePort); - if (client) { - ETHERNET_DEBUG_PRINTLN("Closing previous client"); - client.stop(); - } + disconnectClient(); + onClientConnected((uint32_t)remoteIp); + // Finish old-owner cancellation before publishing the replacement socket. client = newClient; - onClientConnected(); } _isConnected = client.connected(); - if (wasConnected && !_isConnected) onClientDisconnected(); + if (!_isConnected) onClientDisconnected(); } diff --git a/src/helpers/ethernet/RAK13800/RAK13800EthernetInterface.h b/src/helpers/ethernet/RAK13800/RAK13800EthernetInterface.h index 34bb75877..03288bb45 100644 --- a/src/helpers/ethernet/RAK13800/RAK13800EthernetInterface.h +++ b/src/helpers/ethernet/RAK13800/RAK13800EthernetInterface.h @@ -10,6 +10,9 @@ class RAK13800EthernetInterface : public SerialEthernetInterface { EthernetServer server; EthernetClient client; + protected: + void disconnectClient() override; + public: RAK13800EthernetInterface() : server(EthernetServer(ETHERNET_TCP_PORT)) { _isConnected = false; diff --git a/src/helpers/ethernet/SerialEthernetInterface.cpp b/src/helpers/ethernet/SerialEthernetInterface.cpp index 02f3e7a75..ec5b9220f 100644 --- a/src/helpers/ethernet/SerialEthernetInterface.cpp +++ b/src/helpers/ethernet/SerialEthernetInterface.cpp @@ -6,6 +6,10 @@ #define RECV_STATE_LEN1_FOUND 2 #define RECV_STATE_LEN2_FOUND 3 +#if !ETHERNET_RAW_LINE +static constexpr uint32_t ETHERNET_FRAME_TIMEOUT_MS = 5000; +#endif + bool SerialEthernetInterface::begin() { return true; } @@ -18,21 +22,32 @@ void SerialEthernetInterface::enable() { void SerialEthernetInterface::disable() { _isEnabled = false; + disconnectClient(); + onClientDisconnected(); clearBuffers(); } +bool SerialEthernetInterface::isConnected() const { + return _isEnabled && session_active; +} + size_t SerialEthernetInterface::writeFrame(const uint8_t src[], size_t len) { if (len > MAX_FRAME_SIZE) { ETHERNET_DEBUG_PRINTLN("writeFrame(), frame too big, len=%d\n", len); return 0; } - if (isConnected() && len > 0) { - if (!mesh::enqueueCompanionFrame(send_queue, send_queue_len, FRAME_QUEUE_SIZE, + if (_isEnabled && isConnected() && src != nullptr && len > 0) { + // A partly written frame must remain at the head even if a new response + // displaces or reorders lower-priority frames in the waiting tail. + const int pinned = send_offset != 0 ? 1 : 0; + int tail_len = send_queue_len - pinned; + if (!mesh::enqueueCompanionFrame(send_queue + pinned, tail_len, FRAME_QUEUE_SIZE - pinned, src, len)) { ETHERNET_DEBUG_PRINTLN("writeFrame(), send_queue is full!"); return 0; } + send_queue_len = tail_len + pinned; return len; } return 0; @@ -43,37 +58,66 @@ bool SerialEthernetInterface::isWriteBusy() const { } bool SerialEthernetInterface::isReadBusy() const { - return _state != RECV_STATE_IDLE; + return _state != RECV_STATE_IDLE || _rx_len > 0; } bool SerialEthernetInterface::hasPendingIO() const { // Ethernet is polled, so an active connection must keep receiving loop // service even when no complete frame is queued yet. - return isConnected() || send_queue_len > 0 || _state != RECV_STATE_IDLE; + return _isEnabled && isConnected(); } -void SerialEthernetInterface::onClientConnected() { - clearBuffers(); +void SerialEthernetInterface::onClientConnected(uint32_t remote_ip) { + const bool different_owner = queue_has_ip && queue_ip != remote_ip; + if (different_owner) { + // Cancel response producers while the displaced route is unavailable. + // Ethernet has only its original four-frame queue: unlike WiFi there is + // no spare queue for a different IP's backlog, so discard it on takeover. + onClientDisconnected(); + send_queue_len = 0; + } + resetInput(); + send_offset = 0; // Replay a complete frame on a fresh TCP stream. + queue_ip = remote_ip; + queue_has_ip = true; + session_active = true; } void SerialEthernetInterface::onClientDisconnected() { - clearBuffers(); + const bool was_active = session_active; + session_active = false; + resetInput(); + send_offset = 0; + if (was_active && session_changed != nullptr) session_changed(session_context); +} + +void SerialEthernetInterface::rejectInput() { + disconnectClient(); + onClientDisconnected(); } size_t SerialEthernetInterface::checkRecvFrame(uint8_t dest[]) { - + if (!_isEnabled) return 0; if (isConnected()) { +#if !ETHERNET_RAW_LINE + if (isReadBusy() && (uint32_t)((uint32_t)millis() - _rx_started) >= ETHERNET_FRAME_TIMEOUT_MS) { + rejectInput(); + return 0; + } +#endif if (send_queue_len > 0) { // first, check send queue _last_write = millis(); int len = send_queue[0].len; + uint8_t pkt[MAX_FRAME_SIZE + 3]; #if ETHERNET_RAW_LINE ETHERNET_DEBUG_PRINTLN("TX line len=%d", len); - write(send_queue[0].buf, len); - write((const uint8_t *)"\r\n", 2); + memcpy(pkt, send_queue[0].buf, len); + pkt[len] = '\r'; + pkt[len + 1] = '\n'; + const size_t total = len + 2U; #else - uint8_t pkt[3+len]; // use same header as serial interface so client can delimit frames pkt[0] = '>'; pkt[1] = (len & 0xFF); // LSB pkt[2] = (len >> 8); // MSB @@ -82,8 +126,13 @@ size_t SerialEthernetInterface::checkRecvFrame(uint8_t dest[]) { #if ETHERNET_DEBUG_LOGGING && ARDUINO ETHERNET_DEBUG_PRINTLN("TX frame len=%d", len); #endif - write(pkt, 3 + len); + const size_t total = len + 3U; #endif + const size_t remaining = total - send_offset; + const size_t written = write(pkt + send_offset, remaining); + send_offset += written < remaining ? written : remaining; + if (send_offset < total) return 0; + send_offset = 0; send_queue_len--; for (int i = 0; i < send_queue_len; i++) { // delete top item from queue send_queue[i] = send_queue[i + 1]; @@ -99,20 +148,25 @@ size_t SerialEthernetInterface::checkRecvFrame(uint8_t dest[]) { continue; } uint16_t out_len = _rx_len; - if (out_len > MAX_FRAME_SIZE) out_len = MAX_FRAME_SIZE; + if (dest == nullptr) { rejectInput(); return 0; } memcpy(dest, _rx_buf, out_len); - _rx_len = 0; + resetInput(); return out_len; } if (_rx_len < MAX_FRAME_SIZE) { + if (_rx_len == 0) _rx_started = (uint32_t)millis(); _rx_buf[_rx_len] = (uint8_t)c; _rx_len++; + } else { + rejectInput(); // Never execute the prefix of an overlong command. + return 0; } #else switch (_state) { case RECV_STATE_IDLE: if (c == '<') { _state = RECV_STATE_HDR_FOUND; + _rx_started = (uint32_t)millis(); } break; case RECV_STATE_HDR_FOUND: @@ -121,8 +175,12 @@ size_t SerialEthernetInterface::checkRecvFrame(uint8_t dest[]) { break; case RECV_STATE_LEN1_FOUND: _frame_len |= ((uint16_t)c) << 8; + if (_frame_len == 0 || _frame_len > MAX_FRAME_SIZE) { + rejectInput(); + return 0; + } _rx_len = 0; - _state = _frame_len > 0 ? RECV_STATE_LEN2_FOUND : RECV_STATE_IDLE; + _state = RECV_STATE_LEN2_FOUND; break; default: if (_rx_len < MAX_FRAME_SIZE) { @@ -130,15 +188,14 @@ size_t SerialEthernetInterface::checkRecvFrame(uint8_t dest[]) { } _rx_len++; if (_rx_len >= _frame_len) { - if (_frame_len > MAX_FRAME_SIZE) { - _frame_len = MAX_FRAME_SIZE; - } #if ETHERNET_DEBUG_LOGGING && ARDUINO ETHERNET_DEBUG_PRINTLN("RX frame len=%d", _frame_len); #endif - memcpy(dest, _rx_buf, _frame_len); - _state = RECV_STATE_IDLE; - return _frame_len; + if (dest == nullptr) { rejectInput(); return 0; } + const size_t out_len = _frame_len; + memcpy(dest, _rx_buf, out_len); + resetInput(); + return out_len; } } #endif diff --git a/src/helpers/ethernet/SerialEthernetInterface.h b/src/helpers/ethernet/SerialEthernetInterface.h index bed25d35e..1283c7a03 100644 --- a/src/helpers/ethernet/SerialEthernetInterface.h +++ b/src/helpers/ethernet/SerialEthernetInterface.h @@ -16,6 +16,13 @@ class SerialEthernetInterface : public BaseSerialInterface { uint16_t _frame_len; uint16_t _rx_len; uint8_t _rx_buf[MAX_FRAME_SIZE]; + uint32_t _rx_started = 0; + size_t send_offset = 0; + bool session_active = false; + bool queue_has_ip = false; + uint32_t queue_ip = 0; + void (*session_changed)(void*) = nullptr; + void* session_context = nullptr; struct Frame { uint8_t len; @@ -28,12 +35,21 @@ class SerialEthernetInterface : public BaseSerialInterface { void clearBuffers() { send_queue_len = 0; + queue_has_ip = false; + resetInput(); + send_offset = 0; + } + + void resetInput() { _state = 0; _frame_len = 0; _rx_len = 0; + _rx_started = 0; } + void rejectInput(); protected: + virtual void disconnectClient() = 0; public: SerialEthernetInterface() { @@ -46,8 +62,14 @@ class SerialEthernetInterface : public BaseSerialInterface { } bool begin(); - void onClientConnected(); + // Drivers call this while isConnected() is false and BEFORE exposing the + // new socket. Same-IP live replacement retains the existing logical owner. + void onClientConnected(uint32_t remote_ip); void onClientDisconnected(); + void setSessionChangedCallback(void (*callback)(void*), void* context) { + session_changed = callback; + session_context = context; + } // BaseSerialInterface methods void enable() override; diff --git a/src/helpers/ethernet/ch390/CH390EthernetInterface.cpp b/src/helpers/ethernet/ch390/CH390EthernetInterface.cpp index acd67acbf..ba1d3ee8e 100644 --- a/src/helpers/ethernet/ch390/CH390EthernetInterface.cpp +++ b/src/helpers/ethernet/ch390/CH390EthernetInterface.cpp @@ -69,11 +69,20 @@ size_t CH390EthernetInterface::write(const uint8_t *buf, size_t size) { } bool CH390EthernetInterface::isConnected() const { - return _isConnected; + return isEnabled() && _isConnected; +} + +void CH390EthernetInterface::disconnectClient() { + _isConnected = false; + client.stop(); } void CH390EthernetInterface::loop() { - const bool wasConnected = _isConnected; + if (!isEnabled()) return; + if (_isConnected && !client.connected()) { + _isConnected = false; + onClientDisconnected(); + } if (server.hasClient()) { auto newClient = server.available(); @@ -81,16 +90,15 @@ void CH390EthernetInterface::loop() { IPAddress remoteIp = newClient.remoteIP(); uint16_t remotePort = newClient.remotePort(); ETHERNET_DEBUG_PRINTLN("New client accepted %u.%u.%u.%u:%u", remoteIp[0], remoteIp[1], remoteIp[2], remoteIp[3], remotePort); - if (client) { - ETHERNET_DEBUG_PRINTLN("Closing previous client"); - client.stop(); - } + disconnectClient(); + onClientConnected((uint32_t)remoteIp); + // The old owner's producers are cancelled before the new socket can + // become visible to the serial manager or receive any queued replies. client = newClient; - onClientConnected(); } } _isConnected = client.connected(); - if (wasConnected && !_isConnected) onClientDisconnected(); + if (!_isConnected) onClientDisconnected(); } diff --git a/src/helpers/ethernet/ch390/CH390EthernetInterface.h b/src/helpers/ethernet/ch390/CH390EthernetInterface.h index 09c3fc23d..461b94c24 100644 --- a/src/helpers/ethernet/ch390/CH390EthernetInterface.h +++ b/src/helpers/ethernet/ch390/CH390EthernetInterface.h @@ -13,6 +13,9 @@ class CH390EthernetInterface : public SerialEthernetInterface { WiFiServer server; WiFiClient client; + protected: + void disconnectClient() override; + public: CH390EthernetInterface(){ _isConnected = false; diff --git a/src/helpers/ota/OtaManager.cpp b/src/helpers/ota/OtaManager.cpp index 38d60560c..95383cbd6 100644 --- a/src/helpers/ota/OtaManager.cpp +++ b/src/helpers/ota/OtaManager.cpp @@ -1607,7 +1607,9 @@ bool OtaManager::resumeStaged(const uint8_t* want_mid) { || _fstate == WANT_LEAVES || _fstate == VERIFYING_STAGED) { return false; } - if (!_fetch->reopen()) return false; // nothing persisted in the store + const bool automatic_resume = want_mid == nullptr; + const uint32_t expected_target = automatic_resume ? _target : _fexpected_target; + if (!_fetch->reopenFor(want_mid, expected_target)) return false; uint32_t total = _fetch->staged_size(); uint8_t hdr[8]; if (total < 8u + MOTA_MFL + 5u || !_fetch->read(0, hdr, sizeof(hdr)) @@ -1617,11 +1619,9 @@ bool OtaManager::resumeStaged(const uint8_t* want_mid) { MotaManifest m; if (!_fetch->read(8, mbuf, sizeof(mbuf)) || !mota_parse_manifest(mbuf, sizeof(mbuf), m)) return false; if (want_mid && memcmp(m.merkle_root, want_mid, 4) != 0) return false; // a different fw is staged - const bool automatic_resume = want_mid == nullptr; // A boot-time resume is an automatic fetch decision and therefore belongs // only to this node. An explicit pull retains target=0 as an intentional // MID-only wildcard instead of silently narrowing it to the local target. - const uint32_t expected_target = automatic_resume ? _target : _fexpected_target; if (expected_target != 0 && m.target_id != expected_target) return false; if (automatic_resume) { if (_autofetch == AUTOFETCH_OFF) return false; @@ -1670,10 +1670,11 @@ uint32_t OtaManager::blockLen(uint32_t i) const { return (off + _fbs <= _fpsize) ? _fbs : (_fpsize - off); } -bool OtaManager::blockPresent(uint32_t i) const { +bool OtaManager::readBlockPresent(uint32_t i, bool& present) const { uint8_t leaf[4]; if (!_fetch->read(_floff + i * 4, leaf, 4)) return false; - return !(leaf[0]==0xFF && leaf[1]==0xFF && leaf[2]==0xFF && leaf[3]==0xFF); + present = !(leaf[0]==0xFF && leaf[1]==0xFF && leaf[2]==0xFF && leaf[3]==0xFF); + return true; } bool OtaManager::storedLeavesRootMatches(FetchError& error) const { @@ -1772,7 +1773,12 @@ bool OtaManager::handleData(const uint8_t* m, uint16_t n) { if (!decode_data(m, n, dm) || !_fetch) return false; if (_fstate != FETCHING || memcmp(dm.manifest_id, _fid, 4) != 0) return false; if (dm.block_idx >= _fbc) return false; - if (blockPresent(dm.block_idx)) return true; // late duplicate for this terminal + bool present; + if (!readBlockPresent(dm.block_idx, present)) { + failFetch(FETCH_ERROR_STORAGE); + return true; + } + if (present) return true; // late duplicate for this terminal int slot_index = findReassemblySlot(dm.block_idx); if (slot_index < 0) return false; // not one of this node's requested blocks ReassemblySlot& slot = _reasm[slot_index]; @@ -1890,7 +1896,14 @@ bool OtaManager::handleProof(const uint8_t* m, uint16_t n) { if (_fstate != FETCHING || memcmp(pm.manifest_id, _fid, 4) != 0) return false; if (pm.block_idx >= _fbc) return false; int slot_index = findReassemblySlot(pm.block_idx); - if (slot_index < 0 || !_reasm[slot_index].awaiting_proof) return blockPresent(pm.block_idx); + if (slot_index < 0 || !_reasm[slot_index].awaiting_proof) { + bool present; + if (!readBlockPresent(pm.block_idx, present)) { + failFetch(FETCH_ERROR_STORAGE); + return true; + } + return present; + } ReassemblySlot& slot = _reasm[slot_index]; uint32_t block = slot.block; uint32_t blen = blockLen(block); @@ -2012,6 +2025,7 @@ bool OtaManager::fillPipeline() { int slot_index = findEmptyReassemblySlot(); if (slot_index < 0) break; uint32_t block = pickMissingBlock(); + if (_fstate != FETCHING) return false; // a leaf read failed; slots were cleared if (block >= _fbc) break; ReassemblySlot& slot = _reasm[slot_index]; slot.block = block; @@ -2087,7 +2101,15 @@ void OtaManager::requestMissing() { uint32_t OtaManager::pickMissingBlock() { if (_fbc == 0) return _fbc; for (uint32_t i = 0; i < _fbc; i++) { - if (!blockInPipeline(i) && !blockPresent(i)) return i; + if (blockInPipeline(i)) continue; + bool present; + if (!readBlockPresent(i, present)) { + // Unknown is not missing: requesting a committed block would retain an + // occupied slot after its duplicate DATA is discarded on recovery. + failFetch(FETCH_ERROR_STORAGE); + return _fbc; + } + if (!present) return i; } return _fbc; } diff --git a/src/helpers/ota/OtaManager.h b/src/helpers/ota/OtaManager.h index 84be2a2a6..e2614cb11 100644 --- a/src/helpers/ota/OtaManager.h +++ b/src/helpers/ota/OtaManager.h @@ -361,6 +361,8 @@ public: // resume and must match exactly. Re-parses the stored manifest, recomputes geometry, then incrementally // rehashes every staged payload block before continuing FETCHING the holes. A fully staged image also has // to reproduce the manifest Merkle root before it can become COMPLETE. Returns true if it adopted one. + // Raw-flash stores require a unique candidate: boot resume cannot guess age + // from header address, and explicit resume selects by MID/expected target. bool resumeStaged(const uint8_t* want_mid); // Explicit operator/debug re-adoption of one known MID without starting a network fetch. This installs @@ -631,7 +633,7 @@ private: } } void setDigest(uint8_t out[4]) const; // sha2-256:4 over our served mids - bool blockPresent(uint32_t i) const; + bool readBlockPresent(uint32_t i, bool& present) const; bool storedLeavesRootMatches(FetchError& error) const; void beginStagedVerification(); void verifyStagedStep(); diff --git a/src/helpers/ota/OtaStore.h b/src/helpers/ota/OtaStore.h index 285e6c4ba..de0cfe926 100644 --- a/src/helpers/ota/OtaStore.h +++ b/src/helpers/ota/OtaStore.h @@ -61,6 +61,15 @@ public: // OtaManager then reads + parses the stored manifest to recompute geometry and resume the fetch. virtual bool reopen() { return false; } + // Resume a selected image. Single-container stores can use reopen() and let + // the manager validate MID/target. Raw stores that may retain several headers + // must select before adoption: a physical address is not a creation timestamp. + // A null MID means automatic resume; refuse ambiguous candidates instead of + // guessing which capture is newest. A zero target is an explicit wildcard. + virtual bool reopenFor(const uint8_t* want_mid, uint32_t expected_target) { + (void)want_mid; (void)expected_target; return reopen(); + } + // External host stores can disappear temporarily while retaining their file. // Preserve the selected fetch on an I/O failure until that link reconnects. virtual bool canReconnect() const { return false; } diff --git a/src/helpers/ota/OtaStoreFlashEsp32.cpp b/src/helpers/ota/OtaStoreFlashEsp32.cpp index e074203bf..5c3c07c47 100644 --- a/src/helpers/ota/OtaStoreFlashEsp32.cpp +++ b/src/helpers/ota/OtaStoreFlashEsp32.cpp @@ -70,14 +70,14 @@ bool OtaStoreFlashEsp32::begin(uint32_t total_size) { if (!_meta) { _total = 0; return false; } memset(_meta, 0xFF, _meta_flush); memset(_trailer, 0xFF, sizeof(_trailer)); - _pay_open = false; _pay_sec = 0; _pay_max_sec = 0; _flushed = false; + _pay_open = false; _pay_sec = 0; _pay_seen_end = 0; _flushed = false; _io_ok = true; return true; } void OtaStoreFlashEsp32::clear() { free(_meta); _meta = nullptr; - _total = 0; _pay_open = false; _flushed = false; // _part kept (re-acquire is fine) + _total = 0; _pay_open = false; _pay_seen_end = 0; _flushed = false; // _part kept (re-acquire is fine) } OtaStoreFlashEsp32::CandidateProbe OtaStoreFlashEsp32::probe_candidate( @@ -137,6 +137,8 @@ OtaStoreFlashEsp32::CandidateProbe OtaStoreFlashEsp32::probe_candidate( candidate.image_size = manifest.image_size; candidate.meta_bytes = (uint32_t)payload_off64; candidate.pay_size = manifest.payload_size; + memcpy(candidate.mid, manifest.merkle_root, sizeof(candidate.mid)); + candidate.target = manifest.target_id; return CandidateProbe::VALID; } @@ -237,19 +239,24 @@ void OtaStoreFlashEsp32::flush_pay() { void OtaStoreFlashEsp32::open_pay(uint32_t sec) { if (_pay_open) flush_pay(); - if (sec < _pay_max_sec) { - // revisiting an already-flushed sector (out-of-order block) -> read it back so the gaps we don't - // touch are preserved (they were programmed as 0xFF or earlier block data); we erase+reprogram on flush. + if (sec < _pay_seen_end) { + // Any sector up to and INCLUDING the highest opened sector may contain + // acknowledged blocks. Preserve it even after visiting a lower sector + // (e.g. full-image blocks 4,3,5 visit sectors 1,0,1). An exclusive bound + // also distinguishes a genuinely fresh sector 0 from a revisit to it. + // Some gaps below the bound may be unvisited, but their leaves are absent; + // conservatively keeping those bytes cannot erase a committed block. if (esp_partition_read(_part, (size_t)sec * SEC, _pay, SEC) != ESP_OK) _io_ok = false; } else { memset(_pay, 0xFF, SEC); // fresh sector - _pay_max_sec = sec; + _pay_seen_end = sec + 1u; } _pay_sec = sec; _pay_open = true; } bool OtaStoreFlashEsp32::write(uint32_t offset, const uint8_t* d, uint32_t len) { if ((uint64_t)offset + len > _total || !_io_ok) return false; + if (len != 0) _flushed = false; // a later finalize/checkpoint must persist continued writes for (uint32_t pos = offset, end = offset + len; pos < end; ) { uint32_t n = run(pos, end - pos); if (uint8_t* dst = meta_slot(pos)) { // meta / leaves / trailer -> pinned RAM @@ -327,39 +334,59 @@ void OtaStoreFlashEsp32::checkpoint() { // (MOTA_MAGIC + total) sits at a sector boundary (meta_part for full, write_start for delta - both // sector-aligned), so scan sector starts from the bottom up. A candidate is accepted only if: magic + // plausible total, the manifest parses, the container geometry is self-consistent with the total, AND the -// recomputed placement lands the meta exactly where we found the magic. Any miss -> false (fetch fresh), -// so a stray/stale match can only cost a restart, never a corrupt adopt. +// recomputed placement lands the meta exactly where we found the magic. Scan all candidates before +// adoption: a smaller, older container can have a higher address than a newer checkpoint. bool OtaStoreFlashEsp32::reopen() { + return reopenFor(nullptr, 0); +} + +bool OtaStoreFlashEsp32::reopenFor(const uint8_t* want_mid, uint32_t expected_target) { if (!acquire() || _psize < SEC) return false; + bool found = false; + StagedCandidate selected; // The meta/container is staged at the bottom of the slot. Large delta containers can begin well above // the final 512 KB, so scan the complete partition rather than silently losing resumability after reboot. for (uint32_t o = mota_esp32_align_down(_psize - SEC, SEC); ; o -= SEC) { StagedCandidate candidate; - if (probe_candidate(o, candidate) == CandidateProbe::VALID) { - _full = candidate.full; - _image_size = candidate.image_size; - _meta_bytes = candidate.meta_bytes; - _pay_size = candidate.pay_size; - set_layout(candidate.layout); - free(_meta); _meta = (uint8_t*)malloc(_meta_flush); - if (!_meta) { _total = 0; return false; } - if (esp_partition_read(_part, _meta_part, _meta, _meta_flush) != ESP_OK) { - free(_meta); _meta = nullptr; _total = 0; return false; + const CandidateProbe probe = probe_candidate(o, candidate); + if (probe == CandidateProbe::IO_ERROR) return false; + if (probe == CandidateProbe::VALID) { + // No generation marker exists. Unqualified boot resume must not infer + // recency from scan order, target, version, or apparent block progress. + const bool matches = !want_mid || + (memcmp(candidate.mid, want_mid, sizeof(candidate.mid)) == 0 && + (expected_target == 0 || candidate.target == expected_target)); + if (matches) { + if (found) return false; + selected = candidate; + found = true; } - // Raw staging defers the fixed trailer; an unvisited sector may still - // contain old firmware bytes after an early checkpoint. Restore framing - // from the validated layout, not that stale tail. The manager separately - // revalidates the selected image and every present payload block. - memcpy(_trailer, MOTA_TRAILER, sizeof(_trailer)); - if (_full) memcpy(_meta + _meta_bytes, MOTA_TRAILER, sizeof(_trailer)); - _pay_open = false; _pay_sec = 0; _flushed = false; _io_ok = true; - _pay_max_sec = (_pay_part0 + _pay_size + SEC) / SEC; // treat all payload sectors as seen -> RMW preserves committed blocks - OTA_DBG("OTA esp32: reopen %s total=%u meta_part=%u\n", _full ? "FULL" : "DELTA", (unsigned)_total, (unsigned)o); - return true; } if (o == 0) break; } - return false; + if (!found || (expected_target != 0 && selected.target != expected_target)) return false; + _full = selected.full; + _image_size = selected.image_size; + _meta_bytes = selected.meta_bytes; + _pay_size = selected.pay_size; + set_layout(selected.layout); + free(_meta); _meta = (uint8_t*)malloc(_meta_flush); + if (!_meta) { _total = 0; return false; } + if (esp_partition_read(_part, _meta_part, _meta, _meta_flush) != ESP_OK) { + free(_meta); _meta = nullptr; _total = 0; return false; + } + // Raw staging defers the fixed trailer; an unvisited sector may still + // contain old firmware bytes after an early checkpoint. Restore framing + // from the validated layout, not that stale tail. The manager separately + // revalidates the selected image and every present payload block. + memcpy(_trailer, MOTA_TRAILER, sizeof(_trailer)); + if (_full) memcpy(_meta + _meta_bytes, MOTA_TRAILER, sizeof(_trailer)); + _pay_open = false; _pay_sec = 0; _flushed = false; _io_ok = true; + // No volatile visit history survives reboot. Treat the entire partition + // as potentially written so every resumed sector preserves committed data. + _pay_seen_end = _psize / SEC; + OTA_DBG("OTA esp32: reopen %s total=%u meta_part=%u\n", _full ? "FULL" : "DELTA", (unsigned)_total, (unsigned)_meta_part); + return true; } } // namespace ota diff --git a/src/helpers/ota/OtaStoreFlashEsp32.h b/src/helpers/ota/OtaStoreFlashEsp32.h index db8cdd195..fe016e3c7 100644 --- a/src/helpers/ota/OtaStoreFlashEsp32.h +++ b/src/helpers/ota/OtaStoreFlashEsp32.h @@ -25,7 +25,7 @@ // // RX-safety: an ESP32 flash erase+write disables the XIP instruction cache and stalls code running from // flash (the dispatcher loop), exactly like the nRF52 page erase that starved the LoRa RX. So writes are -// COALESCED to the 4 KB sector and each sector is programmed once. The meta region (header+manifest+ +// COALESCED to the 4 KB sector (revisited sectors are read-modify-written). The meta region (header+manifest+ // leaves -- written all transfer long as blocks/leaves arrive, often out of order) is PINNED in RAM and // flushed at finalize() with the radio idle; the bulk payload streams through ONE sliding sector buffer, // flushing the sector it leaves behind (~1 flush per 4 KB, off the per-packet path). A brief stall still @@ -49,6 +49,8 @@ class OtaStoreFlashEsp32 : public OtaStore { uint32_t image_size = 0; uint32_t meta_bytes = 0; uint32_t pay_size = 0; + uint8_t mid[4] = {}; + uint32_t target = 0; }; const esp_partition_t* _part = nullptr; // inactive OTA slot (acquired in plan_layout/begin) @@ -73,10 +75,10 @@ class OtaStoreFlashEsp32 : public OtaStore { // RX-safe staging buffers uint8_t* _meta = nullptr; // heap, sized per fetch: header+manifest+leaves(+full trailer) alignas(4) uint8_t _pay[SEC]; // one sliding payload sector (slot-sector aligned) - uint32_t _pay_sec = 0; // slot sector index currently in _pay (0 = none open) + uint32_t _pay_sec = 0; // slot sector index currently in _pay (valid if _pay_open) uint8_t _trailer[5]; uint32_t _meta_flush = 0; // whole-sector byte count to program for the meta buffer - uint32_t _pay_max_sec = 0; // highest payload slot-sector opened (out-of-order detection) + uint32_t _pay_seen_end = 0; // exclusive highest opened sector; 0 = none seen this fetch bool _pay_open = false; // a sliding sector is currently buffered in _pay bool _flushed = false; bool _io_ok = true; // cleared if any flash erase/write/read fails @@ -113,6 +115,7 @@ public: bool finalize() override; void checkpoint() override; // persist meta(leaves) + open payload sector so a reboot can resume bool reopen() override; // re-attach to a container already staged in the slot (scan + rebuild geometry) + bool reopenFor(const uint8_t* want_mid, uint32_t expected_target) override; // No contiguous RAM/mmap view: the staged container lives in the slot (split for FULL). The ESP32 // apply path reads what it needs via read()/esp_partition_read instead of a data() pointer, so this diff --git a/src/helpers/ota/OtaStoreFlashNrf52.cpp b/src/helpers/ota/OtaStoreFlashNrf52.cpp index 641bcbf91..7d32dac5d 100644 --- a/src/helpers/ota/OtaStoreFlashNrf52.cpp +++ b/src/helpers/ota/OtaStoreFlashNrf52.cpp @@ -562,10 +562,13 @@ void OtaStoreFlashNrf52::checkpoint() { // Re-attach to a container already staged in flash (after a reboot), without erasing. The container is // bottom-aligned (begin: start = (ceiling - total) & ~(PG-1)) and flash is memory-mapped, so scan page -// starts from just below that ceiling down to the app end for MOTA_MAGIC with a self-consistent total; adopt -// the first match (highest address = most recent for the common single-container case). The manager then -// parses the loaded manifest and validates geometry/root, so a stale leftover is rejected there. +// starts from just below that ceiling down to the app end for MOTA_MAGIC with a self-consistent total. +// Address ordering does not establish age: an old small container can mask a newer larger checkpoint. bool OtaStoreFlashNrf52::reopen() { + return reopenFor(nullptr, 0); +} + +bool OtaStoreFlashNrf52::reopenFor(const uint8_t* want_mid, uint32_t expected_target) { // A retained-SRAM suffix cannot survive a power cycle or app restart as a // resumable store. Reset all in-memory split state and adopt only a complete // flash span whose advertised total fits before the selected ceiling. @@ -574,6 +577,10 @@ bool OtaStoreFlashNrf52::reopen() { const uint32_t stage_ceiling = ota_nrf52_effective_stage_ceiling(); uint32_t app_end; if (!protected_app_end(app_base, stage_ceiling, app_end)) return false; + uint32_t selected_start = 0; + uint32_t selected_total = 0; + uint32_t selected_target = 0; + bool selected_bootloader = false; for (uint32_t start = align_down(stage_ceiling - PG, PG); start >= app_end; start -= PG) { const uint8_t* p = (const uint8_t*)(uintptr_t)start; if (memcmp(p, MOTA_MAGIC, 4) != 0) continue; @@ -583,8 +590,10 @@ bool OtaStoreFlashNrf52::reopen() { continue; uint32_t want; // must be valid + placed exactly where begin() would have staged it (same bounds fn) MotaManifest manifest; - const bool manifest_ok = mota_parse_manifest(p + 8u, MOTA_MFL, manifest); - const bool bootloader = manifest_ok && manifest.is_bootloader(); + if (!mota_parse_manifest(p + 8u, MOTA_MFL, manifest)) continue; + const uint64_t payload_off = 8u + (uint64_t)MOTA_MFL + (uint64_t)manifest.block_count * 4u; + if (payload_off > PG || payload_off + manifest.payload_size + 5u != total) continue; + const bool bootloader = manifest.is_bootloader(); if (bootloader) { #if defined(OTA_INTERNAL_BOOTLOADER_UPDATE) SelfFwInfo fi; @@ -600,29 +609,39 @@ bool OtaStoreFlashNrf52::reopen() { total, app_base, app_end, stage_ceiling, want) || want != start) { continue; } - _write_start = start; - _stage_ceiling = stage_ceiling; - _flash_len = stage_ceiling - start; - _ram_len = 0; - _total = total; - _hybrid = false; - _preserve_payload_pages = true; - memcpy(_meta_page, p, PG); // load page 0 (header+manifest+leaves) into RAM to continue - // The tail is deferred until finalize and can still hold old flash bytes. - // Reconstruct only fixed framing; the manager must verify the manifest and - // rehash every present payload block before the container becomes complete. - memcpy(_trailer, MOTA_TRAILER, sizeof(_trailer)); - _pay_idx = 0; - _flushed = false; - _io_ok = true; - _planned_bootloader = bootloader; - _planned_total = bootloader ? total : 0u; - _planned_start = bootloader ? start : 0u; - OTA_DBG("OTA flash: reopen total=%u start=%08x ceiling=%08x\n", - (unsigned)total, (unsigned)start, (unsigned)stage_ceiling); - return true; + if (want_mid && (memcmp(manifest.merkle_root, want_mid, 4) != 0 || + (expected_target != 0 && manifest.target_id != expected_target))) continue; + // Without an explicit MID, every valid candidate participates in ambiguity + // detection; neither target nor firmware version proves which was newest. + if (selected_start != 0) return false; + selected_start = start; + selected_total = total; + selected_target = manifest.target_id; + selected_bootloader = bootloader; } - return false; + if (selected_start == 0 || + (expected_target != 0 && selected_target != expected_target)) return false; + _write_start = selected_start; + _stage_ceiling = stage_ceiling; + _flash_len = stage_ceiling - selected_start; + _ram_len = 0; + _total = selected_total; + _hybrid = false; + _preserve_payload_pages = true; + memcpy(_meta_page, (const uint8_t*)(uintptr_t)selected_start, PG); + // The tail is deferred until finalize and can still hold old flash bytes. + // Reconstruct only fixed framing; the manager must verify the manifest and + // rehash every present payload block before the container becomes complete. + memcpy(_trailer, MOTA_TRAILER, sizeof(_trailer)); + _pay_idx = 0; + _flushed = false; + _io_ok = true; + _planned_bootloader = selected_bootloader; + _planned_total = selected_bootloader ? selected_total : 0u; + _planned_start = selected_bootloader ? selected_start : 0u; + OTA_DBG("OTA flash: reopen total=%u start=%08x ceiling=%08x\n", + (unsigned)_total, (unsigned)_write_start, (unsigned)stage_ceiling); + return true; } } // namespace ota diff --git a/src/helpers/ota/OtaStoreFlashNrf52.h b/src/helpers/ota/OtaStoreFlashNrf52.h index 65a070398..493d103ae 100644 --- a/src/helpers/ota/OtaStoreFlashNrf52.h +++ b/src/helpers/ota/OtaStoreFlashNrf52.h @@ -87,6 +87,7 @@ public: bool finalize() override; void checkpoint() override; // persist page 0 (leaves) + the open payload page so a reboot can resume bool reopen() override; // re-attach to a container already staged in flash (scan for it) + bool reopenFor(const uint8_t* want_mid, uint32_t expected_target) override; // Mark a verified application package approved. For a hybrid package this // also prepares (but does not yet publish) the reset-retained authorization; diff --git a/src/helpers/radiolib/RadioLibWrappers.cpp b/src/helpers/radiolib/RadioLibWrappers.cpp index ce01e0ff0..30810c3d4 100644 --- a/src/helpers/radiolib/RadioLibWrappers.cpp +++ b/src/helpers/radiolib/RadioLibWrappers.cpp @@ -341,6 +341,15 @@ mesh::RadioParamApplyResult RadioLibWrapper::prepareTransmitProfile(uint8_t prof return tuneProfile(profile); } +mesh::RadioParamApplyResult RadioLibWrapper::tryRestoreCodingRate(uint8_t cr) { + const uint8_t resume_rx = beginReconfigure(); + if (resume_rx > 1) return mesh::RadioParamApplyResult::BUSY; + const bool applied = setCodingRate(cr); + if (!applied) _profile_refresh_required = true; + endReconfigure(resume_rx); + return applied ? mesh::RadioParamApplyResult::APPLIED : mesh::RadioParamApplyResult::FAILED; +} + void RadioLibWrapper::serviceProfileScan() { if (_cw_active) return; if (!_params_valid || (_profile_retry_at && (int32_t)(millis() - _profile_retry_at) < 0)) return; diff --git a/src/helpers/radiolib/RadioLibWrappers.h b/src/helpers/radiolib/RadioLibWrappers.h index 4984e8bd8..54d230d0d 100644 --- a/src/helpers/radiolib/RadioLibWrappers.h +++ b/src/helpers/radiolib/RadioLibWrappers.h @@ -211,6 +211,7 @@ public: uint32_t receiveProfileGeneration() const override { return _profile_generation; } mesh::RadioParamApplyResult prepareTransmitProfile(uint8_t profile) override; mesh::RadioParamApplyResult prepareTransmitProfile(uint8_t profile, bool reply_rx_override) override; + mesh::RadioParamApplyResult tryRestoreCodingRate(uint8_t cr) override; mesh::RadioParamApplyResult trySetPrimaryParams(const mesh::RadioProfileParams& params, bool temporary, const uint32_t* timings = nullptr) override; uint16_t profilePreamble(uint8_t profile) const override { diff --git a/test/fixtures/ota_store_flash_nrf52_hybrid/test_ota_store_flash_nrf52_hybrid.cpp b/test/fixtures/ota_store_flash_nrf52_hybrid/test_ota_store_flash_nrf52_hybrid.cpp index 69a828e04..33f8698b1 100644 --- a/test/fixtures/ota_store_flash_nrf52_hybrid/test_ota_store_flash_nrf52_hybrid.cpp +++ b/test/fixtures/ota_store_flash_nrf52_hybrid/test_ota_store_flash_nrf52_hybrid.cpp @@ -44,10 +44,6 @@ bool ota_self_firmware(SelfFwInfo& out) { return true; } -bool mota_parse_manifest(const uint8_t*, uint32_t, MotaManifest&) { - return false; -} - } // namespace ota } // namespace mesh @@ -107,6 +103,20 @@ static std::vector patterned_container(uint32_t total) { bytes[5] = static_cast(total >> 8); bytes[6] = static_cast(total >> 16); bytes[7] = static_cast(total >> 24); + // Use real, parseable metadata so raw-flash candidate selection exercises + // the production parser instead of treating arbitrary page bytes as a header. + std::memset(bytes.data() + 8, 0, MOTA_MFL); + bytes[8] = MOTA_APP_FORMAT_VER; + bytes[8 + 2] = HASH_ALGO_SHA256; + wr_u32le(bytes.data() + 8 + 3, 0x12345678u); + wr_u32le(bytes.data() + 8 + 11, 0x80000u); + const uint32_t blocks = (total - 210u + 1027u) / 1028u; + const uint32_t payload_size = total - 210u - blocks * 4u; + CHECK((payload_size + 1023u) / 1024u == blocks); + wr_u32le(bytes.data() + 8 + 15, payload_size); + bytes[8 + 19] = 10; + wr_u32le(bytes.data() + 8 + 20, total); // distinct synthetic MID for each test capture + bytes[8 + 56] = CODEC_DETOOLS_SEQUENTIAL; return bytes; } @@ -370,6 +380,59 @@ static void fresh_begin_after_reopen_does_not_retain_old_payload_bytes() { for (uint8_t byte : preceding) CHECK(byte == 0xFF); } +static void stale_header_selection_is_explicit_and_unambiguous() { + using namespace mesh::ota; + for (int selection = 0; selection < 7; ++selection) { + reset_target_memory(); + auto older = patterned_container(5547); + auto newer = patterned_container(20000); + if (selection == 3 || selection == 4) + std::memcpy(older.data() + 8 + 20, newer.data() + 8 + 20, 4); + if (selection == 3) wr_u32le(older.data() + 8 + 3, 0x12345679u); + uint32_t old_start, new_start; + { + OtaStoreFlashNrf52 store; + CHECK(store.begin(older.size())); + CHECK(store.write(0, older.data(), older.size())); + CHECK(store.finalize()); + old_start = store.write_start(); + } + { + OtaStoreFlashNrf52 store; + CHECK(store.begin(newer.size())); + CHECK(store.write(0, newer.data(), 1024)); + store.checkpoint(); + new_start = store.write_start(); + } + CHECK(new_start < old_start); + if (selection == 5) *reinterpret_cast(old_start + 8) = 0; + if (selection == 6) wr_u32le(reinterpret_cast(old_start + 8 + 15), 1); + const auto* mapped_flash = reinterpret_cast(FLASH_MAP_START); + const std::vector snapshot(mapped_flash, mapped_flash + FLASH_MAP_SIZE); + OtaStoreFlashNrf52 resumed; + uint8_t unknown_mid[4] = {0}; + const bool automatic = selection == 1 || selection == 5 || selection == 6; + const bool adopted = automatic ? resumed.reopen() : + resumed.reopenFor(selection == 2 ? unknown_mid : newer.data() + 8 + 20, + 0x12345678u); + CHECK(adopted == (selection == 0 || selection == 3 || selection >= 5)); + CHECK(std::memcmp(snapshot.data(), mapped_flash, snapshot.size()) == 0); + if (adopted) { + CHECK(resumed.write_start() == new_start); + uint8_t prefix[1024]; + CHECK(resumed.read(0, prefix, sizeof(prefix))); + CHECK(std::memcmp(prefix, newer.data(), sizeof(prefix)) == 0); + } else { + CHECK(resumed.staged_size() == 0); + } + if (selection == 3) { + OtaStoreFlashNrf52 wildcard; + CHECK(!wildcard.reopenFor(newer.data() + 8 + 20, 0)); + CHECK(!wildcard.reopenFor(nullptr, 0x12345678u)); + } + } +} + int main() { map_target_region(FLASH_MAP_START, FLASH_MAP_SIZE); map_target_region(RAM_MAP_START, RAM_MAP_SIZE); @@ -393,6 +456,8 @@ int main() { std::puts("PASS: resumed out-of-order writes survive repeated checkpoints"); fresh_begin_after_reopen_does_not_retain_old_payload_bytes(); std::puts("PASS: fresh captures after reopen discard stale payload bytes"); - std::puts("10 OtaStoreFlashNrf52 hybrid lifecycle checks passed"); + stale_header_selection_is_explicit_and_unambiguous(); + std::puts("PASS: stale headers require unique explicit MID and target selection"); + std::puts("11 OtaStoreFlashNrf52 hybrid lifecycle checks passed"); return 0; } diff --git a/test/fixtures/ota_store_resume/test.cpp b/test/fixtures/ota_store_resume/test.cpp index f4285c96a..671e8492e 100644 --- a/test/fixtures/ota_store_resume/test.cpp +++ b/test/fixtures/ota_store_resume/test.cpp @@ -8,11 +8,13 @@ using namespace mesh::ota; static std::vector flash(65536, 0xFF); +static size_t fail_read_offset = SIZE_MAX; static const esp_partition_t partition{65536}; const esp_partition_t* esp_ota_get_next_update_partition(const esp_partition_t*) { return &partition; } esp_err_t esp_partition_read(const esp_partition_t*, size_t off, void* data, size_t n) { + if (off == fail_read_offset) return -1; if (off > flash.size() || n > flash.size() - off) return -1; memcpy(data, flash.data() + off, n); return ESP_OK; @@ -168,11 +170,370 @@ static void dirty_partial_checkpoint_can_finish(bool reboot) { assert(output == bytes); } +static std::vector large_container(bool full) { + auto bytes = delta_container(20000); + if (full) { + bytes[8 + 1] = MFLAG_FULL; + bytes[8 + 56] = CODEC_FULL; + wr_u32le(bytes.data() + 8 + 11, rd_u32le(bytes.data() + 8 + 15)); + } + return bytes; +} + +static void begin_container(OtaStoreFlashEsp32& store, + const std::vector& bytes, + const MotaManifest& manifest) { + assert(store.plan_layout(manifest.is_full(), manifest.image_size, + manifest.payload - bytes.data(), manifest.payload_size, false)); + assert(store.begin(bytes.size())); + assert(store.write(0, bytes.data(), manifest.leaves - bytes.data())); + assert(store.write(bytes.size() - 5, MOTA_TRAILER, 5)); +} + +static void expect_bytes(OtaStoreFlashEsp32& store, uint32_t offset, + const uint8_t* expected, uint32_t len) { + std::vector actual(len); + assert(store.read(offset, actual.data(), actual.size())); + assert(memcmp(actual.data(), expected, len) == 0); +} + +static void esp32_revisit_preserves_acknowledged_blocks(bool full, int persistence) { + // Dirty previous firmware distinguishes fresh sectors from resumed ones. + std::fill(flash.begin(), flash.end(), 0xA5); + auto bytes = large_container(full); + MotaManifest manifest; + assert(mota_parse(bytes.data(), bytes.size(), manifest)); + const uint32_t payload_off = manifest.payload - bytes.data(); + const uint32_t leaves_off = manifest.leaves - bytes.data(); + // The full image streams at slot offset zero. A delta pins its first + // container sector, so choose bytes in its second streaming sector instead. + const uint32_t higher = full ? payload_off + 4096u : 8192u; + const uint32_t lower = higher - 4096u; + OtaStoreFlashEsp32 original, reopened; + begin_container(original, bytes, manifest); + assert(original.write(higher, bytes.data() + higher, 1024)); + assert(original.write(leaves_off, manifest.leaves, 4)); + if (persistence) original.checkpoint(); + OtaStoreFlashEsp32& active = persistence == 2 ? reopened : original; + if (persistence == 2) assert(active.reopen()); + assert(active.write(lower, bytes.data() + lower, 1024)); + assert(active.write(higher + 1024, bytes.data() + higher + 1024, 1024)); + expect_bytes(active, higher, bytes.data() + higher, 2048); + expect_bytes(active, lower, bytes.data() + lower, 1024); + expect_bytes(active, leaves_off, manifest.leaves, 4); + assert(active.finalize()); + assert(reopened.reopen()); + expect_bytes(reopened, higher, bytes.data() + higher, 2048); + expect_bytes(reopened, lower, bytes.data() + lower, 1024); +} + +static void deliver_block(OtaManager& receiver, const MotaManifest& manifest, uint32_t block) { + uint8_t wire[256]; + const uint32_t len = std::min(1024u, manifest.payload_size - block * 1024u); + for (uint32_t off = 0; off < len; off += OTA_FRAG_DATA) { + DataMsg data{}; + memcpy(data.manifest_id, manifest.merkle_root, 4); + data.block_idx = block; + data.frag_off = off; + data.data = manifest.payload + block * 1024u + off; + data.data_len = std::min((uint32_t)OTA_FRAG_DATA, len - off); + assert(receiver.on_message(wire, encode_data(wire, sizeof(wire), data))); + } + std::vector scratch(manifest.block_count * 4); + uint8_t siblings[128]; + ProofMsg proof{}; + memcpy(proof.manifest_id, manifest.merkle_root, 4); + proof.block_idx = block; + proof.n_proof = merkle_gen_proof(manifest.leaves, manifest.block_count, block, + scratch.data(), siblings); + proof.proof = siblings; + assert(receiver.on_message(wire, encode_proof(wire, sizeof(wire), proof))); +} + +static void reordered_manager_transfer_is_byte_exact(bool full) { + std::fill(flash.begin(), flash.end(), 0xA5); + auto bytes = large_container(full); + MotaManifest manifest; + assert(mota_parse(bytes.data(), bytes.size(), manifest)); + OtaStoreFlashEsp32 store; + OtaManager receiver; + receiver.begin(manifest.target_id, send, nullptr); + receiver.set_fetch_store(&store); + receiver.set_wire_v2_enabled(false); + assert(receiver.pull(manifest.merkle_root, manifest.target_id) == OtaManager::PULL_STARTED); + for (uint32_t off = 0; off < MOTA_MFL; off += OTA_MF_FRAG) { + ManifestMsg message{}; + memcpy(message.manifest_id, manifest.merkle_root, 4); + message.frag_idx = off / OTA_MF_FRAG; + message.frag_total = (MOTA_MFL + OTA_MF_FRAG - 1) / OTA_MF_FRAG; + message.bytes = manifest.manifest_start + off; + message.len = std::min((uint32_t)OTA_MF_FRAG, MOTA_MFL - off); + uint8_t wire[256]; + // MANIFEST returns false to permit forwarding even when locally accepted. + receiver.on_message(wire, encode_manifest(wire, sizeof(wire), message)); + } + assert(receiver.fetchState() == OtaManager::FETCHING); + // Legitimate requests may complete in any order as their adaptive window + // grows. At width three this includes full blocks 4,3,5; a later flight + // revisits a delta sector after completing a block across its boundary. + for (uint32_t first = 0; first < manifest.block_count; ) { + const uint32_t count = std::min((uint32_t)receiver.fetchPipelineWidth(), + manifest.block_count - first); + if (count > 1) deliver_block(receiver, manifest, first + 1); + deliver_block(receiver, manifest, first); + for (uint32_t index = 2; index < count; ++index) + deliver_block(receiver, manifest, first + index); + first += count; + } + assert(receiver.fetchState() == OtaManager::COMPLETE); + assert(receiver.blocksHave() == manifest.block_count); + std::vector output(bytes.size()); + assert(store.read(0, output.data(), output.size())); + assert(output == bytes); + MotaManifest stored; + assert(mota_parse(output.data(), output.size(), stored)); + assert(mota_check_payload(stored)); + OtaStoreFlashEsp32 resumed; + assert(resumed.reopen()); + expect_bytes(resumed, 0, bytes.data(), bytes.size()); +} + +static void fresh_sector_zero_after_reset(int reset) { + std::fill(flash.begin(), flash.end(), 0xA5); + auto bytes = large_container(true); + MotaManifest manifest; + assert(mota_parse(bytes.data(), bytes.size(), manifest)); + const uint32_t payload_off = manifest.payload - bytes.data(); + OtaStoreFlashEsp32 store; + begin_container(store, bytes, manifest); + if (reset != 0) { + assert(store.write(payload_off, manifest.payload, 1024)); + assert(store.write(payload_off + 4096, manifest.payload + 4096, 1024)); + store.checkpoint(); + if (reset == 1) store.clear(); + else { + assert(store.discard()); + OtaStoreFlashEsp32 abandoned; + assert(!abandoned.reopen()); + } + begin_container(store, bytes, manifest); + } + // A zero-length write must not make even the highest sector "seen" or + // populate the first buffer with bytes left by the previous firmware/fetch. + assert(store.write(payload_off + 8192, nullptr, 0)); + assert(store.write(payload_off, nullptr, 0)); + assert(store.write(payload_off + 32, manifest.payload + 32, 16)); + uint8_t erased[32]; memset(erased, 0xFF, sizeof(erased)); + expect_bytes(store, payload_off, erased, sizeof(erased)); + expect_bytes(store, payload_off + 32, manifest.payload + 32, 16); + assert(store.finalize()); + assert(memcmp(flash.data(), erased, sizeof(erased)) == 0); + assert(memcmp(flash.data() + 32, manifest.payload + 32, 16) == 0); +} + +static void continued_writes_after_finalize_are_persisted(bool checkpoint) { + std::fill(flash.begin(), flash.end(), 0xA5); + auto bytes = large_container(true); + MotaManifest manifest; + assert(mota_parse(bytes.data(), bytes.size(), manifest)); + const uint32_t payload_off = manifest.payload - bytes.data(); + OtaStoreFlashEsp32 store; + begin_container(store, bytes, manifest); + assert(store.write(payload_off, manifest.payload, 1024)); + assert(store.finalize()); + // Finalization closes even sector zero. Reopening that highest sector must + // preserve the old block, and a new write must make persistence dirty again. + assert(store.write(payload_off + 1024, manifest.payload + 1024, 1024)); + if (checkpoint) store.checkpoint(); + else assert(store.finalize()); + OtaStoreFlashEsp32 resumed; + assert(resumed.reopen()); + expect_bytes(resumed, payload_off, manifest.payload, 2048); +} + +struct TransientLeafReadStore : OtaStoreRam<32768> { + bool reconnectable = false; + bool fail_after_commit = false; + mutable bool fail_next_leaf = false; + uint32_t failed_leaf = 0; + bool canReconnect() const override { return reconnectable; } + bool write(uint32_t off, const uint8_t* data, uint32_t len) override { + const bool ok = OtaStoreRam<32768>::write(off, data, len); + if (ok && fail_after_commit && off == 8 + MOTA_MFL && len == 4) { + fail_after_commit = false; + fail_next_leaf = true; + } + return ok; + } + bool read(uint32_t off, uint8_t* data, uint32_t len) const override { + if (fail_next_leaf && off == 8 + MOTA_MFL + failed_leaf * 4 && len == 4) { + fail_next_leaf = false; + return false; + } + return OtaStoreRam<32768>::read(off, data, len); + } +}; + +struct RequestedFlight { + std::vector blocks; + uint32_t requests = 0; + static bool send(void* ctx, const uint8_t* bytes, uint16_t len, bool) { + auto& flight = *static_cast(ctx); + ReqWindowMsg request{}; + if (decode_req_window(bytes, len, request)) { + ++flight.requests; + flight.blocks.clear(); + for (uint8_t i = 0; i < request.n_items; ++i) + flight.blocks.push_back(request.items[i].block_idx); + } + return true; + } +}; + +static void transient_leaf_read_is_storage_failure(int phase, bool reconnectable) { + auto bytes = large_container(true); + MotaManifest manifest; + assert(mota_parse(bytes.data(), bytes.size(), manifest)); + TransientLeafReadStore store; + store.reconnectable = reconnectable; + store.fail_after_commit = phase == 0 || phase == 4; + store.failed_leaf = phase == 4 ? 2 : 0; // fail after one slot was already assigned + RequestedFlight flight; + OtaManager receiver; + receiver.begin(manifest.target_id, RequestedFlight::send, &flight); + receiver.set_fetch_store(&store); + receiver.set_wire_v2_enabled(false); + assert(receiver.pull(manifest.merkle_root, manifest.target_id) == OtaManager::PULL_STARTED); + for (uint32_t off = 0; off < MOTA_MFL; off += OTA_MF_FRAG) { + ManifestMsg message{}; + memcpy(message.manifest_id, manifest.merkle_root, 4); + message.frag_idx = off / OTA_MF_FRAG; + message.frag_total = (MOTA_MFL + OTA_MF_FRAG - 1) / OTA_MF_FRAG; + message.bytes = manifest.manifest_start + off; + message.len = std::min((uint32_t)OTA_MF_FRAG, MOTA_MFL - off); + uint8_t wire[256]; + receiver.on_message(wire, encode_manifest(wire, sizeof(wire), message)); + } + deliver_block(receiver, manifest, 0); + if (phase != 0 && phase != 4) { + store.fail_next_leaf = true; + store.failed_leaf = phase == 3 ? 1 : 0; + uint8_t wire[256]; + if (phase == 2) { + ProofMsg proof{}; + memcpy(proof.manifest_id, manifest.merkle_root, 4); + assert(receiver.on_message(wire, encode_proof(wire, sizeof(wire), proof))); + } else { + DataMsg data{}; + memcpy(data.manifest_id, manifest.merkle_root, 4); + data.block_idx = store.failed_leaf; + data.data = manifest.payload + data.block_idx * 1024; + data.data_len = OTA_FRAG_DATA; + assert(receiver.on_message(wire, encode_data(wire, sizeof(wire), data))); + } + } + assert(!store.fail_next_leaf); + assert(receiver.fetchState() == (reconnectable ? OtaManager::PAUSED : OtaManager::FAILED)); + assert(receiver.fetchError() == OtaManager::FETCH_ERROR_STORAGE); + assert(receiver.blocksHave() == 1); + if (phase == 0 || phase == 4) assert(flight.requests == 1); // never send a partially scanned flight + const auto requests = flight.requests; + receiver.set_clock(1000000); + receiver.loop(); + assert(flight.requests == requests); + uint8_t leaf[4]; + assert(store.read(8 + MOTA_MFL, leaf, sizeof(leaf))); + assert(memcmp(leaf, manifest.leaves, sizeof(leaf)) == 0); + if (reconnectable) assert(receiver.resumeFetchAfterReconnect()); + else assert(receiver.resumeStagedExplicit(manifest.merkle_root, manifest.target_id)); + while (receiver.fetchState() == OtaManager::VERIFYING_STAGED) receiver.loop(); + assert(receiver.blocksHave() == 1); + for (uint32_t guard = 0; guard < manifest.block_count && receiver.fetchState() == OtaManager::FETCHING; ++guard) { + const auto requested = flight.blocks; + assert(!requested.empty()); + for (uint32_t block : requested) { + assert(block != 0); + deliver_block(receiver, manifest, block); + } + } + assert(receiver.fetchState() == OtaManager::COMPLETE); + assert(receiver.blocksHave() == manifest.block_count); + assert(memcmp(store.data(), bytes.data(), bytes.size()) == 0); +} + +static void stale_flash_header_does_not_mask_selected_checkpoint(int selection) { + std::fill(flash.begin(), flash.end(), 0xFF); + auto older = delta_container(5547); + auto newer = delta_container(20000); + MotaManifest old_manifest, new_manifest; + assert(mota_parse(newer.data(), newer.size(), new_manifest)); + if (selection == 3 || selection == 4) + memcpy(older.data() + 8 + 20, new_manifest.merkle_root, 4); + if (selection == 3) wr_u32le(older.data() + 8 + 3, new_manifest.target_id + 1); + assert(mota_parse(older.data(), older.size(), old_manifest)); + uint32_t old_start, new_start; + { + OtaStoreFlashEsp32 store; + begin_container(store, older, old_manifest); + assert(store.write(0, older.data(), older.size())); + assert(store.finalize()); + old_start = store.write_start(); + } + { + OtaStoreFlashEsp32 store; + begin_container(store, newer, new_manifest); + assert(store.write(new_manifest.payload - newer.data(), new_manifest.payload, 1024)); + assert(store.write(new_manifest.leaves - newer.data(), new_manifest.leaves, 4)); + store.checkpoint(); + new_start = store.write_start(); + } + assert(new_start < old_start); + if (selection == 5) flash[old_start + 8] = 0; // invalid manifest cannot mask a good candidate + if (selection == 6) fail_read_offset = 0; // unknown candidate after finding a match fails closed + if (selection == 7) memset(flash.data() + old_start, 0, 4); + const auto snapshot = flash; + OtaStoreFlashEsp32 store; + OtaManager receiver; + receiver.begin(new_manifest.target_id, send, nullptr); + receiver.set_fetch_store(&store); + receiver.set_autofetch(OtaManager::AUTOFETCH_ANY); + uint8_t unknown_mid[4]; memcpy(unknown_mid, new_manifest.merkle_root, 4); unknown_mid[0] ^= 0xFF; + const bool automatic = selection == 1 || selection == 5 || selection == 7; + const bool resumed = automatic ? receiver.resumeStaged(nullptr) : + receiver.resumeStagedExplicit(selection == 2 ? unknown_mid : new_manifest.merkle_root, + new_manifest.target_id); + const bool expected = selection == 0 || selection == 3 || selection == 5 || selection == 7; + assert(resumed == expected); + assert(flash == snapshot); // selecting/rejecting never rewrites either checkpoint + fail_read_offset = SIZE_MAX; + if (resumed) { + assert(store.write_start() == new_start); + while (receiver.fetchState() == OtaManager::VERIFYING_STAGED) receiver.loop(); + assert(receiver.fetchState() == OtaManager::FETCHING); + assert(receiver.blocksHave() == 1); + } else { + assert(store.staged_size() == 0); + } + if (selection == 3) { + OtaStoreFlashEsp32 wildcard; + assert(!wildcard.reopenFor(new_manifest.merkle_root, 0)); // same MID, distinct targets is ambiguous + assert(!wildcard.reopenFor(nullptr, new_manifest.target_id)); // target alone proves no recency + } +} + int main(int argc, char** argv) { const int scenario = argc > 1 ? atoi(argv[1]) : 0; if (scenario < 5) resume_checks_file_envelope(scenario); else if (scenario < 9) esp32_resume_preserves_container_trailer((scenario & 1) == 0, scenario >= 7); else if (scenario < 13) esp32_resume_preserves_container_trailer((scenario & 1) == 0, false, scenario < 11 ? 2278 : 4098); - else dirty_partial_checkpoint_can_finish(scenario == 13); + else if (scenario < 15) dirty_partial_checkpoint_can_finish(scenario == 13); + else if (scenario < 21) esp32_revisit_preserves_acknowledged_blocks( + scenario < 18, (scenario - 15) % 3); + else if (scenario < 23) reordered_manager_transfer_is_byte_exact(scenario == 21); + else if (scenario < 26) fresh_sector_zero_after_reset(scenario - 23); + else if (scenario < 28) continued_writes_after_finalize_are_persisted(scenario == 27); + else if (scenario < 38) transient_leaf_read_is_storage_failure((scenario - 28) % 5, scenario >= 33); + else if (scenario < 46) stale_flash_header_does_not_mask_selected_checkpoint(scenario - 38); + else assert(false); } diff --git a/test/fixtures/radio_profiles/reply_mutation_test.cpp b/test/fixtures/radio_profiles/reply_mutation_test.cpp new file mode 100644 index 000000000..83277de65 --- /dev/null +++ b/test/fixtures/radio_profiles/reply_mutation_test.cpp @@ -0,0 +1,173 @@ +// Reuse the memory filesystem and radio model, not the CLI test entry point. +#define main existing_cli_main +#include "cli_test.cpp" +#undef main +#include +#include +#include + +class Millis : public mesh::MillisecondClock { + public: + unsigned long getMillis() override { return g_mock_millis; } +}; +class Inspector : public mesh::Dispatcher { + public: + Inspector(mesh::Radio& r, mesh::MillisecondClock& c, mesh::PacketManager& m) : Dispatcher(r,c,m) {} + mesh::DispatcherAction onRecvPacket(mesh::Packet*) override { return ACTION_RELEASE; } + using Dispatcher::isPacketRadioCurrent; + using Dispatcher::getNextQueueWakeDelay; + uint8_t getDefaultTxCodingRate() const override { return 5; } +}; + +static void secondaryReplies() { + unsigned checks = 0; + for (const char* command : {"set tempradio2 911.5,500,8,5,rxtx,1", + "set radio2 912.5,500,8,5,rxtx", "set radio2 off", + "set tempradio2 off", "del tempradioat2 all"}) { + for (bool success : {false, true}) { + Fixture f(true); + f.cmd("set radio2 910.5,500,8,5,rxtx"); f.advance(2000); + const auto saved = f.fs.files["/radio_profiles"]; + const uint32_t before = f.radio.p.generation[1]; + Millis ms; + StaticPoolPacketManager manager(8); + Inspector inspector(f.radio, ms, manager); + f.cli.beginReplyCommand(); f.cmd(command); f.cli.endReplyCommand(); + assert(f.cli.hasReplyMutation()); + const uint32_t mutation = f.cli.replyMutationGeneration(); + assert(strstr(f.cmd("get radio2.status"), "reply transmission")); + assert(mutation == f.cli.replyMutationGeneration()); // getters cannot steal ownership + assert(saved == f.fs.files["/radio_profiles"]); // power loss before ACK is safe + Radio reboot; mesh::RadioProfileCLI restored; + restored.begin(&f.fs, &reboot, &f.clock); + assert(reboot.p.secondary.params.freq == 910.5f); + mesh::Packet reply; + reply.radio_bound = reply.radio_reply = true; + reply.tx_radio = mesh::RADIO_TX_BOTH; + reply.radio_origin = 1; reply.radio_origin_generation = before; + reply.radio_profile = 0; reply.radio_generation = f.radio.p.generation[0]; + assert(inspector.isPacketRadioCurrent(&reply)); + f.advance(10000); + assert(before == f.radio.p.generation[1]); + assert(inspector.isPacketRadioCurrent(&reply)); // origin preserved even for primary TX + assert(f.cli.finishReplyMutation(success)); + assert(before == f.radio.p.generation[1]); // callback doesn't change radio under Dispatcher + f.advance(1); + assert(!f.cli.hasReplyMutation()); + if (!success) { + assert(before == f.radio.p.generation[1]); + assert(saved == f.fs.files["/radio_profiles"]); + } else if (strstr(command, "911.5") || strstr(command, "912.5") || !strcmp(command,"set radio2 off")) { + assert(before != f.radio.p.generation[1]); + assert(!inspector.isPacketRadioCurrent(&reply)); + } + ++checks; + } + } + for (unsigned failure = 1; failure <= 2; ++failure) { + Fixture f(true); + f.cmd("set radio2 910.5,500,8,5,rxtx"); f.advance(2000); + const auto saved = f.fs.files["/radio_profiles"]; + f.cli.beginReplyCommand(); f.cmd("set radio2 912.5,500,8,5,rxtx"); f.cli.endReplyCommand(); + f.fs.fail_rename = failure; + f.cli.finishReplyMutation(true); f.advance(1); + assert(f.cli.hasReplyMutation()); + assert(f.radio.p.secondary.params.freq == 910.5f); + assert(saved == f.fs.files["/radio_profiles"]); + assert(strstr(f.cmd("get radio2.status"), "storage commit (retrying)")); + f.advance(999); assert(f.cli.hasReplyMutation()); + f.advance(1); assert(!f.cli.hasReplyMutation()); + assert(f.radio.p.secondary.params.freq == 912.5f); + ++checks; + } + { + Fixture f(true); + f.fs.fail_write = true; + f.cli.beginReplyCommand(); f.cmd("set radio2 912.5,500,8,5,rxtx", false); f.cli.endReplyCommand(); + assert(!f.cli.hasReplyMutation()); // failure reported before a success is queued + ++checks; + } + for (bool rollover : {false, true}) { + Fixture f(true); + if (rollover) { g_mock_millis = UINT32_MAX - 1000; f.cli.loop(); } + f.cli.beginReplyCommand(); f.cmd("set tempradio2 911.5,500,8,5,rxtx,1"); f.cli.endReplyCommand(); + f.advance(65000, false); f.cli.finishReplyMutation(true); f.advance(1, false); + assert(!f.radio.p.enabled()); // no late start or lease extension + ++checks; + } + for (const char* cmd : {"set radio2 off", "set tempradio2 off", "tempradio2 911.5,500,8,5,rxtx,1", + "set radio2 911.5,500,8,5,rxtx", "del tempradioat2 all"}) { + Fixture f(true); + assert(f.cli.handle(cmd, f.reply, sizeof(f.reply), true)); + assert(strstr(f.reply, "requires local USB")); + assert(!f.cli.hasReplyMutation()); assert(!f.radio.p.enabled()); + assert(f.cli.handle("get radio2", f.reply, sizeof(f.reply), true)); + assert(!strcmp(f.reply, "> off")); + ++checks; + } + std::printf("Secondary reply transaction: %u scenarios passed\n", checks); +} + +struct FaultRadio : Radio { + uint8_t physical_cr = 5; + bool sending = false, hold_restore = false, busy_restore = false; + unsigned fail_restores = 0, restore_calls = 0, recoveries = 0; + std::vector sent_cr; + bool setCodingRate(uint8_t cr) override { physical_cr = cr; return true; } + mesh::RadioParamApplyResult tryRestoreCodingRate(uint8_t cr) override { + ++restore_calls; + if (busy_restore) return mesh::RadioParamApplyResult::BUSY; + if (hold_restore || fail_restores) { + if (fail_restores) --fail_restores; + return mesh::RadioParamApplyResult::FAILED; + } + physical_cr = cr; return mesh::RadioParamApplyResult::APPLIED; + } + bool startSendRaw(const uint8_t*, int) override { sending = true; sent_cr.push_back(physical_cr); return true; } + bool isSendComplete() override { return sending; } + void onSendFinished() override { sending = false; } + bool recoverRadio(bool) override { ++recoveries; return true; } +}; + +static void codingRateRestore() { + unsigned checks = 0; + for (unsigned mode = 0; mode < 5; ++mode) { + g_mock_millis = mode == 4 ? UINT32_MAX - 20 : 1000; + FaultRadio radio; + Millis ms; StaticPoolPacketManager manager(8); Inspector d(radio, ms, manager); + d.begin(); + auto queue = [&](uint8_t cr) { + auto* pkt = d.obtainNewPacket(); assert(pkt); + pkt->header = ROUTE_TYPE_FLOOD | (PAYLOAD_TYPE_GRP_TXT << PH_TYPE_SHIFT); + pkt->path_len = 0; pkt->payload_len = 1; pkt->payload[0] = 42; pkt->tx_cr = cr; + assert(d.sendPacket(pkt, 0)); + }; + auto tick = [&](uint32_t delta) { g_mock_millis += delta; d.loop(); }; + radio.fail_restores = 1; + radio.hold_restore = mode == 1; + radio.busy_restore = mode == 2; + queue(7); tick(1); tick(1); + assert(radio.sent_cr.size() == 1 && radio.sent_cr[0] == 7); + uint32_t wake = 0; + assert(d.getNextQueueWakeDelay(wake) && wake <= 250); // recovery survives idle/sleep + const auto calls = radio.restore_calls; + tick(100); assert(radio.restore_calls == calls); // no hot retry + queue(0); tick(1); + assert(radio.sent_cr.size() == 1); // ordinary traffic never inherits CR7 + if (mode == 1 || mode == 2) { + for (unsigned i=0; i<40; ++i) tick(250); + assert(radio.sent_cr.size() == 1 && manager.getOutboundTotal() == 1); + if (mode == 1) assert(radio.recoveries > 0); + else assert(radio.recoveries == 0); // legitimate RX/BUSY never reset + } + radio.hold_restore = radio.busy_restore = false; radio.fail_restores = 0; + if (mode == 3) radio.p.primary.cr = 6; // settings changed during recovery + tick(250); tick(1); + assert(radio.sent_cr.size() == 2 && radio.sent_cr[1] == (mode == 3 ? 6 : 5)); + assert(manager.getFreeCount() == 8); + ++checks; + } + std::printf("Coding-rate restoration: %u scenarios passed\n", checks); +} + +int main() { secondaryReplies(); codingRateRestore(); } diff --git a/test/fixtures/remote_cli_command_identity/test.cpp b/test/fixtures/remote_cli_command_identity/test.cpp new file mode 100644 index 000000000..553acbdf5 --- /dev/null +++ b/test/fixtures/remote_cli_command_identity/test.cpp @@ -0,0 +1,415 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#undef MESH_DEBUG_PRINTLN +#define MESH_DEBUG_PRINTLN(...) ((void)0) +#ifndef MESH_ENABLE_HOST_CLI +#define MESH_ENABLE_HOST_CLI 0 +#endif +#define PAYLOAD_TYPE_REQ 0 +#define PAYLOAD_TYPE_RESPONSE 1 +#define PAYLOAD_TYPE_TXT_MSG 2 +#define TXT_TYPE_PLAIN 0 +#define TXT_TYPE_CLI_DATA 1 +#define TXT_TYPE_CLI_COMMAND 2 +#define SERVER_RESPONSE_DELAY 1 +#define TXT_ACK_DELAY 1 + +namespace mesh { +struct Packet { + uint8_t path[MAX_PATH_SIZE] = {}; + uint8_t path_len = 0, radio_profile = 0; + uint32_t radio_generation = 1; + bool isRouteFlood() const { return false; } + uint8_t getPathHashSize() const { return 1; } +}; +struct Utils { + template + static void sha256(uint8_t* dest, size_t size, Args...) { memset(dest, 0, size); } + static void toHex(char* dest, const uint8_t* bytes, size_t size) { + for (size_t i = 0; i < size; ++i) snprintf(dest + i * 2, 3, "%02x", bytes[i]); + } +}; +struct Console { + std::vector records; + void printf(const char*, const char* record) { records.emplace_back(record); } +}; +static Console console; +inline Console& usbConsolePort() { return console; } +} +struct TransportKey { uint8_t key[32] = {}; }; +struct Region { bool isWildcard() const { return true; } }; +struct ClientInfo { + struct { uint8_t pub_key[PUB_KEY_SIZE] = {}; } id; + uint32_t last_timestamp = 0, last_activity = 0; + bool admin = true; + bool region_mgr = false; + bool isAdmin() const { return admin; } + bool isRegionMgr() const { return region_mgr; } + bool isFilterMgr() const { return false; } +}; +struct Clock { uint32_t getCurrentTime() const { return 1700000000; } }; +struct ReceiveProfileScope { + template ReceiveProfileScope(Mesh&, uint8_t, uint32_t) {} +}; +static uint32_t now_millis = 100; +static uint32_t millis() { return now_millis; } + +class MyMesh { +public: + struct { + ClientInfo clients[3]; + int num_clients = 3; + int getNumClients() const { return num_clients; } + ClientInfo* getClientByIdx(int i) { return &clients[i]; } + const ClientInfo* getClientByIdx(int i) const { return &clients[i]; } + void remove(int index) { + ClientInfo* c = &clients[index]; + #include "acl_delete.inc" + } + } acl; + struct { + int getTransportKeysFor(const Region&, TransportKey*, int) { return 0; } + } region_map; + Region* recv_pkt_region = nullptr; + mesh::DeferredCliCommand deferred_cli_command; + mesh::RemoteCliReplyCache remote_cli_reply_cache; + mesh::TempRadioReplyBarrier temp_radio_reply_barrier; + struct { + struct Profiles { + bool pending = false; + uint32_t generation = 0; + uint32_t replyMutationGeneration() const { return generation; } + bool hasReplyMutation() const { return pending; } + void beginReplyCommand() {} + void endReplyCommand() {} + } profiles; + Profiles& radioProfiles() { return profiles; } + } _cli; + uint32_t primary_radio_mutation_generation = 0, radio_reply_deadline = 0; + bool radio_reply_secondary = false; + bool deferred_cli_reply_scoped = false; + TransportKey deferred_cli_reply_scope; + int matching_peer_indexes[3] = {0, 1, 2}; + uint8_t reply_data[MAX_PACKET_PAYLOAD + 1] = {}; + Clock clock; + int executions = 0, acknowledgements = 0; + bool fail_command = false; + bool host_cli_waiting = false; + bool host_cli_claimed = false, host_cli_claim_emit = false; + uint32_t host_cli_deadline = 0, host_cli_claim_emit_at = 0; + uint64_t host_cli_nonce = 0, host_cli_claim_challenge = 0; + struct { + void sign(uint8_t* signature, const uint8_t*, size_t) { + memset(signature, 0x5a, SIGNATURE_SIZE); + } + } self_id; + struct RNG { + void random(uint8_t* bytes, size_t count) { memset(bytes, 0x42, count); } + } rng; + int delete_during_command = -1; + // The hardware-side parser callback is replaced by this accepted-operation + // seam. Receive normalization, fingerprinting, dispatch, cache admission and + // exact reply-barrier ownership below remain production code. + enum class Mutation { None, Primary, Secondary } accepted_mutation = Mutation::None; + bool queue_accepts = true; + int cancellations = 0; + mesh::Packet queued_ack; + std::vector replies; + std::vector> recipients; + std::vector> secrets; + + MyMesh() { now_millis = 100;mesh::console.records.clear(); + acl.clients[0].id.pub_key[0] = 0x12; acl.clients[1].id.pub_key[0] = 0x77; + acl.clients[2].id.pub_key[0] = 0x88; } + Clock* getRTCClock() { return &clock; } + RNG* getRNG() { return &rng; } + uint32_t futureMillis(uint32_t delay) const { return millis() + delay; } + bool millisHasNowPassed(uint32_t deadline) const { return int32_t(millis() - deadline) >= 0; } + int handleRequest(ClientInfo*, uint32_t, uint8_t*, size_t) { return 0; } + template mesh::Packet* createPathReturn(Args...) { return nullptr; } + template mesh::Packet* createDatagram(Args...) { return nullptr; } + template void sendFloodReply(Args...) {} + mesh::Packet* createAck(uint32_t) { return nullptr; } + template void sendClientReply(Args...) { ++acknowledgements; } + bool sendRemoteCliReply(ClientInfo* client, const uint8_t* secret, uint8_t, uint32_t, + const char* reply, const TransportKey*, mesh::Packet** queued = nullptr) { + if (!queue_accepts) return false; + if (queued) *queued = &queued_ack; + std::array key{}, shared{}; + memcpy(key.data(), client->id.pub_key, key.size()); + memcpy(shared.data(), secret, shared.size()); + recipients.push_back(key);secrets.push_back(shared); + replies.emplace_back(reply); return true; + } + void scheduleNormalRadio() {} + void finishRadioReply(bool delivered) { + if (!delivered) ++cancellations; + _cli.radioProfiles().pending = false; + temp_radio_reply_barrier.clear(); + radio_reply_deadline = 0; + radio_reply_secondary = false; + } + void clearDeferredCliCommand(); + bool completeHostCliRequest(const char*); + bool handleHostCliSerialReply(const char*, char*); + void handleCommand(uint32_t, ClientInfo*, char* command, char* reply, int, uint8_t) { + ++executions; + #include "normalization.inc" + if (delete_during_command >= 0) acl.remove(delete_during_command); + if (!fail_command) { + if (accepted_mutation == Mutation::Primary) ++primary_radio_mutation_generation; + else if (accepted_mutation == Mutation::Secondary) { + ++_cli.radioProfiles().generation; + _cli.radioProfiles().pending = true; + } + } + strcpy(reply, fail_command ? "Err - simulated failure" : "OK"); + } + void onPeerDataRecv(mesh::Packet*, uint8_t, int, const uint8_t*, uint8_t*, size_t); + void processDeferredCliCommand(); + + void receive(const char* text, uint32_t timestamp, uint32_t logical_id = 99, + int sender = 0, bool process = true) { + uint8_t wire[MAX_PACKET_PAYLOAD + 1] = {}; + uint8_t secret[PUB_KEY_SIZE] = {}; + memcpy(secret, acl.clients[sender].id.pub_key, sizeof(secret)); + memcpy(wire, ×tamp, sizeof(timestamp)); + wire[4] = TXT_TYPE_CLI_COMMAND << 2; + memcpy(wire + 5, text, strlen(text)); + const size_t size = mesh::RemoteCliRequest::append(wire, sizeof(wire) - 1, + 5, strlen(text), logical_id); + assert(size != 0); + mesh::Packet packet; + onPeerDataRecv(&packet, PAYLOAD_TYPE_TXT_MSG, sender, secret, wire, size); + if (process) processDeferredCliCommand(); + } +}; + +#include "production.inc" + +#if MESH_ENABLE_HOST_CLI +static void changeHostRequester(MyMesh& value, int change) { + if (change == 0) value.acl.remove(0); + if (change == 1) value.acl.remove(1); + if (change == 2) value.acl.clients[1].id.pub_key[31] = 1; + if (change == 3) { + value.acl.clients[1].admin = false; + value.acl.clients[1].region_mgr = true; + } +} +static void hostReply(MyMesh& value, const char* text, char* reply) { + char command[160]; + snprintf(command, sizeof(command), "host.reply %08X %016llX %s", + unsigned(value.deferred_cli_command.request_id), + static_cast(value.host_cli_nonce), text); + assert(value.handleHostCliSerialReply(command, reply)); +} +static void hostIdentityLifecycle() { + for (int change : {0, 1, 2, 3}) { + // Authorization must still be live when an admitted command reaches USB. + MyMesh value;value.receive("ab|host test", 101, 99, 1, false); + changeHostRequester(value, change);value.processDeferredCliCommand(); + assert(value.executions == 0); + assert(mesh::console.records.size() == (change == 0 ? 1U : 0U)); + assert(value.host_cli_waiting == (change == 0)); + if (change == 3) assert(value.replies[0] == "ab|Err - not permitted"); + } + for (bool change_after_claim_request : {false, true}) for (int change : {0, 1, 2, 3}) { + MyMesh value;value.receive("ab|host test", 101, 99, 1); + assert(value.executions == 0 && value.host_cli_waiting); + assert(mesh::console.records.size() == 1 && value.host_cli_nonce != 0); + if (!change_after_claim_request) changeHostRequester(value, change); + char reply[160] = {}; + hostReply(value, "@claim=0000000000000123", reply); + assert(!strcmp(reply, "OK - host claim accepted")); + if (change_after_claim_request) changeHostRequester(value, change); + now_millis += mesh::HostCliBridge::SERVICE_CLAIM_EMIT_DELAY_MILLIS; + value.processDeferredCliCommand(); + // Provisional serial acceptance is not permission to execute: no signed + // CLAIMED record may escape after removal or admin-to-manager downgrade. + assert(mesh::console.records.size() == (change == 0 ? 2U : 1U)); + assert(value.host_cli_claimed == (change == 0)); + if (change != 0) { + assert(!value.deferred_cli_command.pending && !value.host_cli_waiting); + assert(!value.host_cli_claim_emit && value.host_cli_nonce == 0); + assert(value.replies.empty()); + } else { + assert(mesh::console.records.back().find("HOSTCLI/1 CLAIMED") != std::string::npos); + hostReply(value, "OK - service reply", reply); + assert(!strcmp(reply, "OK - host reply accepted")); + assert(value.replies.size() == 1 && value.replies[0] == "ab|OK - service reply"); + assert(value.recipients[0][0] == 0x77 && value.secrets[0][0] == 0x77); + assert(!value.deferred_cli_command.pending && !value.host_cli_waiting); + value.receive("ab|host test", 102, 99, 0); + assert(mesh::console.records.size() == 2 && value.replies.size() == 2); + } + } + for (int change : {0, 1, 2, 3}) { + MyMesh value;value.receive("host test", 101, 99, 1); + char reply[160] = {}; + hostReply(value, "@claim=0000000000000123", reply); + now_millis += mesh::HostCliBridge::SERVICE_CLAIM_EMIT_DELAY_MILLIS; + value.processDeferredCliCommand();assert(value.host_cli_claimed); + changeHostRequester(value, change); + hostReply(value, "OK - service reply", reply); + assert(!strcmp(reply, change == 0 ? "OK - host reply accepted" : "Err - host requester is unavailable")); + assert(value.replies.size() == (change == 0 ? 1U : 0U)); + if (change == 0) assert(value.recipients[0][0] == 0x77 && value.secrets[0][0] == 0x77); + assert(!value.deferred_cli_command.pending && !value.host_cli_waiting); + assert(value.host_cli_nonce == 0); + } +} +#endif + +int main() { +#if MESH_ENABLE_HOST_CLI + hostIdentityLifecycle(); +#endif + // Dispatch must follow full identity across ACL movement before execution, + // and snapshot the reply destination before a handler compacts the ACL. + for (const int deleted : {0, 1}) { + MyMesh value; + value.delete_during_command = deleted; + const char* command = "setperm 12 0"; + value.receive(command, 101, 99, 1); + assert(value.executions == 1 && value.replies.size() == 1); + assert(value.recipients[0][0] == 0x77 && value.secrets[0][0] == 0x77); + const auto fp = mesh::RemoteCliReplyCache::fingerprint(command, strlen(command)); + assert(value.remote_cli_reply_cache.matches(value.recipients[0].data(), 99, fp)); + assert(!value.remote_cli_reply_cache.matches(value.acl.clients[1].id.pub_key, 99, fp)); + if (deleted == 0) { + value.receive(command, 102, 99, 0); + assert(value.executions == 1 && value.recipients.back()[0] == 0x77); + } + } + for (const bool host_completion : {false, true}) { + for (const int change : {0, 1, 2, 3}) { + MyMesh value; + value.receive("get name", 101, 99, 1, false); + if (change == 0) value.acl.remove(0); // same key, different index + if (change == 1) value.acl.remove(1); // sender deleted, slot reused + if (change == 2) value.acl.clients[1].id.pub_key[31] = 1; // prefix collision + if (change == 3) value.acl.clients[1].admin = false; // role revoked + if (host_completion) { + value.host_cli_waiting = true; + assert(value.completeHostCliRequest("OK - service reply") == (change == 0)); + assert(value.executions == 0); + } else { + value.processDeferredCliCommand(); + assert(value.executions == (change == 0 ? 1 : 0)); + } + assert(value.replies.size() == (change == 0 ? 1U : 0U)); + if (change == 0) { + assert(value.recipients[0][0] == 0x77 && value.secrets[0][0] == 0x77); + } + assert(!value.deferred_cli_command.pending); + } + } + const char* commands[] = { + "advert", "ADVERT", "advert\r\n", " ab|ADVERT \t\r\n", + "setperm 1212121212121212121212121212121212121212121212121212121212121212 3", + "Ab|Setperm 1212121212121212121212121212121212121212121212121212121212121212 3\n", + }; + for (const auto command : commands) { + for (const bool fail : {false, true}) { + MyMesh value; + value.fail_command = fail; + value.receive(command, 101); + assert(value.executions == 1 && value.replies.size() == 1); + const auto expected = value.replies.front(); + value.receive(command, 101); // exact same-timestamp retry + value.receive(command, 102); // fresh timestamp, same logical request + value.receive(command, 100); // delayed retry may replay result only + assert(value.executions == 1 && value.replies.size() == 4); + for (const auto& reply : value.replies) assert(reply == expected); + assert(value.acl.clients[0].last_timestamp == 102); + value.receive("reboot", 100); // uncached stale text remains blocked + value.receive("reboot", 102); // equal timestamp cannot run new text + assert(value.executions == 1 && value.replies.size() == 4); + value.receive(command, 103, 100); // genuinely new request + assert(value.executions == 2 && value.replies.size() == 5); + } + } + { + MyMesh value; + value.receive("ADVERT\n", 101, 99, 0, false); + value.receive("ADVERT\n", 102, 99, 0, false); + assert(value.executions == 0 && value.replies.empty()); + value.processDeferredCliCommand(); + assert(value.executions == 1 && value.replies.size() == 1); + value.receive("ADVERT\n", 103); + assert(value.executions == 1 && value.replies.size() == 2); + } + { + MyMesh value; + value.receive("ADVERT", 101); + value.receive("ADVERT", 101, 99, 1); // another authenticated identity + assert(value.executions == 2 && value.replies.size() == 2); + value.acl.clients[0].admin = false; + value.receive("ADVERT", 102); // revoked role cannot recover cached replies + assert(value.executions == 2 && value.replies.size() == 2); + } + // Every accepted primary operation owns its ACK irrespective of the user's + // spelling. These used to be detected by a literal "tempradio " prefix. + for (const char* command : {"tempradio 910.525,62.5,7,5,10", + "set tempradio 910.525,62.5,7,5,10", + "A7|set tempradio 910.525,62.5,7,5,10", + " A7|TEMPRADIO 910.525,62.5,7,5,10\r\n"}) { + for (const auto mutation : {MyMesh::Mutation::Primary, MyMesh::Mutation::Secondary}) { + MyMesh value;value.accepted_mutation = mutation; + value.receive(command, 101); + assert(value.executions == 1 && value.replies.size() == 1); + assert(value.temp_radio_reply_barrier.waiting()); + assert(value.temp_radio_reply_barrier.contains(&value.queued_ack)); + assert(value.radio_reply_deadline == millis() + 300000UL); + assert(value.radio_reply_secondary == (mutation == MyMesh::Mutation::Secondary)); + value.receive(command, 101);value.receive(command, 102); + assert(value.executions == 1 && value.replies.size() == 1); // no untracked success copy + value.receive("advert", 103, 100); + assert(value.executions == 1 && value.replies.size() == 2); + assert(value.replies.back().find("pending") != std::string::npos); + assert(value.temp_radio_reply_barrier.contains(&value.queued_ack)); + value.finishRadioReply(true); + value.receive(command, 104); + assert(value.executions == 1 && value.replies.size() == 2); // no post-completion replay + } + } + for (const auto mutation : {MyMesh::Mutation::Primary, MyMesh::Mutation::Secondary}) { + MyMesh value;value.accepted_mutation = mutation;value.queue_accepts = false; + const char* command = "A7|set tempradio 910.525,62.5,7,5,10"; + value.receive(command, 101); + assert(value.executions == 1 && value.replies.empty() && value.cancellations == 1); + assert(!value.temp_radio_reply_barrier.waiting() && !value._cli.radioProfiles().pending); + value.queue_accepts = true; + value.receive(command, 102); + assert(value.executions == 1 && value.replies.empty()); // canceled success is not replayable + } + { + MyMesh value;value._cli.radioProfiles().pending = true; + value.receive("set tempradio 910.525,62.5,7,5,10", 101); + assert(value.executions == 0 && value.replies.size() == 1); + assert(value.replies.back().find("pending") != std::string::npos); + assert(value._cli.radioProfiles().pending); // do not replace the old accepted operation + } + { + MyMesh value;value.accepted_mutation = MyMesh::Mutation::Primary;value.fail_command = true; + value.receive("set tempradio invalid", 101); + value.receive("set tempradio invalid", 102); + assert(value.executions == 1 && value.replies.size() == 2); + assert(!value.temp_radio_reply_barrier.waiting()); // validation errors remain recoverable + } + puts("CLI wire identity checks passed"); +} diff --git a/test/fixtures/replay_reset_integration/test_replay_reset_integration.cpp b/test/fixtures/replay_reset_integration/test_replay_reset_integration.cpp index 50cb3f0d0..d44903beb 100644 --- a/test/fixtures/replay_reset_integration/test_replay_reset_integration.cpp +++ b/test/fixtures/replay_reset_integration/test_replay_reset_integration.cpp @@ -93,7 +93,8 @@ struct FakeRNG { struct CountingReplyCache { bool hit; int& calls; - bool lookup(const uint8_t*, uint32_t, uint32_t, const char**) { + bool lookup(const uint8_t*, uint32_t, uint32_t, const char**, bool* authoritative) { + *authoritative = false; ++calls; return hit; } @@ -108,9 +109,16 @@ public: int clock_sync_mesh_suppressed_by = CLOCK_SYNC_MESH_SUPPRESS_NONE; int clock_sync_last_result = 0; mesh::ReplayResetNonce replay_reset_nonce; - struct { bool pending = false; int client_index = -1; } deferred_cli_command; + struct { bool pending = false; uint8_t client_pub_key[32] = {}; } deferred_cli_command; int mailbox_clears = 0; + void queueFor(int index) { + deferred_cli_command.pending = true; + std::memset(deferred_cli_command.client_pub_key, 0, 32); + if (index >= 0) std::memcpy(deferred_cli_command.client_pub_key, + acl.clients[index].id.pub_key, 32); + } + FakeClock* getRTCClock() { return &clock; } FakeRNG* getRNG() { return &rng; } void clearDeferredCliCommand() { deferred_cli_command.pending = false; ++mailbox_clears; } @@ -254,7 +262,7 @@ static void failure_is_not_live_and_token_is_consumed() { auto* admin = &value.acl.clients[0]; const auto confirm = challenge(value, admin, value.acl.clients[1]); value.acl.fail_persist = true; - value.deferred_cli_command = {true, 1}; + value.queueFor(1); CHECK(contains(run(value, admin, confirm), "no live timestamps changed")); CHECK(value.acl.calls == 1 && value.acl.clients[1].last_timestamp > NOW); CHECK(value.acl.clients[1].observed_path_pending && value.deferred_cli_command.pending); @@ -295,13 +303,13 @@ static void every_replay_family_preserves_receive_floor() { static void pending_usb_affected_and_unrelated_mailboxes() { auto value = populated(); - value.deferred_cli_command = {true, 2}; + value.queueFor(2); run(value, nullptr, request(value.acl.clients[1]), true); CHECK(value.deferred_cli_command.pending && value.mailbox_clears == 0); - value.deferred_cli_command = {true, 1}; + value.queueFor(1); run(value, nullptr, request(value.acl.clients[1]), true); CHECK(!value.deferred_cli_command.pending && value.mailbox_clears == 1); - value.deferred_cli_command = {true, -1}; + value.queueFor(-1); run(value, nullptr, "replay reset all CONFIRM", true); CHECK(!value.deferred_cli_command.pending && value.mailbox_clears == 2); } @@ -310,7 +318,7 @@ static void remote_keeps_its_executing_mailbox() { auto value = populated(); auto* admin = &value.acl.clients[0]; const auto confirm = challenge(value, admin, *admin); - value.deferred_cli_command = {true, 0}; + value.queueFor(0); CHECK(contains(run(value, admin, confirm), "OK")); CHECK(value.deferred_cli_command.pending && value.mailbox_clears == 0); } diff --git a/test/fixtures/serial_ethernet_sessions/test.cpp b/test/fixtures/serial_ethernet_sessions/test.cpp new file mode 100644 index 000000000..a600bb5b5 --- /dev/null +++ b/test/fixtures/serial_ethernet_sessions/test.cpp @@ -0,0 +1,251 @@ +#include +#include +#include +#include +#include +#include + +uint32_t mock_millis = 100; +using Bytes = std::vector; +using Socket = std::shared_ptr; +static constexpr uint32_t A = 0xc0a80114; +static constexpr uint32_t B = 0xc0a8011e; + +static Bytes framed(std::initializer_list frames) { + Bytes result; + for (const auto& frame : frames) { +#if !ETHERNET_RAW_LINE + result.insert(result.end(), {'>', uint8_t(frame.size()), uint8_t(frame.size() >> 8)}); +#endif + result.insert(result.end(), frame.begin(), frame.end()); +#if ETHERNET_RAW_LINE + result.insert(result.end(), {'\r', '\n'}); +#endif + } + return result; +} + +static Bytes command(const Bytes& payload) { +#if ETHERNET_RAW_LINE + Bytes data = payload; + data.push_back('\n'); +#else + Bytes data{'<', uint8_t(payload.size()), uint8_t(payload.size() >> 8)}; + data.insert(data.end(), payload.begin(), payload.end()); +#endif + return data; +} + +struct Transport : SerialEthernetInterface { + bool connected = false; + Socket client; + Socket next_client; + bool isConnected() const override { return isEnabled() && connected; } + void disconnectClient() override { + connected = false; + if (client) client->connected = false; + } + int available() override { return client ? int(client->received.size()) : 0; } + int read() override { + if (!client || client->received.empty()) return -1; + const int value = client->received.front(); + client->received.pop_front(); + ++client->read_count; + return value; + } + size_t write(const uint8_t* src, size_t size) override { + if (!isConnected() || !client) return 0; + size = std::min(size, client->write_limit); + client->sent.insert(client->sent.end(), src, src + size); + return size; + } + Socket connect(uint32_t ip, const Bytes& input = {}) { + next_client = std::make_shared(IPAddress(ip)); + next_client->received.insert(next_client->received.end(), input.begin(), input.end()); + disconnectClient(); + onClientConnected(ip); // Same ordering as both hardware driver loops. + client = next_client; + next_client.reset(); + connected = true; + return client; + } +}; + +struct Fixture { + Transport transport; + MultiSerialInterface routes; + uint8_t input[MAX_FRAME_SIZE] = {}; + int callbacks = 0; + bool pending_operation = false; + Fixture() { + assert(routes.addInterface(InterfaceType::Ethernet, &transport)); + routes.enable(); + transport.setSessionChangedCallback([](void* context) { + auto& f = *static_cast(context); + assert(!f.transport.isConnected()); + if (f.transport.next_client) { + assert(f.transport.next_client->sent.empty()); + assert(f.transport.next_client->read_count == 0); + } + ++f.callbacks; + f.pending_operation = false; + f.routes.forgetReplyRouteForDisconnected(&f.transport); + }, this); + } + size_t tick() { return routes.checkRecvFrame(input); } + void enqueue(const Bytes& data) { + assert(transport.writeFrame(data.data(), data.size()) == data.size()); + } + void drain(const Socket& socket) { + socket->write_limit = std::numeric_limits::max(); + for (int i = 0; i < 20; ++i) tick(); + } +}; + +int main(int argc, char** argv) { + assert(argc == 2); + const int scenario = std::atoi(argv[1]); + Fixture f; + const Bytes reply{0x01, 0x21, 0x22, 0x23}; + const Bytes other{0x02, 0x31}; + const Bytes push{0x80, 0x41, 0x42}; + auto socket = f.transport.connect(A); + if (scenario <= 2) { + f.enqueue(reply); + socket->write_limit = scenario == 0 ? 2 : scenario == 1 ? 4 : 0; + f.tick(); + assert(socket->sent.size() == socket->write_limit); + f.drain(socket); + assert(socket->sent == framed({reply})); + } else if (scenario == 3 || scenario == 4) { + f.enqueue(push); + socket->write_limit = 2; + f.tick(); + if (scenario == 4) { + f.enqueue(Bytes{0x81, 0x51}); + f.enqueue(Bytes{0x88, 0x61}); + } + f.enqueue(reply); + if (scenario == 4) f.enqueue(other); + f.drain(socket); + assert(socket->sent == (scenario == 4 ? framed({push, reply, other, {0x81, 0x51}}) + : framed({push, reply}))); + } else if (scenario == 5) { + Bytes maximum(MAX_FRAME_SIZE, 0x45); + maximum[0] = 0x80; + f.enqueue(maximum); + socket->write_limit = framed({maximum}).size() - 1; + f.tick(); + assert(socket->sent.size() == socket->write_limit); + f.enqueue(reply); + f.drain(socket); + assert(socket->sent == framed({maximum, reply})); + } else if (scenario == 6 || scenario == 7) { + f.pending_operation = true; + f.enqueue(reply); + socket->write_limit = 2; + f.tick(); + auto replacement = f.transport.connect(scenario == 6 ? A : B); + assert(!socket->connected); + assert(f.callbacks == (scenario == 6 ? 0 : 1)); + assert(f.pending_operation == (scenario == 6)); + f.drain(replacement); + assert(replacement->sent == (scenario == 6 ? framed({reply}) : Bytes{})); + if (scenario == 7) { + auto returned = f.transport.connect(A); + f.drain(returned); + assert(returned->sent.empty()); // No extra held queue on Ethernet. + } + } else if (scenario == 8) { + f.pending_operation = true; + f.enqueue(reply); + f.transport.disconnectClient(); + f.transport.onClientDisconnected(); + assert(f.callbacks == 1 && !f.pending_operation); + assert(!f.transport.hasPendingIO()); + mock_millis += 60000; + auto returned = f.transport.connect(A); + f.drain(returned); + assert(returned->sent == framed({reply}) && f.callbacks == 1); + } else if (scenario == 9) { + const Bytes request = command(Bytes{0x04}); + socket->received.insert(socket->received.end(), request.begin(), request.end()); + assert(f.tick() == 1); + assert(f.routes.captureReplyRoute() == &f.transport); + f.routes.lockReplyRoute(); + f.pending_operation = true; + auto replacement = f.transport.connect(B, command(Bytes{0x05})); + assert(!f.pending_operation && f.callbacks == 1); + assert(f.routes.captureReplyRoute() == nullptr); + assert(replacement->sent.empty() && replacement->read_count == 0); + assert(f.tick() == 1 && f.input[0] == 0x05); + assert(f.routes.captureReplyRoute() == &f.transport); + } else if (scenario == 10) { + f.enqueue(reply); + f.pending_operation = true; + f.transport.disable(); + f.transport.disable(); + assert(!socket->connected && !f.pending_operation && f.callbacks == 1); + assert(!f.transport.hasPendingIO()); + f.transport.enable(); + auto replacement = f.transport.connect(A); + f.drain(replacement); + assert(replacement->sent.empty()); + } else if (scenario == 11) { +#if ETHERNET_RAW_LINE + socket->received = std::deque(MAX_FRAME_SIZE + 1, 'a'); + socket->received.push_back('\n'); +#else + socket->received = {'<', 0xff, 0xff}; +#endif + const Bytes tail = command(Bytes{0x71}); + socket->received.insert(socket->received.end(), tail.begin(), tail.end()); + assert(f.tick() == 0); + assert(!socket->connected && f.callbacks == 1); + assert(!f.transport.isReadBusy()); + for (int i = 0; i < 5; ++i) assert(f.tick() == 0); + f.transport.connect(A, command(Bytes{0x72})); + assert(f.tick() == 1 && f.input[0] == 0x72); + } else if (scenario == 12 || scenario == 14) { + if (scenario == 14) mock_millis = UINT32_MAX - 2000; + Bytes partial = command(Bytes{'a', 'b'}); + partial.pop_back(); + socket->received.insert(socket->received.end(), partial.begin(), partial.end()); + assert(f.tick() == 0); + assert(f.transport.isReadBusy()); + mock_millis += 4999; + assert(f.tick() == 0 && socket->connected); + mock_millis += 1; +#if ETHERNET_RAW_LINE + // Human terminal input is not subject to the binary frame deadline. + assert(f.tick() == 0 && socket->connected); + mock_millis += 60000; + socket->received.push_back('\n'); + assert(f.tick() == 2 && f.input[0] == 'a' && f.input[1] == 'b'); + assert(f.callbacks == 0 && !f.transport.isReadBusy()); +#else + assert(f.tick() == 0 && !socket->connected); + assert(f.callbacks == 1 && !f.transport.isReadBusy()); +#endif + } else if (scenario == 13) { + Bytes maximum(MAX_FRAME_SIZE, 'a'); + const Bytes bytes = command(maximum); + for (size_t i = 0; i < bytes.size(); ++i) { + socket->received.push_back(bytes[i]); + assert(f.tick() == (i + 1 == bytes.size() ? MAX_FRAME_SIZE : 0)); + } + for (int i = 0; i < MAX_FRAME_SIZE; ++i) assert(f.input[i] == 'a'); + assert(!f.transport.isReadBusy() && socket->connected); + const Bytes next = command(Bytes{'b'}); + socket->received.insert(socket->received.end(), next.begin(), next.end()); + assert(f.tick() == 1 && f.input[0] == 'b'); + } else if (scenario == 15) { + const Bytes data = command(reply); + socket->received.insert(socket->received.end(), data.begin(), data.end()); + assert(f.transport.checkRecvFrame(nullptr) == 0); + assert(!socket->connected && f.callbacks == 1); + } else { + assert(false && "unknown scenario"); + } + std::printf("PASS: Ethernet session scenario %d\n", scenario); +} diff --git a/test/fixtures/serial_wifi_sessions/mocks/WiFi.h b/test/fixtures/serial_wifi_sessions/mocks/WiFi.h index 26b504798..c3eb82331 100644 --- a/test/fixtures/serial_wifi_sessions/mocks/WiFi.h +++ b/test/fixtures/serial_wifi_sessions/mocks/WiFi.h @@ -24,6 +24,7 @@ struct MockSocket { IPAddress address; bool connected = true; size_t write_limit = 0; + size_t read_limit = std::numeric_limits::max(); size_t read_count = 0; std::vector sent; std::deque received; @@ -41,6 +42,7 @@ public: IPAddress remoteIP() const { return _socket ? _socket->address : IPAddress(); } int available() const { return connected() ? int(_socket->received.size()) : 0; } size_t read(uint8_t* dest, size_t size) { + if (_socket) size = std::min(size, _socket->read_limit); size_t read = 0; while (connected() && read < size && !_socket->received.empty()) { dest[read++] = _socket->received.front(); diff --git a/test/fixtures/serial_wifi_sessions/test.cpp b/test/fixtures/serial_wifi_sessions/test.cpp index 15c83861d..f3fcfbfe8 100644 --- a/test/fixtures/serial_wifi_sessions/test.cpp +++ b/test/fixtures/serial_wifi_sessions/test.cpp @@ -42,6 +42,7 @@ struct Fixture { transport.setSessionChangedCallback([](void* context) { Fixture& fixture = *static_cast(context); ++fixture.callbacks; + assert(!fixture.transport.isConnected()); fixture.pending_operation = false; // Cancellation must happen before a new socket sees an old response or // has any of its own command bytes consumed. @@ -56,14 +57,14 @@ struct Fixture { size_t tick() { return transport.checkRecvFrame(input); } - Socket connect(IPAddress address, const Bytes& command = {}) { + Socket connect(IPAddress address, const Bytes& command = {}, bool valid = true) { Socket socket = std::make_shared(address); socket->received.insert(socket->received.end(), command.begin(), command.end()); next_client = socket; WiFiServer::incoming.push_back(WiFiClient(socket)); tick(); next_client.reset(); - assert(transport.isConnected()); + assert(transport.isConnected() == valid); return socket; } @@ -95,13 +96,15 @@ int main(int argc, char** argv) { } else if (scenario == 1) { auto old = f.connect(A); f.enqueue(reply_a); + f.pending_operation = true; old->connected = false; f.tick(); assert(!f.transport.isConnected()); + assert(f.callbacks == 1 && !f.pending_operation); mock_millis += 60000; // Same-IP retention is not the different-IP grace timer. auto replacement = f.connect(A); f.drain(replacement); - assert(replacement->sent == framed({reply_a}) && f.callbacks == 0); + assert(replacement->sent == framed({reply_a}) && f.callbacks == 1); } else if (scenario == 2) { f.connect(A); f.enqueue(reply_a); @@ -337,6 +340,72 @@ int main(int argc, char** argv) { auto other_returned = f.connect(B); f.drain(other_returned); assert(other_returned->sent == framed({other_maximum})); + } else if (scenario >= 25 && scenario <= 27) { + auto socket = f.connect(A); + f.pending_operation = true; + const Bytes invalid = scenario == 25 ? Bytes{'<', 0xff, 0xff} + : scenario == 26 ? Bytes{'?', 1, 0} : Bytes{'<', 0, 0}; + socket->received.insert(socket->received.end(), invalid.begin(), invalid.end()); + socket->received.insert(socket->received.end(), {'<', 1, 0, 0x71}); + assert(f.tick() == 0); + assert(!socket->connected && !f.transport.hasReceivedFrameHeader()); + assert(f.callbacks == 1 && !f.pending_operation); + assert(socket->read_count == 3); // Do not wait for or scan the illegal body. + for (int i = 0; i < 10; ++i) assert(f.tick() == 0); + f.connect(A, Bytes{'<', 1, 0, 0x72}); + assert(f.input[0] == 0x72); + } else if (scenario == 28 || scenario == 29 || scenario == 34) { + if (scenario == 34) mock_millis = UINT32_MAX - 2000; + auto socket = f.connect(A, scenario == 29 ? Bytes{'<', 2} : Bytes{'<', 2, 0, 0x71}); + f.pending_operation = true; + mock_millis += 4999; + assert(f.tick() == 0 && f.transport.isConnected()); + mock_millis += 1; + f.transport.loop(); + assert(!socket->connected && !f.transport.hasReceivedFrameHeader()); + assert(f.callbacks == 1 && !f.pending_operation); + assert(f.tick() == 0); + f.connect(A, Bytes{'<', 1, 0, 0x72}); + assert(f.input[0] == 0x72); + } else if (scenario == 30 || scenario == 31) { + auto socket = f.connect(A, scenario == 30 ? Bytes{'<', 2, 0} : Bytes{}); + if (scenario == 30) socket->received = {0x71, 0x72}; + else socket->received = {'<', 2, 0, 0x71, 0x72}; + socket->read_limit = 1; + assert(f.tick() == 0); + assert(!socket->connected && !f.transport.hasReceivedFrameHeader()); + assert(f.callbacks == 1); + f.connect(A, Bytes{'<', 1, 0, 0x73}); + assert(f.input[0] == 0x73); + } else if (scenario == 32) { + auto socket = f.connect(A, Bytes{'<'}); + socket->received.push_back(2); + assert(f.tick() == 0); + socket->received.insert(socket->received.end(), {0, 0x71}); + assert(f.tick() == 0); + mock_millis += 4999; + socket->received.push_back(0x72); + assert(f.tick() == 2); + assert(f.input[0] == 0x71 && f.input[1] == 0x72); + assert(f.callbacks == 0 && socket->connected); + } else if (scenario == 33) { + auto socket = f.connect(A); + socket->received = {'<', MAX_FRAME_SIZE, 0}; + for (int i = 0; i < MAX_FRAME_SIZE; ++i) socket->received.push_back(uint8_t(i)); + socket->received.insert(socket->received.end(), {'<', 1, 0, 0x72}); + assert(f.tick() == MAX_FRAME_SIZE); + for (int i = 0; i < MAX_FRAME_SIZE; ++i) assert(f.input[i] == uint8_t(i)); + assert(f.tick() == 1 && f.input[0] == 0x72); + assert(f.callbacks == 0 && socket->connected); + } else if (scenario == 35) { + auto socket = f.connect(A, Bytes{'<', 2, 0, 0x71}); + f.pending_operation = true; + f.enqueue(reply_a); + socket->connected = false; + f.transport.loop(); + assert(f.callbacks == 1 && !f.pending_operation); + f.transport.disable(); + assert(f.callbacks == 1); // No second cancellation for an ended session. } else { assert(false && "unknown scenario"); } diff --git a/test/test_build_size_detection.sh b/test/test_build_size_detection.sh index 38304a4d1..bac3bff88 100644 --- a/test/test_build_size_detection.sh +++ b/test/test_build_size_detection.sh @@ -69,6 +69,39 @@ apply_nrf52_size_profile ikoka_nano_nrf_22dbm_repeater [[ "$PLATFORMIO_BUILD_FLAGS" == *' -Os'* ]] || fail "full-sensor repeater omitted size optimization" [[ "$PLATFORMIO_BUILD_UNFLAGS" == *'-Ofast'* ]] || fail "aggressive optimization was not removed" [[ "$PLATFORMIO_BUILD_FLAGS" != *'-UENV_INCLUDE_'* ]] || fail "sensor support was reduced" + +# The legacy RAK4631 Ethernet image uses the same optimizer without inheriting +# Full Companion's transport/source-only OTA overlay or reducing its sensors. +( + PIO_ENV_PLATFORM_BY_NAME[RAK_4631_companion_radio_ethernet]=NRF52_PLATFORM + PIO_ENV_OTA_BY_NAME[RAK_4631_companion_radio_ethernet]=1 + expected_flags='-DENABLE_OTA=1 -DOTA_FLASH_STORE=1 -DETHERNET_ENABLED=1 -DENV_INCLUDE_GPS=1 -DENV_INCLUDE_BME680_BSEC=1' + for profile in auto standard; do + BUILD_PROFILE_FOR_TARGET=$profile + PLATFORMIO_BUILD_FLAGS=$expected_flags + PLATFORMIO_BUILD_UNFLAGS='-D EXTRAFS=1' + PLATFORMIO_BOARD_UPLOAD_MAXIMUM_SIZE=712704 + PLATFORMIO_BOARD_BUILD_LDSCRIPT=boards/nrf52840_s140_v6.ld + apply_nrf52_size_profile RAK_4631_companion_radio_ethernet + [ "$PLATFORMIO_BUILD_FLAGS" = "$expected_flags -Os" ] \ + || fail "Ethernet Companion changed features instead of only optimization ($profile)" + [ "$PLATFORMIO_BUILD_UNFLAGS" = $'-D EXTRAFS=1\n-Ofast' ] \ + || fail "Ethernet Companion removed flags other than aggressive optimization ($profile)" + [ "$PLATFORMIO_BOARD_UPLOAD_MAXIMUM_SIZE" = 712704 ] \ + || fail "Ethernet Companion changed its application size limit" + [ "$PLATFORMIO_BOARD_BUILD_LDSCRIPT" = boards/nrf52840_s140_v6.ld ] \ + || fail "Ethernet Companion changed its flash layout" + done + for target in RAK_4631_companion_radio_usb RAK_4631_companion_radio_ble Other_companion_radio_ethernet; do + PIO_ENV_PLATFORM_BY_NAME[$target]=NRF52_PLATFORM + PLATFORMIO_BUILD_FLAGS='' + PLATFORMIO_BUILD_UNFLAGS='' + apply_nrf52_size_profile "$target" + [ -z "$PLATFORMIO_BUILD_FLAGS" ] && [ -z "$PLATFORMIO_BUILD_UNFLAGS" ] \ + || fail "RAK Ethernet optimization expanded to unrelated target $target" + done +) + for target in Heltec_t096_repeater_lora_ota_no_external_sensors Heltec_t1_repeater_lora_ota_no_external_sensors; do PIO_ENV_PLATFORM_BY_NAME[$target]=NRF52_PLATFORM PLATFORMIO_BUILD_FLAGS='' diff --git a/test/test_companion_prefs_transactions.py b/test/test_companion_prefs_transactions.py new file mode 100644 index 000000000..299f064a2 --- /dev/null +++ b/test/test_companion_prefs_transactions.py @@ -0,0 +1,325 @@ +"""Fault-inject storage in the actual Companion frame/web/terminal setters.""" +from pathlib import Path +import os +import re +import subprocess +import tempfile +import unittest + +from test_replay_reset_integration import extract_braced + +ROOT = Path(__file__).resolve().parents[1] +SOURCE = ROOT / 'examples/companion_radio/MyMesh.cpp' + +HARNESS = r''' +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +using std::min; +using std::isfinite; +#define constrain(value,lo,hi) ((value)<(lo)?(lo):((value)>(hi)?(hi):(value))) +#define COMPANION_FEATURE_TEMP_RADIO 0 +@CODES@ +struct StrHelper { + static void strncpy(char* out,const char* in,size_t n) { + ::strncpy(out,in,n-1);out[n-1]=0; + } +}; +struct Prefs { + char node_name[32]="original"; + char default_scope_name[31]="old-scope"; + uint8_t default_scope_key[16]={9}; + float freq=915,bw=125,airtime_factor=1,rx_delay_base=2; + uint8_t sf=7,cr=5,rx_ps_level=1,rx_ps_preamble=16,rx_powersaving_enabled=0; + uint32_t rx_ps_rx_us=3000,rx_ps_sleep_us=4000,ble_pin=123456,gps_interval=60; + uint8_t manual_add_contacts=0,telemetry_mode_base=0,telemetry_mode_loc=0; + uint8_t telemetry_mode_env=0,advert_loc_policy=0,multi_acks=0; + uint8_t path_hash_mode=0,client_repeat=0,autoadd_config=0,autoadd_max_hops=2,gps_enabled=0; + uint8_t rx_boosted_gain=0,powersaving_enabled=0; + int8_t tx_power_dbm=3; +}; +struct Sensors { + double node_lat=10,node_lon=20; + std::string gps="0",interval="60",other="old"; + bool accepts=true; + bool powersaving_enabled=false; + void setPowerSavingEnabled(bool enabled) { powersaving_enabled=enabled; } + const char* getSettingByKey(const char* key) { + return !strcmp(key,"gps") ? gps.c_str() : interval.c_str(); + } + bool setSettingValue(const char* key,const char* value) { + if(!accepts)return false; + if(!strcmp(key,"gps"))gps=value; + else if(!strcmp(key,"gps_interval"))interval=value; + else other=value; + return true; + } +} sensors; +struct Driver { + bool supported=true,rxps=false; + uint32_t rx=3000,sleep=4000; + unsigned calls=0; + std::deque results; + bool supportsRxBoostedGainMode() const { return true; } + bool supportsRxPowerSaving() const { return supported; } + bool setRxPowerSaving(bool enable,uint32_t r,uint32_t s) { + ++calls;bool ok=true; + if(!results.empty()){ok=results.front();results.pop_front();} + if(ok){rxps=enable;rx=r;sleep=s;} + return ok; + } + void setRxBoostedGainMode(uint8_t) {} + bool setTxPower(int8_t) { return true; } +} radio_driver; +@PARSERS@ +struct Clock { uint32_t getMillis() const { return 100; } } clock_source; +struct MyMesh { + Prefs _prefs,durable; + double durable_lat=10,durable_lon=20; + bool storage_accepts=true,_radio_available=true; + unsigned saves=0,gps_policy_updates=0; + bool saved_radio_apply_pending=false,command_radio_apply_pending=false; + uint32_t radio_apply_retry_at=0; + uint8_t radio_apply_failures=0; + Clock* _ms=&clock_source; + uint8_t cmd_frame[180]={}; + std::vector replies; + std::string output; + MyMesh() { memcpy(&durable,&_prefs,sizeof(Prefs));sensors=Sensors();radio_driver=Driver(); } + @SAVE_PREFERENCE@ + bool saveAdvertName(const char*); + bool saveAdvertLocation(double,double); + bool applyAndSaveRxPowerSaving(const char*,char*); + bool applyAndSavePowerSaving(const char*,char*); + bool applyAndSaveRxBoostedGain(bool) { return true; } + bool savePrefs() { + ++saves;if(!storage_accepts)return false; + memcpy(&durable,&_prefs,sizeof(Prefs));durable_lat=sensors.node_lat;durable_lon=sensors.node_lon;return true; + } + void writeOKFrame() { replies.push_back(0); } + void writeErrFrame(uint8_t error) { replies.push_back(error); } + void updateGpsTelemetryPolicy() { ++gps_policy_updates; } + void applyGpsPrefs() { + sensors.setSettingValue("gps",_prefs.gps_enabled?"1":"0"); + char s[16];snprintf(s,sizeof(s),"%lu",(unsigned long)_prefs.gps_interval); + sensors.setSettingValue("gps_interval",s); + } + MyMesh& terminalOutput() { return *this; } + void print(const char* value) { output+=value; } + void frame(size_t len) { @FRAMES@ } + void web(const char* key,const char* value,char* reply) { @WEB@ } + void terminal(const char* config) { @TERMINAL@ } + bool gps(const char* gps_value,char* reply) { const size_t reply_size=160;@GPS@;return false; } + bool hasOutbound() const { return false; } + bool millisHasNowPassed(uint32_t) const { return true; } + uint32_t futureMillis(uint32_t delay) const { return delay+100; } + bool applySavedRadioParams() { return true; } + void recover() { @RECOVERY@ } + void expectOriginal() const { + assert(!memcmp(&_prefs,&durable,sizeof(Prefs))); + assert(!strcmp(_prefs.node_name,"original")); + assert(_prefs.freq==915 && _prefs.bw==125 && _prefs.sf==7 && _prefs.cr==5); + assert(_prefs.airtime_factor==1 && _prefs.rx_delay_base==2 && _prefs.ble_pin==123456); + assert(_prefs.manual_add_contacts==0 && _prefs.telemetry_mode_base==0); + assert(_prefs.telemetry_mode_loc==0 && _prefs.telemetry_mode_env==0); + assert(_prefs.advert_loc_policy==0 && _prefs.multi_acks==0 && _prefs.client_repeat==0); + assert(_prefs.path_hash_mode==0 && _prefs.autoadd_config==0 && _prefs.autoadd_max_hops==2); + assert(!strcmp(_prefs.default_scope_name,"old-scope") && _prefs.default_scope_key[0]==9); + assert(_prefs.gps_enabled==0 && _prefs.gps_interval==60); + assert(sensors.node_lat==10 && sensors.node_lon==20 && durable_lat==10 && durable_lon==20); + } + void laterSaveCannotResurrectFailure() { + expectOriginal();storage_accepts=true; + assert(saveAdvertName("later")); + assert(durable.ble_pin==123456 && durable.airtime_factor==1 && durable.rx_delay_base==2); + assert(durable.path_hash_mode==0 && durable.gps_enabled==0 && durable.gps_interval==60); + assert(durable.autoadd_config==0 && durable.autoadd_max_hops==2); + assert(durable.freq==915 && durable.bw==125 && durable.sf==7 && durable.cr==5); + assert(durable.rx_ps_rx_us==3000 && durable.rx_ps_sleep_us==4000 && !durable.rx_powersaving_enabled); + assert(durable_lat==10 && durable_lon==20); + } +}; +@METHODS@ +static void u32(std::vector& bytes,uint32_t value) { + for(unsigned i=0;i<4;++i) bytes.push_back(uint8_t(value>>(8*i))); +} +static void send(MyMesh& m,const std::vector& bytes) { + memcpy(m.cmd_frame,bytes.data(),bytes.size());m.frame(bytes.size()); +} +static bool error(const char* reply) { return strstr(reply,"Error") || strstr(reply,"ERROR"); } +int main() { + std::vector> frames={{CMD_SET_ADVERT_NAME,'n','e','w'}, + {CMD_SET_OTHER_PARAMS,1,0x2a,1,2}, {CMD_SET_OTHER_PARAMS,1}, + {CMD_SET_PATH_HASH_MODE,0,2},{CMD_SET_AUTOADD_CONFIG,31,100}, + {CMD_SET_AUTOADD_CONFIG,31},{CMD_SET_DEFAULT_FLOOD_SCOPE}}; + std::vector location={CMD_SET_ADVERT_LATLON};u32(location,30000000);u32(location,40000000); + frames.push_back(location); + std::vector tuning={CMD_SET_TUNING_PARAMS};u32(tuning,9000);u32(tuning,5000);frames.push_back(tuning); + std::vector pin={CMD_SET_DEVICE_PIN};u32(pin,654321);frames.push_back(pin); + std::vector scope(48,0);scope[0]=CMD_SET_DEFAULT_FLOOD_SCOPE; + memcpy(scope.data()+1,"new-scope",9);scope[32]=42;frames.push_back(scope); +#if ENV_INCLUDE_GPS == 1 + for(const char* value:{"gps:1","gps_interval:120"}) { + std::vector custom={CMD_SET_CUSTOM_VAR}; + custom.insert(custom.end(),value,value+strlen(value));frames.push_back(custom); + } +#endif + unsigned checks=0; + for(const auto& bytes:frames) { + for(bool save_ok:{false,true}) { + MyMesh m;m.storage_accepts=save_ok;send(m,bytes);++checks; + assert(m.replies.size()==1 && m.replies[0]==(save_ok?0:ERR_CODE_FILE_IO_ERROR)); + assert(m.saves==1); + if(!save_ok) { + assert(m.gps_policy_updates==0 && sensors.gps=="0" && sensors.interval=="60"); + m.laterSaveCannotResurrectFailure(); + } else { + assert(!memcmp(&m._prefs,&m.durable,sizeof(Prefs))); + assert(m.durable_lat==sensors.node_lat && m.durable_lon==sensors.node_lon); + if(bytes[0]==CMD_SET_OTHER_PARAMS)assert(m.gps_policy_updates==1); + if(bytes[0]==CMD_SET_AUTOADD_CONFIG && bytes.size()==3)assert(m.durable.autoadd_max_hops==64); + } + } + } + const char* web[][2]={{"name","new"},{"lat","30"},{"lon","40"},{"radio","920,250,9,6"}, + {"af","3"},{"rxdelay","4"},{"repeat","on"}}; + for(const auto& command:web)for(bool save_ok:{false,true}) { + MyMesh m;char reply[160]={};m.storage_accepts=save_ok; + m.web(command[0],command[1],reply);++checks; + assert(error(reply)!=save_ok && m.saves==1); + if(!save_ok)m.laterSaveCannotResurrectFailure(); + else assert(!memcmp(&m._prefs,&m.durable,sizeof(Prefs))); + } + for(const char* command:{"name new","lat 30","lon 40","af 3"})for(bool save_ok:{false,true}) { + MyMesh m;m.storage_accepts=save_ok;m.terminal(command);++checks; + assert(error(m.output.c_str())!=save_ok && m.saves==1); + if(!save_ok)m.laterSaveCannotResurrectFailure(); + } + for(const auto& bytes:std::vector>{ + {CMD_SET_PATH_HASH_MODE,0,3},{CMD_SET_TUNING_PARAMS},{CMD_SET_OTHER_PARAMS}, + {CMD_SET_AUTOADD_CONFIG}}) { + MyMesh m;send(m,bytes);++checks; + assert(m.replies.size()==1 && m.replies[0]==ERR_CODE_ILLEGAL_ARG && m.saves==0); + m.expectOriginal(); + } +#if ENV_INCLUDE_GPS == 1 + for(bool save_ok:{false,true}) { + MyMesh m;char reply[160]={};m.storage_accepts=save_ok; + assert(m.gps("on",reply));++checks; + assert(error(reply)!=save_ok && m.saves==1); + assert(sensors.gps==(save_ok?"1":"0")); + if(!save_ok)m.laterSaveCannotResurrectFailure(); + } +#endif + // Checked RXPS applies roll back hardware too, and keep retrying a busy + // rollback until the saved (including disabled) duty-cycle mode is restored. + for(const char* value:{"10000 20000","level 2","off"})for(bool save_ok:{false,true}) { + MyMesh m;char reply[160]={};m.storage_accepts=save_ok; + assert(m.applyAndSaveRxPowerSaving(value,reply)==save_ok);++checks; + if(error(reply)==save_ok || m.saves!=1) fprintf(stderr,"RXPS %s: %s, saves=%u\n",value,reply,m.saves); + assert(error(reply)!=save_ok && m.saves==1); + if(!save_ok) { + assert(!radio_driver.rxps && radio_driver.rx==3000 && radio_driver.sleep==4000); + m.laterSaveCannotResurrectFailure(); + } else assert(radio_driver.rxps==bool(m.durable.rx_powersaving_enabled)); + } + { + MyMesh m;char reply[160]={};m.storage_accepts=false; + radio_driver.results={true,false,false,true}; + assert(!m.applyAndSaveRxPowerSaving("10000 20000",reply));++checks; + m.expectOriginal();assert(m.saved_radio_apply_pending && radio_driver.rxps); + m.recover();assert(m.saved_radio_apply_pending && radio_driver.rxps); + m.recover();assert(!m.saved_radio_apply_pending && !radio_driver.rxps); + assert(radio_driver.rx==3000 && radio_driver.sleep==4000); + } + { + MyMesh m;char reply[160]={};radio_driver.results={false}; + assert(!m.applyAndSaveRxPowerSaving("10000 20000",reply));++checks; + assert(m.saves==0);m.expectOriginal(); + } + for(bool initial:{false,true})for(bool save_ok:{false,true}) { + MyMesh m;char reply[160]={};m.storage_accepts=save_ok; + m._prefs.powersaving_enabled=m.durable.powersaving_enabled=initial; + sensors.powersaving_enabled=initial; + assert(m.applyAndSavePowerSaving(initial?"off":"on",reply)==save_ok);++checks; + assert(error(reply)!=save_ok && m.saves==1); + const bool expected=save_ok?!initial:initial; + assert(m._prefs.powersaving_enabled==expected && m.durable.powersaving_enabled==expected); + assert(sensors.powersaving_enabled==expected); + m.storage_accepts=true;assert(m.saveAdvertName("later")); + assert(m.durable.powersaving_enabled==expected); + } + printf("%u Companion persistence scenarios passed (GPS=%d)\n",checks,ENV_INCLUDE_GPS); +} +''' + + +class CompanionPrefsTransactionTests(unittest.TestCase): + def test_production_setters_roll_back_failed_saves(self): + text = SOURCE.read_text(encoding='utf-8') + header = (SOURCE.parent / 'MyMesh.h').read_text(encoding='utf-8') + frames = ( + 'CMD_SET_ADVERT_NAME && len >= 2', 'CMD_SET_ADVERT_LATLON && len >= 9', + 'CMD_SET_TUNING_PARAMS', 'CMD_SET_OTHER_PARAMS', + 'CMD_SET_PATH_HASH_MODE && len >= 3 && cmd_frame[1] == 0', + 'CMD_SET_DEVICE_PIN && len >= 5', 'CMD_SET_CUSTOM_VAR && len >= 4', + 'CMD_SET_DEFAULT_FLOOD_SCOPE && len >= 1', 'CMD_SET_AUTOADD_CONFIG', + ) + web = ( + 'strcmp(key, "name") == 0', + 'strcmp(key, "lat") == 0 || strcmp(key, "lon") == 0', + 'strcmp(key, "radio") == 0', + 'strcmp(key, "af") == 0 || strcmp(key, "rxdelay") == 0', + 'strcmp(key, "radio.rxgain") == 0 || strcmp(key, "repeat") == 0', + ) + terminal = ( + 'strncmp(config, "af ", 3) == 0', + 'strncmp(config, "name ", 5) == 0 && config[5] != 0', + 'strncmp(config, "lat ", 4) == 0', 'strncmp(config, "lon ", 4) == 0', + ) + replacements = { + '@CODES@': '\n'.join(re.findall(r'^#define (?:CMD_SET_\w+|ERR_CODE_\w+)\s+\d+',text,re.M)), + '@PARSERS@': '\n'.join(extract_braced(text, sig) for sig in ( + 'static bool wcParseBool(', 'static bool wcParseDouble(', 'static bool wcCopyValue(', + 'static uint32_t nextRadioApplyRetryDelay(', + )), + '@SAVE_PREFERENCE@': extract_braced(header, 'template bool savePreference('), + '@METHODS@': '\n'.join(extract_braced(text,sig) for sig in ( + 'bool MyMesh::saveAdvertName(', 'bool MyMesh::saveAdvertLocation(', + 'bool MyMesh::applyAndSaveRxPowerSaving(', + 'bool MyMesh::applyAndSavePowerSaving(', + )), + '@FRAMES@': ' else '.join(extract_braced(text, f'if (cmd_frame[0] == {cmd})') for cmd in frames), + '@WEB@': ' else '.join(extract_braced(text,f'if ({expr})') for expr in web), + '@TERMINAL@': ' else '.join(extract_braced(text,f'if ({expr})') for expr in terminal), + '@GPS@': extract_braced(text,'if (gps_value)'), + '@RECOVERY@': extract_braced(text,'if (!command_radio_apply_pending && saved_radio_apply_pending && !hasOutbound()'), + } + harness = HARNESS + for marker,value in replacements.items(): + harness = harness.replace(marker,value) + with tempfile.TemporaryDirectory(prefix='companion-prefs-') as tmp: + root = Path(tmp) + (root/'Arduino.h').write_text('#pragma once\n#include \n#include \n') + cpp = root/'test.cpp' + cpp.write_text(harness,encoding='utf-8') + binary = root/('test.exe' if os.name=='nt' else 'test') + for gps in (0,1): + with self.subTest(gps=gps): + subprocess.run([os.environ.get('CXX','g++'),'-std=c++17','-Wall','-Wextra','-Werror', + '-Wno-sign-compare','-Wno-type-limits',f'-DENV_INCLUDE_GPS={gps}', + '-I',str(root),'-I',str(ROOT/'src'),str(cpp),'-o',str(binary)],check=True) + subprocess.run([str(binary)],check=True) + + +if __name__=='__main__': + unittest.main() diff --git a/test/test_companion_primary_radio_persistence.py b/test/test_companion_primary_radio_persistence.py new file mode 100644 index 000000000..715f58150 --- /dev/null +++ b/test/test_companion_primary_radio_persistence.py @@ -0,0 +1,247 @@ +"""Execute primary CLI transactions through real parsers and preference images.""" +from pathlib import Path +import os +import re +import shutil +import subprocess +import tempfile +import unittest + +from test_companion_preferences_transaction import HARNESS as PREFERENCES_HARNESS +from test_replay_reset_integration import extract_braced + +ROOT = Path(__file__).resolve().parents[1] + +HARNESS = r''' +#include +#include +#include +#define constrain(value,low,high) ((value)<(low)?(low):((value)>(high)?(high):(value))) +struct Clock : mesh::RTCClock { + uint32_t getCurrentTime() override { return 1700000000; } + void setCurrentTime(uint32_t) override {} +}; +struct Radio : mesh::Radio { + mesh::RadioProfiles p; + Radio() { p.primary.freq=909.5f; p.primary.bw=62.5f; p.primary.sf=7; p.primary.cr=5; } + mesh::RadioProfiles* profiles() override { return &p; } + const mesh::RadioProfiles* profiles() const override { return &p; } + bool validateProfile(const mesh::RadioProfileParams& v) const override { + return mesh::RadioProfiles::valid(v) && v.bw<=500; + } + uint16_t profilePreamble(uint8_t profile) const override { return p.preamble(profile,32); } + int recvRaw(uint8_t*,int) override { return 0; } + uint32_t getEstAirtimeFor(int) override { return 10; } + float packetScore(float,int) override { return 0; } + bool startSendRaw(const uint8_t*,int) override { return true; } + bool isSendComplete() override { return true; } + void onSendFinished() override {} + bool isInRecvMode() const override { return true; } +}; +struct Sensors { double node_lat=0,node_lon=0; } sensors; +@COMMAND_FILTER@ +struct MyMesh { + CompanionNodePrefs _prefs; + DataStore store; + DataStore* _store=&store; + MemoryFS profile_fs; + Radio radio; + Clock clock; + mesh::RadioProfileCLI _radio_profiles; + char reply[160]={}; + MyMesh() { + _prefs.freq=909.5f; _prefs.bw=62.5f; _prefs.sf=7; _prefs.cr=5; + _prefs.airtime_factor=1; _prefs.rx_ps_level=1; _prefs.rx_ps_preamble=16; + _prefs.rx_ps_rx_us=3000; _prefs.rx_ps_sleep_us=4000; + _radio_profiles.begin(&profile_fs,&radio,&clock); + assert(_radio_profiles.savePrimaryPreamble(48)); + assert(savePrefs()); + } + @SAVE_PREFS@ + void bootSanitize() { @COMPANION_SANITIZE@ } + bool handleCommand(const char*,uint32_t,char*); + const char* command(const char* value,uint32_t timestamp=0) { + memset(reply,0,sizeof(reply)); + assert(handleCommand(value,timestamp,reply)); + assert(memchr(reply,0,sizeof(reply))); + return reply; + } + void loadSaved(CompanionNodePrefs& loaded) { + double lat=0,lon=0; + assert(store.loadPrefs(loaded,lat,lon)); + } + uint16_t rebootPreamble() { + Radio reboot_radio;mesh::RadioProfileCLI reboot_cli; + reboot_cli.begin(&profile_fs,&reboot_radio,&clock); + return reboot_cli.primaryPreamble(); + } + void expectOriginal() { + assert(_prefs.freq==909.5f && _prefs.bw==62.5f && _prefs.sf==7 && _prefs.cr==5); + assert(_prefs.rx_ps_rx_us==3000 && _prefs.rx_ps_sleep_us==4000); + assert(_radio_profiles.primaryPreamble()==48 && rebootPreamble()==48); + CompanionNodePrefs loaded;loadSaved(loaded); + assert(loaded.freq==909.5f && loaded.bw==62.5f && loaded.sf==7 && loaded.cr==5); + } +}; +@DISPATCH@ +// CommonCLI's infrastructure roles share the same accepted duty-cycle range. +// Extract their actual setter and boot sanitization, with only storage mocked. +struct Infrastructure { + struct Prefs { float airtime_factor=1; } prefs; + Prefs* _prefs=&prefs; + uint8_t image[sizeof(float)]={}; + void savePrefs() { memcpy(image,&prefs.airtime_factor,sizeof(float)); } + void command(const char* config,char* reply) { @INFRA_SETTER@ } + void reboot() { memcpy(&prefs.airtime_factor,image,sizeof(float));@INFRA_SANITIZE@ } +}; +static bool error(const char* reply) { return strstr(reply,"Error") || strstr(reply,"ERROR"); } +int main() { + unsigned checks=0; + // The outer profile validator accepts 125.005, but CommonRadioPrefs rejects + // it. Rejection must preserve both on-disk images, even with a dirty adapter. + for(uint32_t timestamp:{0U,1700000000U})for(bool dirty:{false,true}) { + for(const char* command:{"set radio 915,125.005,7,5,32", + "set radio 915,125,13,5,32", "set radio 149,125,7,5,32", + "set radio 915,125,7,5,7", "set radio 915,125,7,5,32,extra", + "set radio 915,nan,7,5,32"}) { + MyMesh m;const auto profile=m.profile_fs.files["/radio_profiles"]; + const auto prefs=m.store.fs.files["/new_prefs"]; + if(dirty)m._prefs.getRadioPrefs()->markDirty(); + assert(error(m.command(command,timestamp))); + assert(m.profile_fs.files["/radio_profiles"]==profile); + assert(m.store.fs.files["/new_prefs"]==prefs);m.expectOriginal();++checks; + } + } + // Profile-store write failure must roll back the accepted in-memory tuple; + // preference-store failure must also restore the previously saved preamble. + for(bool profile_failure:{false,true}) { + MyMesh m;const auto profile=m.profile_fs.files["/radio_profiles"]; + const auto prefs=m.store.fs.files["/new_prefs"]; + m.profile_fs.fail_write=profile_failure;m.store.fs.fail_write=!profile_failure; + assert(error(m.command("set radio 915,125,8,6,32"))); + assert(m.profile_fs.files["/radio_profiles"]==profile); + assert(m.store.fs.files["/new_prefs"]==prefs); + m.profile_fs.fail_write=m.store.fs.fail_write=false; + m.expectOriginal();assert(!m._prefs.isDirty()); + assert(saveUnrelated(m));m.expectOriginal();++checks; + } + for(const char* command:{"set radio 915,125,8,6,32", "zz|set radio 915,125.0005,8,6,32"}) { + MyMesh m;assert(!error(m.command(command))); + assert(m.rebootPreamble()==32 && !m._prefs.isDirty()); + CompanionNodePrefs loaded;m.loadSaved(loaded); + assert(loaded.freq==915 && loaded.sf==8 && loaded.cr==6); + assert(fabsf(loaded.bw-125)<0.001f);++checks; + } + { // Omitted suffix intentionally restores the automatic preamble. + MyMesh m;assert(!error(m.command("set radio 915,125,8,6"))); + assert(m.rebootPreamble()==0);++checks; + } + for(float duty:{1.0f,2.5f,9.9f,10.0f,100.0f}) { + MyMesh m;char command[64];snprintf(command,sizeof(command),"set dutycycle %.1f",double(duty)); + assert(!error(m.command(command)));const float factor=m._prefs.airtime_factor; + assert(fabsf(factor-(100/duty-1))<0.0001f); + CompanionNodePrefs loaded;m.loadSaved(loaded);assert(loaded.airtime_factor==factor); + MyMesh reboot;reboot.store.fs.files=m.store.fs.files; + reboot.loadSaved(reboot._prefs);reboot.bootSanitize(); + assert(reboot._prefs.airtime_factor==factor); + char expected[32];snprintf(expected,sizeof(expected),"> %.1f%%",double(duty)); + assert(!strcmp(reboot.command("get dutycycle"),expected)); + Infrastructure infra;char reply[160]={};infra.command(command+4,reply); + assert(!error(reply));infra.prefs.airtime_factor=1;infra.reboot(); + assert(infra.prefs.airtime_factor==factor);++checks; + } + for(float invalid:{-1.0f,1000.0f,std::numeric_limits::infinity(), + std::numeric_limits::quiet_NaN()}) { + MyMesh m;m._prefs.airtime_factor=invalid;m.bootSanitize(); + const float expected=!isfinite(invalid)?1:(invalid<0?0:99); + assert(m._prefs.airtime_factor==expected); + Infrastructure infra;infra.prefs.airtime_factor=invalid;infra.savePrefs();infra.reboot(); + assert(infra.prefs.airtime_factor==expected);++checks; + } + for(const char* command:{"set dutycycle 0.9","set dutycycle 100.1","set dutycycle nan","set af 99"}) { + MyMesh m;const auto prefs=m.store.fs.files["/new_prefs"]; + assert(error(m.command(command)));assert(m.store.fs.files["/new_prefs"]==prefs); + assert(m._prefs.airtime_factor==1);++checks; + } + printf("%u production primary-radio persistence scenarios passed\n",checks); +} +''' + + +class CompanionPrimaryRadioPersistenceTests(unittest.TestCase): + def test_production_validation_rollback_and_dutycycle_boot(self): + companion = (ROOT/'examples/companion_radio/MyMesh.cpp').read_text(encoding='utf-8') + header = (ROOT/'examples/companion_radio/MyMesh.h').read_text(encoding='utf-8') + infra = (ROOT/'src/helpers/CommonCLI.cpp').read_text(encoding='utf-8') + datastore = (ROOT/'examples/companion_radio/DataStore.cpp').read_text(encoding='utf-8') + handler = extract_braced(companion, 'bool MyMesh::handleCommand(') + # Keep the complete preprocessing sequence through the shared primary + # tuple validation. Only unrelated dispatch routes are omitted. + dispatch = handler[:handler.index(' if (sender_timestamp == 0 && handleDirectCommand')] + dispatch += extract_braced(handler, 'if (isCompanionRadioPrefsCommand(command))') + dispatch += '\nreturn false;\n}' + + def boot_checks(source, member): + nonfinite = re.search(r'if \(!isfinite\(' + re.escape(member) + r'\)\)[^;]+;', source) + clamp = re.search(re.escape(member) + r' = constrain\([^;]+;', source) + self.assertIsNotNone(nonfinite) + self.assertIsNotNone(clamp) + return nonfinite.group() + '\n' + clamp.group() + + support = PREFERENCES_HARNESS[:PREFERENCES_HARNESS.index('int main(')] + support = support.replace('@METHODS@', '\n'.join(extract_braced(datastore, sig) for sig in ( + 'bool DataStore::loadPrefs(', 'bool DataStore::loadPrefsInt(', 'bool DataStore::savePrefs('))) + replacements = { + '@SAVE_PREFS@': extract_braced(header, 'bool savePrefs()'), + '@COMMAND_FILTER@': extract_braced(companion, 'static bool isCompanionRadioPrefsCommand('), + '@DISPATCH@': dispatch, + '@COMPANION_SANITIZE@': boot_checks(companion, '_prefs.airtime_factor'), + '@INFRA_SANITIZE@': boot_checks(infra, '_prefs->airtime_factor'), + '@INFRA_SETTER@': extract_braced(infra, 'if (memcmp(config, "dutycycle ", 10) == 0)'), + } + harness = support + HARNESS + for marker, value in replacements.items(): + harness = harness.replace(marker, value) + # Saving an unrelated field must not resurrect a failed tuple later. + harness = harness.replace('static bool error(', '''static bool saveUnrelated(MyMesh& m) { + m._prefs.ble_pin=654321;return m.savePrefs(); +} +static bool error(''') + compiler = shutil.which('g++') or shutil.which('clang++') + self.assertIsNotNone(compiler) + with tempfile.TemporaryDirectory(prefix='companion-primary-radio-') as directory: + work = Path(directory) + # Reuse the profile fixture's identity/crypto mocks and add only + # unused JSON conversion links required by real NodePrefs adapters. + utils = (ROOT/'test/mocks/Utils.h').read_text() + utils = utils.replace('class Utils {\npublic:', '''class Utils { +public: + static void printHex(Stream&,const uint8_t*,size_t){assert(false);} + static void fromHex(uint8_t*,size_t,const char*){assert(false);} +''') + (work/'Utils.h').write_text('#include \n#include \n'+utils) + (work/'Identity.h').write_text((ROOT/'test/mocks/Identity.h').read_text()) + transaction = (ROOT/'src/helpers/ContactFileTransaction.h').read_text() + (work/'ContactFileTransaction.h').write_text(transaction.replace( + '#include "IdentityStore.h"', '#include ')) + (work/'test.cpp').write_text(harness, encoding='utf-8') + binary = work/('test.exe' if os.name=='nt' else 'test') + flags = [] if os.name=='nt' else ['-fsanitize=address,undefined', + '-fno-sanitize-recover=all','-fno-pie','-no-pie'] + build = subprocess.run([compiler,'-std=c++17','-Wall','-Wextra', + '-Wno-unused-parameter','-Wno-sign-compare','-Wno-reorder', + '-DESP32_PLATFORM=1',*flags,'-I',str(work), + '-I',str(ROOT/'test/fixtures/radio_profiles/mocks'), + '-I',str(ROOT/'test/mocks'),'-I',str(ROOT/'src'), + '-I',str(ROOT/'src/helpers'),'-I',str(ROOT),str(work/'test.cpp'), + *[str(ROOT/'src/helpers'/name) for name in ( + 'ConfigSerializer.cpp','DynamicConfigSerializer.cpp', + 'CommonRadioPrefs.cpp','TxtDataHelpers.cpp','RadioProfileCLI.cpp')], + '-o',str(binary)],capture_output=True,text=True) + self.assertEqual(build.returncode,0,build.stdout+build.stderr) + run = subprocess.run([str(binary)],capture_output=True,text=True) + self.assertEqual(run.returncode,0,run.stdout+run.stderr) + + +if __name__=='__main__': + unittest.main() diff --git a/test/test_companion_radio_settings_transaction.py b/test/test_companion_radio_settings_transaction.py index d412e4925..7feb71325 100644 --- a/test/test_companion_radio_settings_transaction.py +++ b/test/test_companion_radio_settings_transaction.py @@ -43,7 +43,7 @@ struct Prefs { uint8_t sf=7, cr=5, client_repeat=0, rx_ps_level=1, rx_ps_preamble=16; uint32_t rx_ps_rx_us=700, rx_ps_sleep_us=25000; int8_t tx_power_dbm=3; - bool rx_boosted_gain=false; + bool rx_boosted_gain=false, rx_powersaving_enabled=false; }; void recalcRxPowerSavingFromLevel(uint8_t,uint8_t sf,float bw,uint8_t, uint32_t* rx,uint32_t* sleep) { @@ -61,6 +61,8 @@ struct Driver { return ok; } void setRxBoostedGainMode(bool) {} + bool supportsRxPowerSaving() const { return false; } + bool setRxPowerSaving(bool,uint32_t,uint32_t) { return true; } } radio_driver; @RETRY_DELAY@ struct MyMesh { diff --git a/test/test_companion_wifi_session.py b/test/test_companion_wifi_session.py index 2cccd7132..dbd50ef80 100644 --- a/test/test_companion_wifi_session.py +++ b/test/test_companion_wifi_session.py @@ -1,6 +1,7 @@ """Exercise WiFi session cancellation against production route ownership code.""" from pathlib import Path import os +import re import subprocess import tempfile import unittest @@ -130,11 +131,60 @@ class CompanionWiFiSessionTest(unittest.TestCase): self.assertLess(add, start) def test_session_reset_cancels_only_wifi_owned_work(self): + self.check_session_reset("WiFi") + + def test_session_reset_cancels_only_ethernet_owned_work(self): + self.check_session_reset("Ethernet") + + def test_callback_is_registered_before_ethernet_interface_can_start(self): + source = MAIN.read_text(encoding="utf-8") + self.assertLess( + source.index("ethernet_interface.setSessionChangedCallback(cancelCompanionEthernetSession, nullptr)"), + source.index("ethernet_interface.begin();"), + ) + + def test_ethernet_callback_guards_cover_profiles_without_usb_or_wifi(self): + # Preserve the real main.cpp conditional nesting, not just an extracted + # callback body: Ethernet-only builds must see its definition as well + # as its setup registration. Headers and unrelated code are unnecessary + # for this preprocessor test and would require the firmware toolchain. + markers = [] + continuation = False + for line in MAIN.read_text(encoding="utf-8").splitlines(keepends=True): + conditional = re.match(r"^\s*#\s*(?:if|ifdef|ifndef|elif|else|endif)\b", line) + if continuation or conditional: + markers.append(line) + continuation = line.rstrip().endswith("\\") + elif "static void cancelCompanionEthernetSession(" in line: + markers.append("ETHERNET_SESSION_CALLBACK_DECLARED\n") + elif "ethernet_interface.setSessionChangedCallback(" in line: + markers.append("ETHERNET_SESSION_CALLBACK_REGISTERED\n") + source = "".join(markers) + for platform in (("ESP32_PLATFORM", "ESP32"), ("NRF52_PLATFORM",)): + for ethernet in (False, True): + for usb in (False, True): + with self.subTest(platform=platform, ethernet=ethernet, usb=usb): + flags = [f"-D{name}=1" for name in platform] + if ethernet: + flags.append("-DETHERNET_ENABLED=1") + if usb: + flags.append("-DENABLE_USB_INTERFACE=1") + result = subprocess.run( + [os.environ.get("CXX", "g++"), "-E", "-P", "-x", "c++", + *flags, "-"], input=source, text=True, capture_output=True, + ) + self.assertEqual(result.returncode, 0, result.stderr) + expected = ("ETHERNET_SESSION_CALLBACK_DECLARED\n" + "ETHERNET_SESSION_CALLBACK_REGISTERED") if ethernet else "" + self.assertEqual(result.stdout.strip(), expected) + + def check_session_reset(self, transport): main = MAIN.read_text(encoding="utf-8") mesh = MESH.read_text(encoding="utf-8") - callback = extract_braced(main, "static void cancelCompanionWiFiSession(") + callback = extract_braced(main, f"static void cancelCompanion{transport}Session(") operations = extract_braced(mesh, "void MyMesh::cancelSerialOperationsForRoute(") - source = HARNESS.replace("@CANCEL_OPERATIONS@", operations).replace( + harness = HARNESS.replace("WiFi", transport).replace("wifi", transport.lower()) + source = harness.replace("@CANCEL_OPERATIONS@", operations).replace( "@CANCEL_SESSION@", callback ) with tempfile.TemporaryDirectory() as directory: diff --git a/test/test_deferred_cli_command/test_deferred_cli_command.cpp b/test/test_deferred_cli_command/test_deferred_cli_command.cpp index 21fd4b9e9..e3f87e1ed 100644 --- a/test/test_deferred_cli_command/test_deferred_cli_command.cpp +++ b/test/test_deferred_cli_command/test_deferred_cli_command.cpp @@ -2,27 +2,38 @@ #include +static const uint8_t FIRST[PUB_KEY_SIZE] = {0x12}; +static const uint8_t SECOND[PUB_KEY_SIZE] = {0x77}; + TEST(DeferredCliCommand, CopiesAuthenticatedCommandContext) { mesh::DeferredCliCommand deferred; uint8_t secret[PUB_KEY_SIZE]; memset(secret, 0x5A, sizeof(secret)); const char command[] = "del flood.moderation.all"; - ASSERT_TRUE(deferred.enqueue(7, 123456U, 2, secret, command, + uint8_t sender[PUB_KEY_SIZE]; + memcpy(sender, FIRST, sizeof(sender)); + ASSERT_TRUE(deferred.enqueue(sender, 123456U, 2, secret, command, strlen(command), 654321U, 1, 91)); EXPECT_TRUE(deferred.pending); - EXPECT_EQ(7, deferred.client_index); + EXPECT_EQ(0, memcmp(FIRST, deferred.client_pub_key, PUB_KEY_SIZE)); EXPECT_EQ(123456U, deferred.sender_timestamp); EXPECT_EQ(654321U, deferred.request_id); + EXPECT_EQ(mesh::RemoteCliReplyCache::fingerprint(command, strlen(command)), + deferred.command_fingerprint); EXPECT_EQ(2, deferred.path_hash_size); EXPECT_EQ(1, deferred.radio_profile); EXPECT_EQ(91U, deferred.radio_generation); EXPECT_EQ(0, memcmp(secret, deferred.secret, sizeof(secret))); EXPECT_STREQ(command, deferred.command); - EXPECT_TRUE(deferred.matches(7, 654321U, command, strlen(command))); - EXPECT_FALSE(deferred.matches(6, 654321U, command, strlen(command))); - EXPECT_FALSE(deferred.matches(7, 654322U, command, strlen(command))); - EXPECT_FALSE(deferred.matches(7, 654321U, "region save", 11)); + EXPECT_TRUE(deferred.matches(FIRST, 654321U, command, strlen(command))); + EXPECT_FALSE(deferred.matches(SECOND, 654321U, command, strlen(command))); + EXPECT_FALSE(deferred.matches(FIRST, 654322U, command, strlen(command))); + EXPECT_FALSE(deferred.matches(FIRST, 654321U, "region save", 11)); + sender[31] = 1; + EXPECT_FALSE(deferred.matches(sender, 654321U, command, strlen(command))); + EXPECT_FALSE(deferred.matches(nullptr, 654321U, command, strlen(command))); + EXPECT_EQ(0, deferred.client_pub_key[31]); secret[0] = 0; EXPECT_EQ(0x5A, deferred.secret[0]); @@ -34,18 +45,20 @@ TEST(DeferredCliCommand, RejectsSecondCommandUntilCleared) { const char first[] = "region save"; const char second[] = "advert"; - ASSERT_TRUE(deferred.enqueue(1, 10U, 1, secret, first, strlen(first))); - EXPECT_FALSE(deferred.enqueue(2, 11U, 3, secret, second, strlen(second))); - EXPECT_EQ(1, deferred.client_index); + ASSERT_TRUE(deferred.enqueue(FIRST, 10U, 1, secret, first, strlen(first))); + EXPECT_FALSE(deferred.enqueue(SECOND, 11U, 3, secret, second, strlen(second))); + EXPECT_EQ(0, memcmp(FIRST, deferred.client_pub_key, PUB_KEY_SIZE)); EXPECT_STREQ(first, deferred.command); deferred.clear(); EXPECT_FALSE(deferred.pending); - EXPECT_FALSE(deferred.matches(1, 10U, first, strlen(first))); + EXPECT_FALSE(deferred.matches(FIRST, 10U, first, strlen(first))); + EXPECT_EQ(0, deferred.client_pub_key[0]); EXPECT_EQ(0, deferred.secret[0]); EXPECT_EQ(0U, deferred.request_id); + EXPECT_EQ(0U, deferred.command_fingerprint); EXPECT_EQ(0, deferred.command[0]); - ASSERT_TRUE(deferred.enqueue(2, 11U, 3, secret, second, strlen(second))); + ASSERT_TRUE(deferred.enqueue(SECOND, 11U, 3, secret, second, strlen(second))); EXPECT_STREQ(second, deferred.command); } @@ -56,11 +69,56 @@ TEST(DeferredCliCommand, RejectsAnOverlongCommand) { memset(command, 'x', sizeof(command)); command[sizeof(command) - 1] = 0; - EXPECT_FALSE(deferred.enqueue(0, 1U, 1, secret, command, + EXPECT_FALSE(deferred.enqueue(FIRST, 1U, 1, secret, command, sizeof(deferred.command))); + EXPECT_FALSE(deferred.enqueue(nullptr, 1U, 1, secret, "advert", 6)); EXPECT_FALSE(deferred.pending); } +TEST(DeferredCliCommand, PreservesWireIdentityAcrossCommandMutation) { + mesh::DeferredCliCommand deferred; + mesh::RemoteCliReplyCache replies; + uint8_t secret[PUB_KEY_SIZE] = {}; + uint8_t sender[PUB_KEY_SIZE] = {0x12}; + const char wire[] = "ADVERT\r\n"; + const auto fingerprint = mesh::RemoteCliReplyCache::fingerprint(wire, strlen(wire)); + ASSERT_TRUE(deferred.enqueue(sender, 100U, 1, secret, wire, strlen(wire), 99U)); + strcpy(deferred.command, "advert"); + EXPECT_EQ(fingerprint, deferred.command_fingerprint); + ASSERT_TRUE(replies.remember(sender, deferred.request_id, + deferred.command_fingerprint, "OK")); + EXPECT_TRUE(replies.matches(sender, 99U, fingerprint)); + EXPECT_FALSE(replies.matches(sender, 99U, + mesh::RemoteCliReplyCache::fingerprint("advert", 6))); + // A rejected enqueue must not overwrite the identity of the current owner. + EXPECT_FALSE(deferred.enqueue(SECOND, 101U, 1, secret, "reboot", 6, 100U)); + EXPECT_EQ(fingerprint, deferred.command_fingerprint); + deferred.clear(); + EXPECT_EQ(0U, deferred.command_fingerprint); +} + +TEST(DeferredCliCommand, ResolvesFullKeyAfterCompactionAndRejectsReusedSlot) { + struct Client { struct { uint8_t pub_key[PUB_KEY_SIZE]; } id; }; + struct ACL { + Client clients[2] = {}; + int count = 2; + int getNumClients() const { return count; } + const Client* getClientByIdx(int i) const { return &clients[i]; } + } acl; + memcpy(acl.clients[0].id.pub_key, FIRST, PUB_KEY_SIZE); + memcpy(acl.clients[1].id.pub_key, SECOND, PUB_KEY_SIZE); + mesh::DeferredCliCommand deferred; + uint8_t secret[PUB_KEY_SIZE] = {}; + EXPECT_EQ(-1, deferred.findClientIndex(acl)); + ASSERT_TRUE(deferred.enqueue(SECOND, 1, 1, secret, "advert", 6)); + EXPECT_EQ(1, deferred.findClientIndex(acl)); + acl.clients[0] = acl.clients[1]; + acl.count = 1; + EXPECT_EQ(0, deferred.findClientIndex(acl)); + acl.clients[0].id.pub_key[31] = 1; + EXPECT_EQ(-1, deferred.findClientIndex(acl)); +} + int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); diff --git a/test/test_indicator_exclusive_transport.py b/test/test_indicator_exclusive_transport.py index 0c7878e60..607052344 100644 --- a/test/test_indicator_exclusive_transport.py +++ b/test/test_indicator_exclusive_transport.py @@ -89,6 +89,9 @@ assert re.search( ) assert re.search( r"#ifdef WIFI_SSID\s+" + # Registering a callback does not start the WiFi stack. It must remain + # available for session cleanup, while startup stays behind the boot gate. + r"wifi_interface\.setSessionChangedCallback\(cancelCompanionWiFiSession, nullptr\);\s+" r"#if defined\(COMPANION_EXCLUSIVE_WIFI_BLE\)\s+" r"if \(companionTransportWiFiActiveAtBoot\(\)\) \{.*?" r"WiFi\.onEvent\(.*?startCompanionWiFi\(\);.*?" diff --git a/test/test_local_cli_access.py b/test/test_local_cli_access.py index ed0b116b8..1bf0c8646 100644 --- a/test/test_local_cli_access.py +++ b/test/test_local_cli_access.py @@ -27,7 +27,8 @@ PREAMBLE = r''' #define TXT_TYPE_CLI_COMMAND 1 namespace mesh { struct RadioProfileCLI { - bool handle(const char*, char*, size_t) { return false; } + bool remote_origin=false; + bool handle(const char*, char*, size_t, bool remote=false) { remote_origin=remote;return false; } static bool parseSuffix(const char* input, unsigned fields, char* legacy, size_t size, uint16_t& preamble) { return cli::parseRadioPreambleSuffix(input, fields, legacy, size, preamble); } @@ -108,6 +109,7 @@ struct MyMesh { int resets=0, saves=0; enum class RadioSettingResult { Saved, RadioRejected, SaveFailed }; RadioSettingResult applyAndSaveTxPower(int8_t); + bool saveAdvertName(const char*); bool handleCommand(const char*,uint32_t,char*); bool handleDirectCommand(const char*,char*,size_t); void onCLICommandRecv(const ContactInfo&,mesh::Packet*,uint32_t,const char*,char*); @@ -163,12 +165,15 @@ int main() { assert(!strstr(reply,"secret") && !strstr(reply,"token") && !strstr(reply,"A5A5")); } assert(node.handleCommand("set freq 915.25",0,reply)); + assert(!node._radio_profiles.remote_origin); assert(node._prefs.freq==915.25f && strstr(reply,"reboot to apply")); assert(node.handleCommand("set freq 920",100,reply)); + assert(node._radio_profiles.remote_origin); assert(node._prefs.freq==920.0f && strstr(reply,"reboot to apply")); for(uint32_t stamp:{0u,100u}) { reply[0]=0; node.onCLICommandRecv(ContactInfo{},nullptr,stamp,"A7|set freq 921.125",reply); + assert(node._radio_profiles.remote_origin); // even a zero-timestamp LoRa command assert(node._prefs.freq==921.125f && !strcmp(reply,"A7|OK - reboot to apply")); assert(node._prefs.bw==125.0f && node._prefs.sf==7 && node._prefs.cr==5); for(const char* bad:{"set freq 149.9","set freq 2500.1","set freq 920oops", @@ -309,6 +314,7 @@ int main() { implementation = "\n".join(extract_braced(source, signature) for signature in ( "static bool isCompanionRadioPrefsCommand(", "MyMesh::RadioSettingResult MyMesh::applyAndSaveTxPower(", + "bool MyMesh::saveAdvertName(", "bool MyMesh::handleDirectCommand(", "bool MyMesh::handleCommand(", "void MyMesh::onCLICommandRecv(")) for export, webconfig in ((0, False), (1, False), (0, True), (1, True)): diff --git a/test/test_ota_store_flash_nrf52_hybrid.py b/test/test_ota_store_flash_nrf52_hybrid.py index 1ab411ff3..a4166781d 100644 --- a/test/test_ota_store_flash_nrf52_hybrid.py +++ b/test/test_ota_store_flash_nrf52_hybrid.py @@ -30,6 +30,8 @@ class OtaStoreFlashNrf52HybridTest(unittest.TestCase): "-Wall", "-Wextra", "-Werror", + "-Wno-unused-parameter", # shared board interfaces and host mocks + "-Wno-sign-compare", # existing Stream mock "-DNRF52_PLATFORM=1", "-DOTA_FLASH_STORE=1", "-DOTA_HYBRID_RAM_STORE=1", @@ -37,7 +39,11 @@ class OtaStoreFlashNrf52HybridTest(unittest.TestCase): "-DMOTA_NRF52_TEST_LAYOUT_STAGE_CEILING=0x000ED000u", f"-I{FIXTURE / 'mocks'}", f"-I{ROOT / 'src'}", + f"-I{ROOT / 'test/mocks'}", str(FIXTURE / "test_ota_store_flash_nrf52_hybrid.cpp"), + str(ROOT / "src/helpers/ota/MotaContainer.cpp"), + str(ROOT / "src/helpers/ota/MerkleTree.cpp"), + str(ROOT / "src/Utils.cpp"), "-o", str(binary), ], @@ -51,10 +57,10 @@ class OtaStoreFlashNrf52HybridTest(unittest.TestCase): ) self.assertEqual(checked.returncode, 0, checked.stdout + checked.stderr) self.assertIn( - "10 OtaStoreFlashNrf52 hybrid lifecycle checks passed", + "11 OtaStoreFlashNrf52 hybrid lifecycle checks passed", checked.stdout, ) - self.assertEqual(checked.stdout.count("PASS:"), 10) + self.assertEqual(checked.stdout.count("PASS:"), 11) if __name__ == "__main__": diff --git a/test/test_ota_store_resume.py b/test/test_ota_store_resume.py index af237bc23..81b67fab1 100644 --- a/test/test_ota_store_resume.py +++ b/test/test_ota_store_resume.py @@ -25,14 +25,14 @@ class OtaStoreResumeTest(unittest.TestCase): binary = path / "resume.exe" result = subprocess.run([ "c++", "-std=c++17", *flags, "-DENABLE_OTA=1", "-DESP32_PLATFORM=1", - "-DOTA_FLASH_STORE=1", "-I", str(FIXTURE / "mocks"), + "-DOTA_FLASH_STORE=1", "-DOTA_FETCH_PIPELINE=4", "-I", str(FIXTURE / "mocks"), "-I", str(ROOT / "src"), "-I", str(ROOT / "test/mocks"), str(FIXTURE / "test.cpp"), *[str(ROOT / "src/helpers/ota" / name) for name in sources], str(ROOT / "src/Utils.cpp"), str(tinf), "-o", str(binary), ], capture_output=True, text=True) self.assertEqual(result.returncode, 0, result.stderr) - for scenario in range(15): + for scenario in range(46): with self.subTest(scenario=scenario): subprocess.run([str(binary), str(scenario)], check=True, timeout=10) diff --git a/test/test_radio_profile_scan.py b/test/test_radio_profile_scan.py index cb2063fd4..3d5bb0c5a 100644 --- a/test/test_radio_profile_scan.py +++ b/test/test_radio_profile_scan.py @@ -41,6 +41,9 @@ struct RadioLibWrapper { uint16_t _physical_preamble=32; float _cur_freq=909.5, _cur_bw=62.5; unsigned applies=0, failRxStarts=0; + unsigned coding_writes=0; + bool coding_success=true; + bool setCodingRate(uint8_t) { ++coding_writes; return coding_success; } RadioLibWrapper() { state=STATE_RX; elapsed_us=100000; auto& p=_profiles.primary; @@ -73,6 +76,7 @@ struct RadioLibWrapper { mesh::RadioParamApplyResult tuneProfile(uint8_t); mesh::RadioParamApplyResult prepareTransmitProfile(uint8_t); mesh::RadioParamApplyResult prepareTransmitProfile(uint8_t, bool); + mesh::RadioParamApplyResult tryRestoreCodingRate(uint8_t); mesh::RadioParamApplyResult trySetParams(float,float,uint8_t,uint8_t,const uint32_t* = nullptr); mesh::RadioParamApplyResult trySetPrimaryParams(const mesh::RadioProfileParams&,bool,const uint32_t* = nullptr); void enable() { @@ -85,6 +89,19 @@ struct RadioLibWrapper { @METHODS@ int main() { using Result=mesh::RadioParamApplyResult; + for (unsigned blocked=0; blocked<5; ++blocked) { + RadioLibWrapper w; + w._cw_active = blocked==0; w.busy = blocked==1; w.packet = blocked==2; + if (blocked==3) state |= STATE_INT_READY; + if (blocked==4) state = STATE_TX_WAIT; + assert(w.tryRestoreCodingRate(5)==Result::BUSY && w.coding_writes==0); + } + for (bool success : {false,true}) { + RadioLibWrapper w; w.coding_success=success; + assert(w.tryRestoreCodingRate(5)==(success?Result::APPLIED:Result::FAILED)); + assert(w.coding_writes==1 && state==STATE_RX); + assert(w._profile_refresh_required==!success); + } { // Force only admits a reply on an active RX-only profile. It cannot // enable a disabled profile or erase an in-progress reception. @@ -258,7 +275,7 @@ class ProfileScanTest(unittest.TestCase): source=(ROOT/'src/helpers/radiolib/RadioLibWrappers.cpp').read_text() names=[('uint8_t','beginReconfigure'),('void','endReconfigure'), ('void','serviceProfileScan')]+[('mesh::RadioParamApplyResult',name) for name in - ['tuneProfile','prepareTransmitProfile','trySetParams','trySetPrimaryParams']] + ['tuneProfile','prepareTransmitProfile','tryRestoreCodingRate','trySetParams','trySetPrimaryParams']] methods='\n'.join(method(source,f'{kind} RadioLibWrapper::{name}(') for kind,name in names) methods+='\n'+method(source, 'mesh::RadioParamApplyResult RadioLibWrapper::prepareTransmitProfile(uint8_t profile, bool') diff --git a/test/test_radio_reply_mutation.py b/test/test_radio_reply_mutation.py new file mode 100644 index 000000000..277323a05 --- /dev/null +++ b/test/test_radio_reply_mutation.py @@ -0,0 +1,200 @@ +"""Compile production secondary CLI and Dispatcher with injected transport/storage faults.""" +from pathlib import Path +import os +import shutil +import subprocess +import tempfile +import unittest + +ROOT = Path(__file__).resolve().parents[1] + + +def method(text, signature): + start = text.index(signature) + end = text.index('{', start) + 1 + depth = 1 + while depth: + depth += (text[end] == '{') - (text[end] == '}') + end += 1 + return text[start:end] + + +LIFECYCLE = r''' +#include +#include +#include +#include +#include +static uint32_t now_ms = 1000; +uint32_t millis() { return now_ms; } +namespace mesh { +struct Packet {}; +struct Mesh { + virtual void onSendComplete(Packet*) {} + virtual void onSendFail(Packet*) {} + virtual void onRadioProfileCopyQueued(Packet*, const Packet*, uint8_t) {} +}; +} +struct Profiles { + bool pending = false, accepted = false; + unsigned completions = 0; + bool finishReplyMutation(bool delivered) { + ++completions; accepted = delivered; pending = delivered; return true; + } +}; +struct CLI { + Profiles profiles; + Profiles& radioProfiles() { return profiles; } +}; +struct Manager { + std::vector queue; + int getOutboundTotal() const { return queue.size(); } + mesh::Packet* getOutboundByIdx(int i) { return queue[i]; } + mesh::Packet* removeOutboundByIdx(int i) { + auto* packet = queue[i]; queue.erase(queue.begin()+i); return packet; + } +}; +struct MyMesh : mesh::Mesh { + CLI _cli; + Manager manager; Manager* _mgr = &manager; + mesh::TempRadioReplyBarrier temp_radio_reply_barrier; + bool radio_reply_secondary = false, primary_radio_mutation_starts_temp = false; + uint32_t radio_reply_deadline = 0; + mesh::Packet* pending_battery_alert_packet = nullptr; + bool battery_alert_sent = false; + uint64_t last_battery_alert_sent = 0, uptime_millis = 0; + uint32_t last_millis = 0; + mesh::Packet* outbound = nullptr; + unsigned released = 0, normal_schedules = 0, cancellations = 0; + const mesh::Packet* getOutboundInFlight() const { return outbound; } + void cancelOutboundRadioRetry(const mesh::Packet* p) { assert(p == outbound); ++cancellations; } + void releasePacket(mesh::Packet*) { ++released; } + bool scheduleNormalRadio() { ++normal_schedules; return true; } + void onSendComplete(mesh::Packet*) override; + void onSendFail(mesh::Packet*) override; + void onRadioProfileCopyQueued(mesh::Packet*, const mesh::Packet*, uint8_t) override; + void finishRadioReply(bool); + void serviceRadioReplyDeadline(); + void arm(mesh::Packet* a, mesh::Packet* b = nullptr) { + temp_radio_reply_barrier.prepare(a); + if (b) onRadioProfileCopyQueued(b, a, 0); + temp_radio_reply_barrier.arm(a); + radio_reply_deadline = now_ms + 300000; + } +}; +@METHODS@ +int main() { + unsigned scenarios = 0; + for (bool secondary : {false, true}) for (unsigned success = 0; success < 4; ++success) { + MyMesh m; mesh::Packet a, b, unrelated; + m.radio_reply_secondary = secondary; + m.primary_radio_mutation_starts_temp = true; + m._cli.profiles.pending = secondary; + m.arm(&a, &b); + m.onSendComplete(&unrelated); + assert(m.temp_radio_reply_barrier.waiting()); + if (success & 1) m.onSendComplete(&a); else m.onSendFail(&a); + assert(m.temp_radio_reply_barrier.waiting()); + assert(m._cli.profiles.completions == 0 && m.normal_schedules == 0); + if (success & 2) m.onSendComplete(&b); else m.onSendFail(&b); + assert(!m.temp_radio_reply_barrier.waiting()); + assert(m.radio_reply_deadline == 0); + if (secondary) { + assert(m._cli.profiles.completions == 1 && m._cli.profiles.accepted == (success != 0)); + assert(m.normal_schedules == 0); + } else assert(m.normal_schedules == (success == 0 ? 1U : 0U)); + ++scenarios; + } + { // Deleting an active schedule may restore saved radio, but must not erase other slots on ACK failure. + MyMesh m; mesh::Packet a; + m.primary_radio_mutation_starts_temp = false; + m.arm(&a); m.onSendFail(&a); + assert(!m.temp_radio_reply_barrier.waiting() && m.normal_schedules == 0); + ++scenarios; + } + for (bool rollover : {false, true}) { + now_ms = rollover ? UINT32_MAX - 10 : 1000; + MyMesh m; mesh::Packet a, b, ordinary; + m.radio_reply_secondary = true; + m.arm(&a, &b); m.manager.queue = {&a, &ordinary, &b}; + now_ms += 299999; m.serviceRadioReplyDeadline(); + assert(m.temp_radio_reply_barrier.waiting() && m.released == 0); + ++now_ms; m.serviceRadioReplyDeadline(); + assert(!m.temp_radio_reply_barrier.waiting() && m.released == 2); + assert(m.manager.queue.size() == 1 && m.manager.queue[0] == &ordinary); + assert(!m._cli.profiles.accepted && m._cli.profiles.completions == 1); + ++scenarios; + } + { // A physically transmitting copy retains ownership until its actual completion/watchdog. + MyMesh m; mesh::Packet a, b; + m.radio_reply_secondary = true; + m.arm(&a, &b); m.outbound = &a; m.manager.queue = {&b}; + now_ms += 300000; m.serviceRadioReplyDeadline(); + assert(m.cancellations == 1 && m.released == 1); + assert(m.temp_radio_reply_barrier.waiting() && m._cli.profiles.completions == 0); + m.onSendComplete(&a); + assert(!m.temp_radio_reply_barrier.waiting() && m._cli.profiles.accepted); + ++scenarios; + } + { // Once one copy succeeded, a timeout can retire the other without canceling the mutation. + MyMesh m; mesh::Packet a, b; + m.radio_reply_secondary = true; + m.arm(&a, &b); m.onSendComplete(&a); m.manager.queue = {&b}; + now_ms += 300000; m.serviceRadioReplyDeadline(); + assert(m.released == 1 && m._cli.profiles.accepted); + assert(!m.temp_radio_reply_barrier.waiting()); + ++scenarios; + } + assert(scenarios == 13); +} +''' + + +class RadioReplyMutationTest(unittest.TestCase): + @staticmethod + def compiler_flags(): + return ['-std=c++17', '-Wall', '-Wextra'] + ([] if os.name == 'nt' else + ['-fsanitize=address,undefined', '-fno-omit-frame-pointer', '-fno-pie', '-no-pie']) + + def test_radio_transactions_and_coding_rate_restore(self): + compiler = shutil.which('g++') or shutil.which('clang++') + self.assertIsNotNone(compiler) + with tempfile.TemporaryDirectory() as work: + exe = Path(work) / 'radio_replies.exe' + result = subprocess.run([compiler, *self.compiler_flags(), + '-I', str(ROOT/'test/fixtures/radio_profiles/mocks'), + '-I', str(ROOT/'test/mocks'), '-I', str(ROOT/'src'), + str(ROOT/'src/helpers/RadioProfileCLI.cpp'), + str(ROOT/'src/Dispatcher.cpp'), str(ROOT/'src/Packet.cpp'), + str(ROOT/'src/helpers/StaticPoolPacketManager.cpp'), + str(ROOT/'test/fixtures/radio_profiles/reply_mutation_test.cpp'), + '-o', str(exe)], capture_output=True, text=True) + self.assertEqual(result.returncode, 0, result.stderr[-14000:]) + env = os.environ.copy() + # The production pool is a boot-lifetime singleton without a + # destructor; this harness constructs several independent pools. + # Keep bounds/UAF/UB sanitizers, but don't count that intentional + # lifetime allocation as a host-process teardown leak. + if os.name != 'nt': + env['ASAN_OPTIONS'] = env.get('ASAN_OPTIONS', '') + ':detect_leaks=0' + subprocess.run([str(exe)], check=True, env=env) + + def test_production_repeater_ack_callbacks_and_deadline(self): + source = (ROOT/'examples/simple_repeater/MyMesh.cpp').read_text(encoding='utf-8') + methods = '\n'.join(method(source, signature) for signature in ( + 'void MyMesh::onSendComplete(', 'void MyMesh::onSendFail(', + 'void MyMesh::onRadioProfileCopyQueued(', 'void MyMesh::finishRadioReply(', + 'void MyMesh::serviceRadioReplyDeadline(')) + compiler = shutil.which('g++') or shutil.which('clang++') + self.assertIsNotNone(compiler) + with tempfile.TemporaryDirectory() as work: + cpp = Path(work)/'lifecycle.cpp'; exe = Path(work)/'lifecycle.exe' + cpp.write_text(LIFECYCLE.replace('@METHODS@', methods), encoding='utf-8') + result = subprocess.run([compiler, *self.compiler_flags(), '-I', str(ROOT/'src'), + str(cpp), '-o', str(exe)], capture_output=True, text=True) + self.assertEqual(result.returncode, 0, result.stderr[-14000:]) + subprocess.run([str(exe)], check=True) + + +if __name__ == '__main__': + unittest.main() diff --git a/test/test_remote_cli_command_identity.py b/test/test_remote_cli_command_identity.py new file mode 100644 index 000000000..aad76f88c --- /dev/null +++ b/test/test_remote_cli_command_identity.py @@ -0,0 +1,73 @@ +"""Exercise production repeater receive/dispatch against mutating CLI handlers.""" +from pathlib import Path +import os +import re +import subprocess +import sys +import tempfile +import unittest + +from test_replay_reset_integration import extract_braced + +ROOT = Path(__file__).resolve().parents[1] +SOURCE = ROOT / "examples/simple_repeater/MyMesh.cpp" +FIXTURE = ROOT / "test/fixtures/remote_cli_command_identity/test.cpp" + + +class RemoteCliCommandIdentityTest(unittest.TestCase): + def test_wire_identity_survives_mutation_and_retries(self): + source = SOURCE.read_text(encoding="utf-8") + receive = extract_braced(source, "void MyMesh::onPeerDataRecv(") + dispatch = extract_braced(source, "void __attribute__((noinline)) MyMesh::processDeferredCliCommand()") + trim_start = source.index(" char* command_end = command + strlen(command);") + trim_end = source.index("\n\n", trim_start) + normalize_start = source.index(" while (*command == ' ') command++;", trim_end) + normalize_end = source.index(" mesh::cli::normalizeCommandVerb(command);", normalize_start) + normalize_end += len(" mesh::cli::normalizeCommandVerb(command);") + mutation = source[trim_start:trim_end] + "\n" + source[normalize_start:normalize_end] + # setperm splits the argument in the same buffer; retain its real + # delimiter mutation without pulling ACL/filesystem hardware into this test. + setperm = source.index('char* hex = &command[8];', normalize_end) + tokenize_end = source.index('uint8_t pubkey[PUB_KEY_SIZE];', setperm) + tokenize = source[setperm:tokenize_end] + tokenize = re.sub(r"size_t hex_len = [^;]+;", "", tokenize) + # Close the successful-argument branch whose delimiter statement we extracted. + mutation += '\nif (strncmp(command, "setperm ", 8) == 0) {\n' + tokenize + '\n}\n}\n' + generated = mutation + "\n" + acl = (ROOT / "src/helpers/ClientACL.cpp").read_text(encoding="utf-8") + delete_start = acl.index(" num_clients--; // delete from contacts[]") + delete_end = acl.index(" } else {", delete_start) + completion = extract_braced(source, "bool MyMesh::completeHostCliRequest(") + serial_reply = extract_braced(source, "bool MyMesh::handleHostCliSerialReply(") + clear = extract_braced(source, "void MyMesh::clearDeferredCliCommand()") + with tempfile.TemporaryDirectory(prefix="meshcore-cli-identity-") as directory: + work = Path(directory) + (work / "normalization.inc").write_text(generated, encoding="utf-8") + (work / "acl_delete.inc").write_text(acl[delete_start:delete_end], encoding="utf-8") + (work / "production.inc").write_text("\n".join( + (receive, dispatch, completion, serial_reply, clear)), encoding="utf-8") + binary = work / "cli-identity.exe" + sanitizer = ["-fsanitize=address,undefined", "-fno-sanitize-recover=all", + "-fno-pie", "-no-pie"] if sys.platform.startswith("linux") else [] + for host_enabled in (0, 1): + with self.subTest(host_enabled=host_enabled): + result = subprocess.run([ + os.environ.get("CXX", "g++"), "-std=c++17", "-Wall", "-Wextra", + "-Werror", "-Wno-unused-parameter", *sanitizer, + f"-DMESH_ENABLE_HOST_CLI={host_enabled}", + f"-I{work}", f"-I{ROOT / 'src'}", str(FIXTURE), "-o", str(binary), + ], capture_output=True, text=True, timeout=60) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + checked = subprocess.run([str(binary)], capture_output=True, text=True, timeout=10) + self.assertEqual(checked.returncode, 0, checked.stdout + checked.stderr) + self.assertIn("CLI wire identity checks passed", checked.stdout) + + def test_host_completion_uses_original_wire_fingerprint(self): + source = SOURCE.read_text(encoding="utf-8") + completion = extract_braced(source, "bool MyMesh::completeHostCliRequest(") + self.assertIn("deferred_cli_command.command_fingerprint", completion) + self.assertNotIn("RemoteCliReplyCache::fingerprint(", completion) + + +if __name__ == "__main__": + unittest.main() diff --git a/test/test_remote_cli_reply_cache/test_remote_cli_reply_cache.cpp b/test/test_remote_cli_reply_cache/test_remote_cli_reply_cache.cpp index 41718dcdf..d58b09ea9 100644 --- a/test/test_remote_cli_reply_cache/test_remote_cli_reply_cache.cpp +++ b/test/test_remote_cli_reply_cache/test_remote_cli_reply_cache.cpp @@ -133,6 +133,27 @@ TEST(RemoteCliRequest, LogicalIdExtensionIsBackwardCompatible) { legacy, sizeof(legacy), 5, logical_id)); } +TEST(RemoteCliReplyCache, AuthoritativeReplyFlagIsTypedAndResetOnReuse) { + mesh::RemoteCliReplyCache cache; + uint8_t key[PUB_KEY_SIZE] = {}; + bool authoritative = false; + ASSERT_TRUE(cache.remember(key, 1, 2, "ab|OK", true)); + ASSERT_TRUE(cache.lookup(key, 1, 2, nullptr, &authoritative)); + EXPECT_TRUE(authoritative); + ASSERT_TRUE(cache.remember(key, 1, 2, "ordinary reply")); + ASSERT_TRUE(cache.lookup(key, 1, 2, nullptr, &authoritative)); + EXPECT_FALSE(authoritative); + ASSERT_TRUE(cache.remember(key, 1, 2, "pending", true)); + for (unsigned i=2; i<=mesh::RemoteCliReplyCache::ENTRY_COUNT+1; ++i) { + ASSERT_TRUE(cache.remember(key, i, 2, "replacement")); + } + ASSERT_TRUE(cache.lookup(key, mesh::RemoteCliReplyCache::ENTRY_COUNT+1, 2, nullptr, &authoritative)); + EXPECT_FALSE(authoritative); + authoritative = true; + EXPECT_FALSE(cache.lookup(key, 1234, 2, nullptr, &authoritative)); + EXPECT_FALSE(authoritative); +} + int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); diff --git a/test/test_repeater_radio_timing_integration.py b/test/test_repeater_radio_timing_integration.py index c8c472b00..14c1f91e6 100644 --- a/test/test_repeater_radio_timing_integration.py +++ b/test/test_repeater_radio_timing_integration.py @@ -57,6 +57,8 @@ struct CLI { bool preamble_save_success = true; unsigned preamble_saves = 0; bool savePrimaryPreamble(uint16_t) { ++preamble_saves; return preamble_save_success; } + bool hasReplyMutation() const { return false; } + bool finishReplyMutation(bool) { return true; } }; struct Radio { uint32_t last_rx = 0; uint32_t getLastRecvMillis() { return last_rx; } }; struct Barrier { bool held = false; bool waiting() const { return held; } void clear() { held = false; } }; @@ -83,6 +85,9 @@ public: uint32_t getLastMeshCoreRecvMillis() const { return last_meshcore_rx; } RTC rtc; Barrier temp_radio_reply_barrier; + uint32_t primary_radio_mutation_generation = 0, radio_reply_deadline = 0; + bool primary_radio_mutation_starts_temp = false; + bool radio_reply_secondary = false; mesh::RxInactivityWatchdog rx_inactivity_watchdog; mesh::RepeaterRadioTiming radio_timing; ScheduledRadioSetting scheduled_radio_settings[MAX_SCHEDULED_RADIO_SETTINGS]; @@ -157,6 +162,20 @@ static void startTemp(MyMesh& m, int minutes) { assert(m.temp_radio_applied); } int main() { + { // Expiring one scheduled lease must retain a later temporary entry. + MyMesh m; + char reply[160]; + m.addScheduledRadioParams(true, 910, 250, 5, 5, m.rtc.now + 2, m.rtc.now + 62, reply); + m.addScheduledRadioParams(true, 911, 250, 5, 5, m.rtc.now + 122, m.rtc.now + 182, reply); + assert(m.countScheduledRadioSettings(true) == 2); + m.advance(2); m.servicePostMeshLoop(); assert(m.temp_radio_applied); + m.advance(60); m.servicePostMeshLoop(); + assert(m.countScheduledRadioSettings(true) == 1 && !m.temp_radio_applied); + m.advance(60); m.servicePostMeshLoop(); assert(m.temp_radio_applied); + const auto before = m.primary_radio_mutation_generation; + m.queueSavedRadioApply(); + assert(m.primary_radio_mutation_generation != before); + } for (bool rollover : {false, true}) { MyMesh m; if (rollover) { now_ms = UINT32_MAX - 30000; m.last_millis = now_ms; } diff --git a/test/test_replay_reset_integration.py b/test/test_replay_reset_integration.py index 32e71b399..3817d7599 100644 --- a/test/test_replay_reset_integration.py +++ b/test/test_replay_reset_integration.py @@ -4,6 +4,7 @@ from pathlib import Path import re import shutil import subprocess +import sys import tempfile import unittest @@ -56,6 +57,7 @@ static bool apply_actual_receive_cache_gate(const char* command, bool cache_hit, ClientInfo* client = &sender; uint32_t request_id = 1, command_fingerprint = 2; const char* cached_response = nullptr; + bool cached_authoritative_reply = false; CountingReplyCache remote_cli_reply_cache{cache_hit, lookup_calls}; """ + prepare + "\n" + cache_gate + "\nreturn cached_retry;\n}\n" with tempfile.TemporaryDirectory(prefix=".tmp-replay-integration-", dir=ROOT) as directory: @@ -64,6 +66,8 @@ static bool apply_actual_receive_cache_gate(const char* command, bool cache_hit, binary = work / "replay-reset-integration.exe" compiled = subprocess.run([ compiler, "-std=c++17", "-Wall", "-Wextra", "-Werror", + *(["-fsanitize=address,undefined", "-fno-sanitize-recover=all", + "-fno-pie", "-no-pie"] if sys.platform.startswith("linux") else []), f"-I{work}", f"-I{ROOT / 'src'}", str(FIXTURE / "test_replay_reset_integration.cpp"), "-o", str(binary), ], capture_output=True, text=True, timeout=60) diff --git a/test/test_serial_ethernet_sessions.py b/test/test_serial_ethernet_sessions.py new file mode 100644 index 000000000..d268c17b1 --- /dev/null +++ b/test/test_serial_ethernet_sessions.py @@ -0,0 +1,79 @@ +#!/usr/bin/env python3 +"""Exercise production Ethernet framing, backpressure and session ownership.""" + +from pathlib import Path +import os +import shutil +import subprocess +import tempfile +import unittest + +from test_serial_wifi_sessions import SANITIZER_FLAGS +from test_replay_reset_integration import extract_braced + +ROOT = Path(__file__).resolve().parents[1] +FIXTURE = ROOT / "test/fixtures/serial_ethernet_sessions" +MOCKS = ROOT / "test/fixtures/serial_wifi_sessions/mocks" + +SCENARIOS = ( + "short header writes retain suffix", + "short payload writes retain suffix", + "zero-byte write is backpressure", + "higher-priority response cannot interrupt partial head", + "full queue cannot evict partly sent head", + "maximum frame final byte survives priority insertion", + "same-IP live replacement replays whole frame and keeps owner", + "different-IP replacement cancels owner and clears bounded queue", + "actual disconnect cancels owner but keeps complete same-IP backlog", + "replacement clears old stream route before exposing new command", + "disable closes socket cancels once and clears queue", + "oversized input disconnects without truncated execution", + "incomplete command deadline disconnects", + "fragmented maximum frame and next command parse intact", + "incomplete command deadline survives millis rollover", + "null output never dispatches or dereferences a command", +) + + +class SerialEthernetSessionTests(unittest.TestCase): + def test_production_transport_sessions(self): + compiler = os.environ.get("CXX") or shutil.which("g++") or shutil.which("clang++") + if compiler is None: + self.skipTest("a host C++17 compiler is required") + with tempfile.TemporaryDirectory(prefix="meshcore-ethernet-sessions-") as directory: + binary = Path(directory) / "ethernet-sessions.exe" + for raw in (0, 1): + compiled = subprocess.run( + [compiler, "-std=c++17", "-Wall", "-Wextra", "-Werror", + *SANITIZER_FLAGS, f"-DETHERNET_RAW_LINE={raw}", + f"-I{MOCKS}", f"-I{ROOT / 'src'}", + str(FIXTURE / "test.cpp"), + str(ROOT / "src/helpers/ethernet/SerialEthernetInterface.cpp"), + "-o", str(binary)], capture_output=True, text=True, timeout=60, + ) + self.assertEqual(compiled.returncode, 0, compiled.stdout + compiled.stderr) + for scenario, description in enumerate(SCENARIOS): + with self.subTest(raw=raw, scenario=description): + checked = subprocess.run( + [str(binary), str(scenario)], capture_output=True, + text=True, timeout=10, + ) + self.assertEqual(checked.returncode, 0, checked.stdout + checked.stderr) + self.assertIn(f"PASS: Ethernet session scenario {scenario}", checked.stdout) + + def test_hardware_drivers_cancel_before_exposing_new_client(self): + for driver in ("ch390/CH390EthernetInterface", "RAK13800/RAK13800EthernetInterface"): + source = (ROOT / f"src/helpers/ethernet/{driver}.cpp").read_text(encoding="utf-8") + name = driver.split("/")[-1] + loop = extract_braced(source, f"void {name}::loop()") + with self.subTest(driver=name): + self.assertLess(loop.index("disconnectClient();"), loop.index("onClientConnected(")) + self.assertLess(loop.index("onClientConnected((uint32_t)remoteIp);"), + loop.index("client = newClient;")) + close = extract_braced(source, f"void {name}::disconnectClient()") + self.assertIn("_isConnected = false;", close) + self.assertIn("client.stop();", close) + + +if __name__ == "__main__": + unittest.main() diff --git a/test/test_serial_wifi_sessions.py b/test/test_serial_wifi_sessions.py index 9740ce857..2bf9f6dcb 100644 --- a/test/test_serial_wifi_sessions.py +++ b/test/test_serial_wifi_sessions.py @@ -45,6 +45,17 @@ SCENARIOS = ( "disable cancels owner once and resets partial RX state", "maximum-sized partly sent frame stays intact before higher priority response", "maximum-sized partial frame and parked queue survive owner swap", + "oversized header disconnects immediately without reading its body", + "invalid type disconnects without discard loop or tail dispatch", + "zero-length header disconnects without tail dispatch", + "incomplete payload disconnects after bounded deadline", + "incomplete header disconnects after bounded deadline", + "short payload read never dispatches a truncated command", + "short header read disconnects and resets framing", + "fragmented legal frame completes before deadline", + "maximum-sized and consecutive legal frames parse intact", + "incomplete payload deadline survives millis rollover", + "physical disconnect cancels once even with incomplete input and queued reply", ) diff --git a/test/test_temp_radio_reply_delivery_contract.py b/test/test_temp_radio_reply_delivery_contract.py index 4a8f606f8..41dc03905 100644 --- a/test/test_temp_radio_reply_delivery_contract.py +++ b/test/test_temp_radio_reply_delivery_contract.py @@ -52,7 +52,7 @@ class TempRadioReplyDeliveryContractTest(unittest.TestCase): delivery, ) - def test_only_successful_parameterized_temp_radio_requests_track_reply(self): + def test_accepted_mutations_use_typed_identity_not_raw_command_spelling(self): source = SOURCE.read_text(encoding="utf-8") receive = function_body( source, @@ -65,17 +65,16 @@ class TempRadioReplyDeliveryContractTest(unittest.TestCase): "bool MyMesh::onPeerPathRecv(", ) - self.assertIn( - 'strncmp(deferred_cli_command.command, "tempradio ", 10)', command - ) - self.assertIn('strncmp(reply, "OK - temp params for ", 21)', command) + self.assertIn('primary_radio_mutation_generation != primary_mutation_before', command) + self.assertIn('replyMutationGeneration() != secondary_mutation_before', command) + self.assertNotIn('strncmp(deferred_cli_command.command, "tempradio ', command) self.assertIn("arms_temp_radio ? &queued_reply : NULL", command) # A packet-level retry can arrive after the success text was cached but # before the authoritative reply drains. It must not create a second, # untracked success packet either. - self.assertIn("const bool cached_temp_radio_success", receive) - suppress_start = receive.index("if (cached_temp_radio_success)") + self.assertIn("bool cached_authoritative_reply", receive) + suppress_start = receive.index("if (cached_authoritative_reply)") replay_start = receive.index("} else {", suppress_start) replay_end = receive.index( "} else if (deferred_cli_command.matches", replay_start