From 8f09daaad8ea6e811a5b17dd2ceadf9773f9698a Mon Sep 17 00:00:00 2001 From: mikecarper Date: Tue, 15 Sep 2026 15:15:45 -0700 Subject: [PATCH] Improve Companion, repeater, and OTA reliability --- .github/workflows/run-unit-tests.yml | 1 + examples/companion_radio/MyMesh.cpp | 38 +++-- examples/simple_repeater/MyMesh.cpp | 10 +- src/helpers/ota/OtaManager.cpp | 9 +- test/fixtures/ota_store_resume/test.cpp | 43 ++++++ .../remote_cli_command_identity/test.cpp | 52 ++++++- test/test_companion_prefs_transactions.py | 72 ++++++++- ...st_companion_radio_settings_transaction.py | 3 +- test/test_companion_response_bounds.py | 143 ++++++++++++++++++ test/test_ota_store_resume.py | 2 +- 10 files changed, 345 insertions(+), 28 deletions(-) create mode 100644 test/test_companion_response_bounds.py diff --git a/.github/workflows/run-unit-tests.yml b/.github/workflows/run-unit-tests.yml index 465d24fb..31e53ea2 100644 --- a/.github/workflows/run-unit-tests.yml +++ b/.github/workflows/run-unit-tests.yml @@ -85,6 +85,7 @@ jobs: 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_companion_response_bounds.py -v python3 -B test/test_contact_cache.py -v - name: Verify receive recovery and pinned radio status transport diff --git a/examples/companion_radio/MyMesh.cpp b/examples/companion_radio/MyMesh.cpp index 448f9788..f3a8ccd7 100644 --- a/examples/companion_radio/MyMesh.cpp +++ b/examples/companion_radio/MyMesh.cpp @@ -1364,11 +1364,12 @@ void MyMesh::onContactResponse(const ContactInfo &contact, const uint8_t *data, #endif clearPendingReqs(); } else if (mesh::companionStatusTagMatches(pending_status, tag)) { - // Do not expose a truncated or unrelated response as repeater statistics. + // Require a complete response that also fits its host-protocol envelope. // The app parses at least 48 bytes and otherwise throws a RangeError. - if (!mesh::companionStatusResponseIsLongEnough(len)) { + if (!mesh::companionStatusResponseIsLongEnough(len) + || len > MAX_FRAME_SIZE - 4) { MESH_DEBUG_PRINTLN( - "onContactResponse(), short status response: len=%u, expected>=%u", + "onContactResponse(), invalid status response size: len=%u, expected>=%u", (unsigned)len, (unsigned)mesh::COMPANION_MIN_STATUS_RESPONSE_SIZE); clearPendingReqs(); @@ -1384,7 +1385,11 @@ void MyMesh::onContactResponse(const ContactInfo &contact, const uint8_t *data, i += (len - 4); writePendingSerialFrame(out_frame, i); clearPendingReqs(); - } else if (len > 4 && tag == pending_telemetry) { // check for matching response tag + } else if (pending_telemetry && len > 4 && tag == pending_telemetry) { // check for matching response tag + if (len > MAX_FRAME_SIZE - 4) { + clearPendingReqs(); + return; + } int i = 0; out_frame[i++] = PUSH_CODE_TELEMETRY_RESPONSE; out_frame[i++] = 0; // reserved @@ -1394,7 +1399,11 @@ void MyMesh::onContactResponse(const ContactInfo &contact, const uint8_t *data, i += (len - 4); writePendingSerialFrame(out_frame, i); clearPendingReqs(); - } else if (len > 4 && tag == pending_req) { // check for matching response tag + } else if (pending_req && len > 4 && tag == pending_req) { // check for matching response tag + if (len > MAX_FRAME_SIZE - 2) { + clearPendingReqs(); + return; + } int i = 0; out_frame[i++] = PUSH_CODE_BINARY_RESPONSE; out_frame[i++] = 0; // reserved @@ -1438,7 +1447,7 @@ bool MyMesh::onContactPathRecv(ContactInfo& contact, uint8_t* in_path, uint8_t i } void MyMesh::onControlDataRecv(mesh::Packet *packet) { - if (packet->payload_len + 4 > sizeof(out_frame)) { + if (packet->payload_len + 4 > MAX_FRAME_SIZE) { MESH_DEBUG_PRINTLN("onControlDataRecv(), payload_len too long: %d", packet->payload_len); return; } @@ -1458,7 +1467,7 @@ void MyMesh::onControlDataRecv(mesh::Packet *packet) { } void MyMesh::onRawDataRecv(mesh::Packet *packet) { - if (packet->payload_len + 4 > sizeof(out_frame)) { + if (packet->payload_len + 4 > MAX_FRAME_SIZE) { MESH_DEBUG_PRINTLN("onRawDataRecv(), payload_len too long: %d", packet->payload_len); return; } @@ -1482,7 +1491,7 @@ void MyMesh::onTraceRecv(mesh::Packet *packet, uint32_t tag, uint32_t auth_code, const bool binary_trace_match = binary_trace_pending && tag == binary_trace_tag && auth_code == binary_trace_auth; uint8_t path_sz = flags & 0x03; // NEW v1.11+ - if (12 + path_len + (path_len >> path_sz) + 1 > sizeof(out_frame)) { + if (12 + path_len + (path_len >> path_sz) + 1 > MAX_FRAME_SIZE) { MESH_DEBUG_PRINTLN("onTraceRecv(), path_len is too long: %d", (uint32_t)path_len); if (binary_trace_match) clearBinaryTraceReply(); return; @@ -6465,8 +6474,12 @@ bool MyMesh::applyAndSaveRxBoostedGain(bool enabled) { _prefs.rx_boosted_gain = enabled ? 1 : 0; if (!savePrefs()) { _prefs.rx_boosted_gain = previous_pref; - if (_radio_available) { - radio_driver.setRxBoostedGainMode(previous_pref != 0); + if (_radio_available && !radio_driver.setRxBoostedGainMode(previous_pref != 0)) { + // A receive can make the rollback busy after hardware accepted the + // candidate. Retry the durable gain along with the saved radio settings. + saved_radio_apply_pending = true; + radio_apply_retry_at = 0; + radio_apply_failures = 0; } return false; } @@ -8983,8 +8996,9 @@ void MyMesh::loop() { // A power-saving wake can enter begin() with a complete packet already // waiting. Preserve that packet, then apply the persisted radio settings // once the receive/response path is idle. - radio_driver.setRxBoostedGainMode(_prefs.rx_boosted_gain); - if (applySavedRadioParams() + if ((!radio_driver.supportsRxBoostedGainMode() + || radio_driver.setRxBoostedGainMode(_prefs.rx_boosted_gain)) + && applySavedRadioParams() && radio_driver.setTxPower(_prefs.tx_power_dbm) && (!radio_driver.supportsRxPowerSaving() || radio_driver.setRxPowerSaving(_prefs.rx_powersaving_enabled != 0, diff --git a/examples/simple_repeater/MyMesh.cpp b/examples/simple_repeater/MyMesh.cpp index 54fd7b03..8f8d124e 100644 --- a/examples/simple_repeater/MyMesh.cpp +++ b/examples/simple_repeater/MyMesh.cpp @@ -3162,7 +3162,7 @@ void __attribute__((noinline)) MyMesh::processDeferredCliCommand() { 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. + // authenticated pre-command destination/path as a fallback if it is removed. // Command permission checks still use the live ACL entry below. ClientInfo reply_client = *client; @@ -3238,9 +3238,15 @@ void __attribute__((noinline)) MyMesh::processDeferredCliCommand() { remote_cli_reply_cache.remember(deferred_cli_command.client_pub_key, deferred_cli_command.request_id, command_fingerprint, reply, arms_temp_radio); + // Route changes made by this command must apply to its acknowledgement too. + // Resolve the full authenticated key again: compaction may have moved it or + // reused its old slot. Only a removed sender needs the pre-command fallback. + const int reply_client_index = deferred_cli_command.findClientIndex(acl); + ClientInfo* reply_destination = reply_client_index >= 0 + ? acl.getClientByIdx(reply_client_index) : &reply_client; mesh::Packet* queued_reply = NULL; const bool reply_queued = sendRemoteCliReply( - &reply_client, deferred_cli_command.secret, + reply_destination, 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, diff --git a/src/helpers/ota/OtaManager.cpp b/src/helpers/ota/OtaManager.cpp index 95383cbd..038dd489 100644 --- a/src/helpers/ota/OtaManager.cpp +++ b/src/helpers/ota/OtaManager.cpp @@ -1914,14 +1914,13 @@ bool OtaManager::handleProof(const uint8_t* m, uint16_t n) { if (activePipelineSlots() == 0) { finishFlight(); requestMissing(); } return true; } - // verified -> commit the payload block, then its leaf (the present marker). A write failure here means a - // FOLDER destination's seeder link dropped mid-transfer: PAUSE (hold progress on the host, stop - // requesting, do NOT fall back to RAM/flash). The block is left uncommitted (its leaf stays 0xFF), so on - // reconnect resumeStaged() re-requests exactly it. Flash failures also pause instead of being ignored. + // Commit payload before its present marker. Preserve the selected store and + // checkpoint on failure: reconnectable stores pause, while local storage + // reports a retryable failure instead of waiting for a nonexistent link. uint8_t leaf[4]; merkle_leaf(leaf, slot.buf, blen); if (!_fetch->write(_fpoff + block * _fbs, slot.buf, blen) || !_fetch->write(_floff + block * 4, leaf, 4)) { - pauseFetchForDisconnect(); + failFetch(FETCH_ERROR_STORAGE); return true; } _have++; diff --git a/test/fixtures/ota_store_resume/test.cpp b/test/fixtures/ota_store_resume/test.cpp index 671e8492..4443b365 100644 --- a/test/fixtures/ota_store_resume/test.cpp +++ b/test/fixtures/ota_store_resume/test.cpp @@ -356,8 +356,13 @@ struct TransientLeafReadStore : OtaStoreRam<32768> { bool fail_after_commit = false; mutable bool fail_next_leaf = false; uint32_t failed_leaf = 0; + uint32_t fail_write_at = UINT32_MAX; bool canReconnect() const override { return reconnectable; } bool write(uint32_t off, const uint8_t* data, uint32_t len) override { + if (off == fail_write_at) { + fail_write_at = UINT32_MAX; + return false; + } const bool ok = OtaStoreRam<32768>::write(off, data, len); if (ok && fail_after_commit && off == 8 + MOTA_MFL && len == 4) { fail_after_commit = false; @@ -521,6 +526,43 @@ static void stale_flash_header_does_not_mask_selected_checkpoint(int selection) } } +static void storage_write_failure_is_retryable(bool leaf_write, bool reconnectable) { + auto bytes = large_container(true); + MotaManifest manifest; + assert(mota_parse(bytes.data(), bytes.size(), manifest)); + TransientLeafReadStore store; + store.reconnectable = reconnectable; + assert(store.begin(bytes.size())); + assert(store.write(0, bytes.data(), bytes.size())); + std::vector missing((manifest.block_count - 1) * 4, 0xFF); + assert(store.write(8 + MOTA_MFL + 4, missing.data(), missing.size())); + 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.resumeStagedExplicit(manifest.merkle_root, manifest.target_id)); + while (receiver.fetchState() == OtaManager::VERIFYING_STAGED) receiver.loop(); + assert(receiver.blocksHave() == 1); + store.fail_write_at = leaf_write ? 8 + MOTA_MFL + 4 + : uint32_t(manifest.payload - bytes.data()) + 1024; + deliver_block(receiver, manifest, 1); + assert(store.fail_write_at == UINT32_MAX); + assert(receiver.fetchState() == (reconnectable ? OtaManager::PAUSED : OtaManager::FAILED)); + assert(receiver.fetchError() == OtaManager::FETCH_ERROR_STORAGE); + assert(receiver.blocksHave() == 1); + if (reconnectable) assert(receiver.resumeFetchAfterReconnect()); + else assert(receiver.resumeStagedExplicit(manifest.merkle_root, manifest.target_id)); + while (receiver.fetchState() == OtaManager::VERIFYING_STAGED) receiver.loop(); + 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) deliver_block(receiver, manifest, block); + } + assert(receiver.fetchState() == OtaManager::COMPLETE); + assert(memcmp(store.data(), bytes.data(), bytes.size()) == 0); +} + int main(int argc, char** argv) { const int scenario = argc > 1 ? atoi(argv[1]) : 0; if (scenario < 5) resume_checks_file_envelope(scenario); @@ -535,5 +577,6 @@ int main(int argc, char** argv) { 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 if (scenario < 50) storage_write_failure_is_retryable((scenario & 1) != 0, scenario >= 48); else assert(false); } diff --git a/test/fixtures/remote_cli_command_identity/test.cpp b/test/fixtures/remote_cli_command_identity/test.cpp index 553acbdf..4d7c932c 100644 --- a/test/fixtures/remote_cli_command_identity/test.cpp +++ b/test/fixtures/remote_cli_command_identity/test.cpp @@ -54,6 +54,8 @@ 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; + uint8_t out_path_len = 1, alt_path_len = 0xff; + uint8_t out_path[MAX_PATH_SIZE] = {}, alt_path[MAX_PATH_SIZE] = {}; bool admin = true; bool region_mgr = false; bool isAdmin() const { return admin; } @@ -130,6 +132,7 @@ public: std::vector replies; std::vector> recipients; std::vector> secrets; + std::vector reply_destinations; MyMesh() { now_millis = 100;mesh::console.records.clear(); acl.clients[0].id.pub_key[0] = 0x12; acl.clients[1].id.pub_key[0] = 0x77; @@ -152,6 +155,7 @@ public: memcpy(key.data(), client->id.pub_key, key.size()); memcpy(shared.data(), secret, shared.size()); recipients.push_back(key);secrets.push_back(shared); + reply_destinations.push_back(*client); replies.emplace_back(reply); return true; } void scheduleNormalRadio() {} @@ -165,9 +169,21 @@ public: void clearDeferredCliCommand(); bool completeHostCliRequest(const char*); bool handleHostCliSerialReply(const char*, char*); - void handleCommand(uint32_t, ClientInfo*, char* command, char* reply, int, uint8_t) { + void handleCommand(uint32_t, ClientInfo* sender, char* command, char* reply, int, uint8_t) { ++executions; #include "normalization.inc" + if (!fail_command) { + // Accepted route changes mirror handleClientPathCommand's live ACL edits. + if (strcmp(command, "set outpath flood") == 0) sender->out_path_len = 0xfe; + else if (strcmp(command, "set outpath direct") == 0) sender->out_path_len = 0; + else if (strcmp(command, "set outpath ab") == 0) { + sender->out_path_len = 1; + sender->out_path[0] = 0xab; + } else if (strcmp(command, "set altpath cd") == 0) { + sender->alt_path_len = 1; + sender->alt_path[0] = 0xcd; + } + } if (delete_during_command >= 0) acl.remove(delete_during_command); if (!fail_command) { if (accepted_mutation == Mutation::Primary) ++primary_radio_mutation_generation; @@ -283,10 +299,21 @@ int main() { for (const int deleted : {0, 1}) { MyMesh value; value.delete_during_command = deleted; + value.acl.clients[1].out_path[0] = 0x31; + value.acl.clients[1].alt_path_len = 1; + value.acl.clients[1].alt_path[0] = 0x32; + // A reused slot with the same prefix must not supply the final reply route. + value.acl.clients[2].id.pub_key[0] = 0x77; + value.acl.clients[2].id.pub_key[PUB_KEY_SIZE - 1] = 1; + value.acl.clients[2].out_path[0] = 0x99; 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); + assert(value.recipients[0][PUB_KEY_SIZE - 1] == 0); + assert(value.reply_destinations[0].out_path[0] == 0x31); + assert(value.reply_destinations[0].alt_path_len == 1); + assert(value.reply_destinations[0].alt_path[0] == 0x32); 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)); @@ -295,6 +322,29 @@ int main() { assert(value.executions == 1 && value.recipients.back()[0] == 0x77); } } + // Successful path updates affect the first acknowledgement as well as any + // cached retry, including when the original sender moves to another slot. + for (const char* command : {"set outpath flood", "set outpath direct", + "set outpath ab", "set altpath cd"}) { + for (const int deleted : {-1, 0}) { + MyMesh value; + value.delete_during_command = deleted; + value.acl.clients[1].out_path[0] = 0x31; + value.receive(command, 101, 99, 1); + const int index = deleted == 0 ? 0 : 1; + assert(value.executions == 1 && value.reply_destinations.size() == 1); + value.receive(command, 102, 99, index); + assert(value.executions == 1 && value.reply_destinations.size() == 2); + const ClientInfo& live = value.acl.clients[index]; + for (const ClientInfo& sent : value.reply_destinations) { + assert(memcmp(sent.id.pub_key, live.id.pub_key, PUB_KEY_SIZE) == 0); + assert(sent.out_path_len == live.out_path_len); + assert(memcmp(sent.out_path, live.out_path, MAX_PATH_SIZE) == 0); + assert(sent.alt_path_len == live.alt_path_len); + assert(memcmp(sent.alt_path, live.alt_path, MAX_PATH_SIZE) == 0); + } + } + } for (const bool host_completion : {false, true}) { for (const int change : {0, 1, 2, 3}) { MyMesh value; diff --git a/test/test_companion_prefs_transactions.py b/test/test_companion_prefs_transactions.py index 299f064a..d465a2d3 100644 --- a/test/test_companion_prefs_transactions.py +++ b/test/test_companion_prefs_transactions.py @@ -66,10 +66,11 @@ struct Sensors { } sensors; struct Driver { bool supported=true,rxps=false; + bool gain_supported=true,boosted_gain=false; uint32_t rx=3000,sleep=4000; - unsigned calls=0; - std::deque results; - bool supportsRxBoostedGainMode() const { return true; } + unsigned calls=0,gain_calls=0; + std::deque results,gain_results; + bool supportsRxBoostedGainMode() const { return gain_supported; } bool supportsRxPowerSaving() const { return supported; } bool setRxPowerSaving(bool enable,uint32_t r,uint32_t s) { ++calls;bool ok=true; @@ -77,7 +78,12 @@ struct Driver { if(ok){rxps=enable;rx=r;sleep=s;} return ok; } - void setRxBoostedGainMode(uint8_t) {} + bool setRxBoostedGainMode(bool enabled) { + ++gain_calls;bool ok=true; + if(!gain_results.empty()){ok=gain_results.front();gain_results.pop_front();} + if(ok)boosted_gain=enabled; + return ok; + } bool setTxPower(int8_t) { return true; } } radio_driver; @PARSERS@ @@ -100,7 +106,7 @@ struct MyMesh { bool saveAdvertLocation(double,double); bool applyAndSaveRxPowerSaving(const char*,char*); bool applyAndSavePowerSaving(const char*,char*); - bool applyAndSaveRxBoostedGain(bool) { return true; } + bool applyAndSaveRxBoostedGain(bool); 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; @@ -191,7 +197,7 @@ int main() { } } const char* web[][2]={{"name","new"},{"lat","30"},{"lon","40"},{"radio","920,250,9,6"}, - {"af","3"},{"rxdelay","4"},{"repeat","on"}}; + {"af","3"},{"rxdelay","4"},{"repeat","on"},{"radio.rxgain","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; @@ -246,6 +252,59 @@ int main() { assert(!m.applyAndSaveRxPowerSaving("10000 20000",reply));++checks; assert(m.saves==0);m.expectOriginal(); } + // Boosted gain uses the actual setter and the shared saved-radio recovery. + // Cover both directions, rejected saves, busy restoration, and unsupported radios. + for(bool initial:{false,true})for(bool save_ok:{false,true}) { + MyMesh m;m.storage_accepts=save_ok; + m._prefs.rx_boosted_gain=m.durable.rx_boosted_gain=initial; + radio_driver.boosted_gain=initial; + assert(m.applyAndSaveRxBoostedGain(!initial)==save_ok);++checks; + const bool expected=save_ok?!initial:initial; + assert(m._prefs.rx_boosted_gain==expected && m.durable.rx_boosted_gain==expected); + assert(radio_driver.boosted_gain==expected && !m.saved_radio_apply_pending); + assert(m.saves==1 && radio_driver.gain_calls==(save_ok?1U:2U)); + m.storage_accepts=true;assert(m.saveAdvertName("later")); + assert(m.durable.rx_boosted_gain==expected); + } + for(bool initial:{false,true}) { + MyMesh m;m.storage_accepts=false; + m._prefs.rx_boosted_gain=m.durable.rx_boosted_gain=initial; + radio_driver.boosted_gain=initial; + radio_driver.gain_results={true,false,false,true}; + m.radio_apply_retry_at=500;m.radio_apply_failures=3; + assert(!m.applyAndSaveRxBoostedGain(!initial));++checks; + m.expectOriginal(); + assert(m.saved_radio_apply_pending && radio_driver.boosted_gain!=initial); + assert(m.radio_apply_retry_at==0 && m.radio_apply_failures==0); + m.recover(); + assert(m.saved_radio_apply_pending && radio_driver.boosted_gain!=initial); + assert(m.radio_apply_retry_at!=0 && m.radio_apply_failures!=0); + m.recover(); + assert(!m.saved_radio_apply_pending && radio_driver.boosted_gain==initial); + assert(m.radio_apply_retry_at==0 && m.radio_apply_failures==0); + assert(m.saves==1 && radio_driver.gain_calls==4); + m.storage_accepts=true;assert(m.saveAdvertName("later")); + assert(m.durable.rx_boosted_gain==initial); + } + { + MyMesh m;radio_driver.gain_results={false}; + assert(!m.applyAndSaveRxBoostedGain(true));++checks; + assert(m.saves==0 && !radio_driver.boosted_gain && !m.saved_radio_apply_pending); + m.expectOriginal(); + } + { + MyMesh m;radio_driver.gain_supported=false;radio_driver.gain_results={false}; + assert(!m.applyAndSaveRxBoostedGain(true));++checks; + assert(m.saves==0 && radio_driver.gain_calls==0);m.expectOriginal(); + m.saved_radio_apply_pending=true;m.recover(); + assert(!m.saved_radio_apply_pending && radio_driver.gain_calls==0); + } + for(bool save_ok:{false,true}) { + MyMesh m;m._radio_available=false;m.storage_accepts=save_ok; + assert(m.applyAndSaveRxBoostedGain(true)==save_ok);++checks; + assert(m.saves==1 && radio_driver.gain_calls==0 && !m.saved_radio_apply_pending); + assert(m._prefs.rx_boosted_gain==save_ok && m.durable.rx_boosted_gain==save_ok); + } 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; @@ -296,6 +355,7 @@ class CompanionPrefsTransactionTests(unittest.TestCase): '@METHODS@': '\n'.join(extract_braced(text,sig) for sig in ( 'bool MyMesh::saveAdvertName(', 'bool MyMesh::saveAdvertLocation(', 'bool MyMesh::applyAndSaveRxPowerSaving(', + 'bool MyMesh::applyAndSaveRxBoostedGain(', 'bool MyMesh::applyAndSavePowerSaving(', )), '@FRAMES@': ' else '.join(extract_braced(text, f'if (cmd_frame[0] == {cmd})') for cmd in frames), diff --git a/test/test_companion_radio_settings_transaction.py b/test/test_companion_radio_settings_transaction.py index 7feb7132..c1ada833 100644 --- a/test/test_companion_radio_settings_transaction.py +++ b/test/test_companion_radio_settings_transaction.py @@ -60,7 +60,8 @@ struct Driver { if (ok) power=requested; return ok; } - void setRxBoostedGainMode(bool) {} + bool supportsRxBoostedGainMode() const { return false; } + bool setRxBoostedGainMode(bool) { return true; } bool supportsRxPowerSaving() const { return false; } bool setRxPowerSaving(bool,uint32_t,uint32_t) { return true; } } radio_driver; diff --git a/test/test_companion_response_bounds.py b/test/test_companion_response_bounds.py new file mode 100644 index 00000000..276e9e4b --- /dev/null +++ b/test/test_companion_response_bounds.py @@ -0,0 +1,143 @@ +"""Check production Companion response envelope limits without radio hardware.""" +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] + +HARNESS = r''' +#include +#include +#include +#include +#include +#include +#define COMPANION_FEATURE_TEXT_TERMINAL 0 +#define MESH_DEBUG_PRINTLN(...) ((void)0) +#define RESP_SERVER_LOGIN_OK 0 +@CODES@ +@FRAME_LIMIT@ +struct ContactInfo { struct { uint8_t pub_key[32] = {1}; } id; }; +namespace mesh { +struct Packet { + uint8_t payload[256] = {}, payload_len = 0, path_len = 0; + float getSNR() const { return 0; } + int getRSSI() const { return 0; } +}; +} +struct Serial { + std::vector> frames; + bool isConnected() const { return true; } + void writeFrame(const uint8_t* data, size_t n) { + assert(n <= MAX_FRAME_SIZE); + frames.emplace_back(data, data+n); + } + void writeFrameToRoute(Serial*, const uint8_t* data, size_t n) { writeFrame(data,n); } +}; +struct MyMesh { + uint32_t pending_login=0, pending_status=0, pending_telemetry=0, pending_req=0; + uint8_t before[16], out_frame[MAX_FRAME_SIZE+1], after[16]; + Serial serial; + Serial* _serial = &serial; + unsigned clears = 0; + bool binary_trace_pending = true; + uint32_t binary_trace_tag = 17, binary_trace_auth = 23; + Serial* binary_trace_reply_route = &serial; + MyMesh() { + memset(before, 0xA5, sizeof(before)); memset(after, 0xA5, sizeof(after)); + memset(out_frame, 0x5A, sizeof(out_frame)); + } + void clearPendingReqs() { ++clears;pending_login=pending_status=pending_telemetry=pending_req=0; } + void startConnection(const ContactInfo&, uint16_t) {} + void writePendingSerialFrame(const uint8_t* p, size_t n) { serial.writeFrame(p,n); } + void onContactResponse(const ContactInfo&, const uint8_t*, uint8_t); + void onControlDataRecv(mesh::Packet*); + void onRawDataRecv(mesh::Packet*); + void onTraceRecv(mesh::Packet*,uint32_t,uint32_t,uint8_t,const uint8_t*,const uint8_t*,uint8_t); + void clearBinaryTraceReply() { binary_trace_pending = false; } + void checkGuards() const { + for (uint8_t value:before) assert(value==0xA5); + for (uint8_t value:after) assert(value==0xA5); + assert(out_frame[MAX_FRAME_SIZE]==0x5A); + } +}; +@METHODS@ +int main() { + ContactInfo contact; + for (unsigned kind=0; kind<3; ++kind) for (unsigned n=0; n<256; ++n) { + MyMesh value; + uint32_t tag=17; + uint8_t data[256]={};memcpy(data,&tag,4); + if (kind==0) value.pending_status=tag; + if (kind==1) value.pending_telemetry=tag; + if (kind==2) value.pending_req=tag; + value.onContactResponse(contact,data,n); + const unsigned minimum=kind==0 ? mesh::COMPANION_MIN_STATUS_RESPONSE_SIZE : 5; + const unsigned overhead=kind==2 ? 2 : 4; + const bool valid=n>=minimum && n+overhead<=MAX_FRAME_SIZE; + assert(value.serial.frames.size()==(valid ? 1U : 0U)); + if (valid) assert(value.serial.frames[0].size()==n+overhead); + value.checkGuards(); + } + for (unsigned n=0;n<256;++n) { + MyMesh idle;uint8_t data[256]={}; + idle.onContactResponse(contact,data,n); + assert(idle.serial.frames.empty() && idle.clears==0);idle.checkGuards(); + for (bool control:{false,true}) { + MyMesh value;mesh::Packet packet;packet.payload_len=n; + if (control) value.onControlDataRecv(&packet);else value.onRawDataRecv(&packet); + assert(value.serial.frames.size()==(n+4<=MAX_FRAME_SIZE ? 1U : 0U)); + value.checkGuards(); + } + } + for (bool modern:{false,true}) { + MyMesh value;memcpy(&value.pending_login,contact.id.pub_key,4); + uint8_t data[13]={};if (!modern) memcpy(data+4,"OK",2); + value.onContactResponse(contact,data,modern ? 13 : 6); + assert(value.serial.frames.size()==1 && value.clears==1);value.checkGuards(); + } + for (unsigned n=0;n<256;++n) for (uint8_t flags=0;flags<4;++flags) { + MyMesh value;mesh::Packet packet;uint8_t data[256]={}; + value.onTraceRecv(&packet,17,23,flags,data,data,n); + const unsigned framed=13+n+(n>>flags); + assert(value.serial.frames.size()==(framed<=MAX_FRAME_SIZE ? 1U : 0U)); + if (!value.serial.frames.empty()) assert(value.serial.frames[0].size()==framed); + assert(!value.binary_trace_pending);value.checkGuards(); + } + MyMesh empty;empty.onContactResponse(contact,nullptr,255);empty.checkGuards(); +} +''' + + +class CompanionResponseBoundsTest(unittest.TestCase): + def test_response_envelopes(self): + source = (ROOT / 'examples/companion_radio/MyMesh.cpp').read_text(encoding='utf-8') + codes = '\n'.join(re.findall(r'^#define PUSH_CODE_\w+\s+0x[0-9A-Fa-f]+', source, re.M)) + interface = (ROOT / 'src/helpers/BaseSerialInterface.h').read_text(encoding='utf-8') + frame_limit = re.search(r'^#define MAX_FRAME_SIZE\s+\d+', interface, re.M).group(0) + methods = '\n'.join(extract_braced(source, signature) for signature in ( + 'void MyMesh::onContactResponse(', 'void MyMesh::onControlDataRecv(', + 'void MyMesh::onRawDataRecv(', 'void MyMesh::onTraceRecv(', + )) + with tempfile.TemporaryDirectory(prefix='companion-response-') as directory: + work = Path(directory) + cpp = work / 'test.cpp' + cpp.write_text(HARNESS.replace('@CODES@',codes).replace('@METHODS@',methods) + .replace('@FRAME_LIMIT@',frame_limit), encoding='utf-8') + flags = ['-fsanitize=address,undefined','-fno-sanitize-recover=all','-fno-pie','-no-pie'] if sys.platform.startswith('linux') else [] + binary = work / 'test.exe' + compiled = subprocess.run([os.environ.get('CXX','g++'),'-std=c++17','-Wall','-Wextra','-Werror', + *flags,f'-I{ROOT / "src"}',str(cpp),'-o',str(binary)], capture_output=True,text=True,timeout=60) + self.assertEqual(compiled.returncode,0,compiled.stderr) + checked = subprocess.run([str(binary)],capture_output=True,text=True,timeout=10) + self.assertEqual(checked.returncode,0,checked.stderr) + + +if __name__=='__main__': + unittest.main() diff --git a/test/test_ota_store_resume.py b/test/test_ota_store_resume.py index 81b67fab..5f4357c0 100644 --- a/test/test_ota_store_resume.py +++ b/test/test_ota_store_resume.py @@ -32,7 +32,7 @@ class OtaStoreResumeTest(unittest.TestCase): 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(46): + for scenario in range(50): with self.subTest(scenario=scenario): subprocess.run([str(binary), str(scenario)], check=True, timeout=10)