diff --git a/examples/simple_repeater/MyMesh.cpp b/examples/simple_repeater/MyMesh.cpp index ca8595f0..4da6ecd9 100644 --- a/examples/simple_repeater/MyMesh.cpp +++ b/examples/simple_repeater/MyMesh.cpp @@ -3411,6 +3411,7 @@ MyMesh::MyMesh(mesh::MainBoard &board, mesh::Radio &radio, mesh::MillisecondCloc scheduled_temp_radio_end_check_at = 0; scheduled_temp_radio_end_check_final = false; scheduled_radio_retry_at = 0; + scheduled_radio_save_retry_at = 0; scheduled_radio_retry_failures = 0; memset(scheduled_radio_settings, 0, sizeof(scheduled_radio_settings)); _logging = false; @@ -4133,9 +4134,11 @@ void MyMesh::refreshScheduledRadioState() { next_scheduled_radio_time = 0; scheduled_temp_radio_started = false; scheduled_temp_radio_end_time = 0; + bool has_permanent_schedule = false; for (int i = 0; i < MAX_SCHEDULED_RADIO_SETTINGS; i++) { const ScheduledRadioSetting& setting = scheduled_radio_settings[i]; if (!setting.active) continue; + if (!setting.temporary) has_permanent_schedule = true; uint32_t deadline = setting.start_time; if (setting.temporary && setting.started) { @@ -4152,17 +4155,23 @@ void MyMesh::refreshScheduledRadioState() { const uint32_t now = (next_scheduled_radio_time != 0 || scheduled_temp_radio_end_time != 0) ? getRTCClock()->getCurrentTime() : 0; + if (!has_permanent_schedule) scheduled_radio_save_retry_at = 0; if (next_scheduled_radio_time != 0) { - uint32_t delay_ms = 0; - if (next_scheduled_radio_time > now) { - uint32_t delay_secs = next_scheduled_radio_time - now; - // millis timers are only unambiguous for half of their rollover range. - // A minute checkpoint handles RTC corrections without per-loop RTC reads - // or table scans, while bounding a forward clock-sync delay to one minute. - if (delay_secs > SCHEDULED_RADIO_CLOCK_CHECKPOINT_SECS) { - delay_secs = SCHEDULED_RADIO_CLOCK_CHECKPOINT_SECS; - } - delay_ms = delay_secs * 1000UL; + // Storage retries delay only permanent saves. Cache the first actionable + // deadline so a failed save neither spins the main loop nor blocks a + // temporary window. Keep the minute checkpoints for RTC corrections. + uint32_t delay_ms = SCHEDULED_RADIO_CLOCK_CHECKPOINT_SECS * 1000UL; + const int32_t save_remaining_ms = (int32_t)(scheduled_radio_save_retry_at - millis()); + const uint32_t save_wait_ms = scheduled_radio_save_retry_at && save_remaining_ms > 0 + ? (uint32_t)save_remaining_ms : 0; + for (const auto& setting : scheduled_radio_settings) { + if (!setting.active) continue; + const uint32_t deadline = setting.temporary && setting.started ? setting.end_time : setting.start_time; + uint32_t seconds = deadline > now ? deadline - now : 0; + if (seconds > SCHEDULED_RADIO_CLOCK_CHECKPOINT_SECS) seconds = SCHEDULED_RADIO_CLOCK_CHECKPOINT_SECS; + uint32_t candidate_ms = seconds * 1000UL; + if (!setting.temporary && save_wait_ms > candidate_ms) candidate_ms = save_wait_ms; + if (candidate_ms < delay_ms) delay_ms = candidate_ms; } next_scheduled_radio_check_at = futureMillis(delay_ms); } else { @@ -4293,6 +4302,7 @@ void MyMesh::clearScheduledRadioSetting(int idx, bool restore_if_started) { && scheduled_radio_settings[idx].active && scheduled_radio_settings[idx].temporary && scheduled_radio_settings[idx].started; + if (!scheduled_radio_settings[idx].temporary) scheduled_radio_save_retry_at = 0; scheduled_radio_settings[idx].active = false; scheduled_radio_settings[idx].started = false; scheduled_radio_settings[idx].hard_end_uptime_millis = 0; @@ -4629,7 +4639,7 @@ void MyMesh::processScheduledRadioSettings() { } } - while (schedule_due) { + while (schedule_due && (!scheduled_radio_save_retry_at || millisHasNowPassed(scheduled_radio_save_retry_at))) { int due_idx = -1; for (int i = 0; i < MAX_SCHEDULED_RADIO_SETTINGS; i++) { const ScheduledRadioSetting& setting = scheduled_radio_settings[i]; @@ -4647,9 +4657,11 @@ void MyMesh::processScheduledRadioSettings() { ScheduledRadioSetting& setting = scheduled_radio_settings[due_idx]; if (!_cli.radioProfiles().savePrimaryPreamble(setting.preamble)) { - scheduled_radio_retry_at = futureMillis(60000); + scheduled_radio_save_retry_at = futureMillis(60000); + if (!scheduled_radio_save_retry_at) scheduled_radio_save_retry_at = 1; break; } + scheduled_radio_save_retry_at = 0; _prefs.freq = setting.freq; _prefs.bw = setting.bw; _prefs.sf = setting.sf; diff --git a/examples/simple_repeater/MyMesh.h b/examples/simple_repeater/MyMesh.h index 2d0ce2e4..f6f30bf8 100644 --- a/examples/simple_repeater/MyMesh.h +++ b/examples/simple_repeater/MyMesh.h @@ -554,6 +554,7 @@ class MyMesh : public mesh::Mesh, public CommonCLICallbacks unsigned long scheduled_temp_radio_end_check_at; bool scheduled_temp_radio_end_check_final; unsigned long scheduled_radio_retry_at; + unsigned long scheduled_radio_save_retry_at; uint8_t scheduled_radio_retry_failures; ScheduledRadioSetting scheduled_radio_settings[MAX_SCHEDULED_RADIO_SETTINGS]; int matching_peer_indexes[MAX_CLIENTS]; diff --git a/src/helpers/RadioProfileCLI.cpp b/src/helpers/RadioProfileCLI.cpp index 1dafd023..b73acadc 100644 --- a/src/helpers/RadioProfileCLI.cpp +++ b/src/helpers/RadioProfileCLI.cpp @@ -47,6 +47,10 @@ bool parsePreamble(const char* s, uint16_t& result) { bool validReplySetting(uint8_t value) { return (value >= 0xb0 && value <= 0xb4) || value == 0xba || value == 0xbb; } +uint32_t remainingMillis(uint32_t end, uint32_t now) { + const int32_t remaining = (int32_t)(end - now); + return remaining > 0 ? (uint32_t)remaining : 0; +} } bool RadioProfileCLI::readImage(const char* path, uint8_t* bytes, size_t size) { @@ -194,29 +198,36 @@ void RadioProfileCLI::loop() { else temp_remaining_ms_ -= elapsed; } const uint32_t epoch = rtc_ ? rtc_->getCurrentTime() : 0; + // Expire every old lease before starting its successor. Slot insertion order + // must not let an expired entry clear a session started earlier in this loop. for (auto& s : schedules_) { - if (!s.active) continue; - if (s.temporary) { - if (elapsed >= s.remaining_ms || epoch >= s.end) { - if (s.started) { temp_active_ = false; temp_remaining_ms_ = 0; } - s = {}; continue; - } - s.remaining_ms -= elapsed; + if (s.active && s.temporary && (!remainingMillis(s.end_ms, now) || epoch >= s.end)) { + if (s.started) { temp_active_ = false; temp_remaining_ms_ = 0; } + s = {}; } - if (s.started || epoch < s.start) continue; - if (s.temporary) { - if (temp_active_ || temp_pending_) continue; - temporary_ = s.config; - temp_remaining_ms_ = s.remaining_ms; - const uint32_t epoch_left = (s.end - epoch) * 1000UL; - if (temp_remaining_ms_ > epoch_left) temp_remaining_ms_ = epoch_left; - temp_active_ = true; s.started = true; - } else if (save(s.config, primary_preamble_, cross_)) { - saved_ = s.config; s = {}; - } else { - // Avoid a flash write on every main-loop iteration after a storage fault. - s.start = epoch <= UINT32_MAX - 60 ? epoch + 60 : UINT32_MAX; + } + schedule_retry_ms_ = elapsed >= schedule_retry_ms_ ? 0 : schedule_retry_ms_ - elapsed; + while (!schedule_retry_ms_) { + Schedule* next = nullptr; + for (auto& s : schedules_) { + if (s.active && !s.temporary && epoch >= s.start && (!next || s.start < next->start)) next = &s; } + if (!next) break; + if (!save(next->config, primary_preamble_, cross_)) { + // Keep the requested time and order intact. A failed earlier entry must + // not be retried after a later one and overwrite the newer settings. + schedule_retry_ms_ = 60000; + break; + } + saved_ = next->config; *next = {}; + } + for (auto& s : schedules_) { + if (!s.active || !s.temporary || s.started || epoch < s.start || temp_active_ || temp_pending_) continue; + temporary_ = s.config; + temp_remaining_ms_ = remainingMillis(s.end_ms, now); + const uint32_t epoch_left = (s.end - epoch) * 1000UL; + if (temp_remaining_ms_ > epoch_left) temp_remaining_ms_ = epoch_left; + temp_active_ = true; s.started = true; } publish(); } @@ -408,7 +419,7 @@ bool RadioProfileCLI::handle(const char* command, char* reply, size_t capacity) if (base && !save({}, primary_preamble_, cross_)) { snprintf(reply, capacity, "Error: settings could not be saved"); return true; } - if (base) saved_ = {}; + 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 = {}; publish_pending_ = true; publish_after_ms_ = millis() + 2000; @@ -422,6 +433,7 @@ bool RadioProfileCLI::handle(const char* command, char* reply, size_t capacity) } const unsigned first = scheduled_temp ? 4 : 0; if (verb == Delete) { + if (!scheduled_temp) schedule_retry_ms_ = 0; for (unsigned i = 0; i < 4; ++i) if (!index || index == i + 1) { if (schedules_[first+i].started) { temp_active_ = false; temp_remaining_ms_ = 0; @@ -442,7 +454,7 @@ bool RadioProfileCLI::handle(const char* command, char* reply, size_t capacity) const auto& s = schedules_[first+index-1]; if (!s.active) snprintf(reply, capacity, "> off"); else { - formatConfig(reply, capacity, s.config, s.temporary, s.remaining_ms); + formatConfig(reply, capacity, s.config, s.temporary, remainingMillis(s.end_ms, millis())); const size_t used = strlen(reply); if (used < capacity) snprintf(reply+used, capacity-used, "; @%lu-%lu", (unsigned long)s.start, (unsigned long)s.end); } @@ -495,7 +507,7 @@ bool RadioProfileCLI::handle(const char* command, char* reply, size_t capacity) for (unsigned i = 0; i < 4; ++i) if (!schedules_[first+i].active) { auto& s = schedules_[first+i]; s = {}; s.config = config; s.start = start; s.end = end; s.temporary = scheduled_temp; - s.remaining_ms = scheduled_temp ? (end-epoch)*1000UL : 0; s.active = true; + s.end_ms = scheduled_temp ? millis() + (end-epoch)*1000UL : 0; s.active = true; snprintf(reply, capacity, "OK - %s %u queued, preamble=%u", key, i+1, preview.preamble(1, 32)); appendChirpWarning(reply, capacity, preview); return true; } diff --git a/src/helpers/RadioProfileCLI.h b/src/helpers/RadioProfileCLI.h index 321b968f..df629943 100644 --- a/src/helpers/RadioProfileCLI.h +++ b/src/helpers/RadioProfileCLI.h @@ -13,7 +13,7 @@ class RadioProfileCLI { struct Schedule { RadioProfileConfig config; uint32_t start = 0, end = 0; - uint32_t remaining_ms = 0; // monotonic upper bound, including start delay + uint32_t end_ms = 0; // monotonic deadline anchored when the command is accepted bool active = false, temporary = false, started = false; }; private: @@ -33,6 +33,7 @@ class RadioProfileCLI { uint32_t pending_duration_ms_ = 0; uint32_t publish_after_ms_ = 0; uint32_t last_ms_ = 0; + uint32_t schedule_retry_ms_ = 0; bool temp_pending_ = false, temp_active_ = false; bool hold_ = false; bool publish_pending_ = false; diff --git a/test/fixtures/radio_profiles/cli_test.cpp b/test/fixtures/radio_profiles/cli_test.cpp index d3cf5cb3..73b55103 100644 --- a/test/fixtures/radio_profiles/cli_test.cpp +++ b/test/fixtures/radio_profiles/cli_test.cpp @@ -3,6 +3,7 @@ #include #include #include +#include struct Clock : mesh::RTCClock { uint32_t epoch = 1700000000; @@ -42,7 +43,154 @@ struct Fixture { } void advance(uint32_t ms, bool epoch=true) { g_mock_millis+=ms; if(epoch)clock.epoch+=ms/1000; cli.loop(); } }; -int main() { +static void permanentScheduleOrder() { + for (bool fail_first_save : {false, true}) { + Fixture f; + char command[160]; + snprintf(command, sizeof(command), "set radioat2 912.5,500,8,5,rx,%lu,80", + (unsigned long)(f.clock.epoch + 120)); + f.cmd(command); + snprintf(command, sizeof(command), "set radioat2 910.5,500,8,5,rx,%lu,80", + (unsigned long)(f.clock.epoch + 60)); + f.cmd(command); + f.fs.fail_write = fail_first_save; + f.advance(120000); // both are due after sleep or a forward clock correction + if (fail_first_save) { + f.fs.fail_write = false; + f.advance(60000); + } + assert(f.radio.p.secondary.params.freq == 912.5f); + Radio reboot; mesh::RadioProfileCLI restored; + restored.begin(&f.fs, &reboot, &f.clock); + assert(reboot.p.secondary.params.freq == 912.5f); + assert(!strcmp(f.cmd("get radioat2 all"), "> slots: ")); + } +} + +static void adjacentTemporarySchedules() { + for (bool later_first : {false, true}) { + Fixture f; + const uint32_t epoch = f.clock.epoch; + for (unsigned i = 0; i < 2; ++i) { + const bool later = later_first ? i == 0 : i == 1; + char command[160]; + snprintf(command, sizeof(command), "set tempradioat2 %.1f,500,8,5,rxtx,%lu,%lu,80", + later ? 912.5 : 910.5, (unsigned long)(epoch + (later ? 60 : 1)), + (unsigned long)(epoch + (later ? 120 : 60))); + f.cmd(command); + } + f.advance(1000); + assert(f.radio.p.secondary_temporary && f.radio.p.secondary.params.freq == 910.5f); + f.advance(59000); + assert(f.radio.p.secondary_temporary && f.radio.p.secondary.params.freq == 912.5f); + f.advance(59000); + assert(f.radio.p.secondary_temporary && f.radio.p.secondary.params.freq == 912.5f); + f.advance(1000); + assert(!f.radio.p.enabled()); + } +} + +static void scheduledLeaseStartsAtCommandTime() { + for (bool rollover : {false, true}) { + Fixture f; + if (rollover) { g_mock_millis = UINT32_MAX - 30000; f.cli.loop(); } + g_mock_millis += 10000; f.clock.epoch += 10; // command arrives between loop calls + char command[160]; + snprintf(command, sizeof(command), "set tempradioat2 910.5,500,8,5,rx,%lu,%lu,80", + (unsigned long)(f.clock.epoch + 1), (unsigned long)(f.clock.epoch + 60)); + f.cmd(command); + f.advance(1000); + assert(f.radio.p.secondary_temporary); + f.advance(49000); + assert(f.radio.p.secondary_temporary); // idle time before the command is not charged to it + f.advance(9000); + assert(f.radio.p.secondary_temporary); + f.advance(1000); + assert(!f.radio.p.enabled()); + } +} + +static void schedulePermutations() { + int order[] = {0, 1, 2, 3}; + do { + for (bool temporary : {false, true}) for (bool late : {false, true}) { + Fixture f; + const uint32_t epoch = f.clock.epoch; + for (int entry : order) { + char command[160]; + if (temporary) { + snprintf(command, sizeof(command), "set tempradioat2 %.1f,500,8,5,rx,%lu,%lu,80", + 910.5 + entry, (unsigned long)(epoch + 1 + entry * 60), + (unsigned long)(epoch + 61 + entry * 60)); + } else { + snprintf(command, sizeof(command), "set radioat2 %.1f,500,8,5,rx,%lu,80", + 910.5 + entry, (unsigned long)(epoch + (entry + 1) * 60)); + } + f.cmd(command); + } + if (late) { + f.advance(temporary ? 181000 : 240000); + assert(f.radio.p.secondary.params.freq == 913.5f); + } else { + for (int entry = 0; entry < 4; ++entry) { + f.advance(temporary && entry == 0 ? 1000 : 60000); + assert(f.radio.p.secondary.params.freq == 910.5f + entry); + } + } + assert(f.radio.p.secondary_temporary == temporary); + if (temporary) { f.advance(60000); assert(!f.radio.p.enabled()); } + } + } while (std::next_permutation(order, order + 4)); +} + +static void storageBackoffDoesNotDelayTemporaryWindows() { + Fixture f; + f.cmd("set radio2 910.5,500,8,5,rx,80"); f.advance(2000); + char command[160]; + snprintf(command, sizeof(command), "set radioat2 912.5,500,8,5,rx,%lu,80", (unsigned long)(f.clock.epoch + 2)); + f.cmd(command); + snprintf(command, sizeof(command), "set tempradioat2 911.5,500,8,5,rx,%lu,%lu,80", + (unsigned long)(f.clock.epoch + 30), (unsigned long)(f.clock.epoch + 90)); + f.cmd(command); + f.fs.fail_write = true; + f.advance(2000); + f.advance(28000); + assert(f.radio.p.secondary_temporary && f.radio.p.secondary.params.freq == 911.5f); + f.advance(60000); + assert(!f.radio.p.secondary_temporary && f.radio.p.secondary.params.freq == 910.5f); + f.fs.fail_write = false; + f.advance(60000); + assert(f.radio.p.secondary.params.freq == 912.5f); +} + +static void scheduledSaveFailureKeepsOriginalTimes() { + Fixture f; + char command[160]; + const uint32_t start = f.clock.epoch + 60; + snprintf(command, sizeof(command), "set radioat2 910.5,500,8,5,rx,%lu,80", (unsigned long)start); + f.cmd(command); + f.fs.fail_write = true; + f.advance(60000); + char timestamp[32]; snprintf(timestamp, sizeof(timestamp), "@%lu", (unsigned long)start); + assert(strstr(f.cmd("get radioat2 all"), timestamp)); + f.fs.fail_write = false; + f.advance(59000); + assert(!f.radio.p.enabled()); + f.advance(1000); + assert(f.radio.p.secondary.params.freq == 910.5f); +} + +int main(int argc, char** argv) { + if (argc == 2) { + if (!strcmp(argv[1], "permanent_order")) permanentScheduleOrder(); + else if (!strcmp(argv[1], "temporary_boundary")) adjacentTemporarySchedules(); + else if (!strcmp(argv[1], "lease_creation")) scheduledLeaseStartsAtCommandTime(); + else if (!strcmp(argv[1], "save_failure")) scheduledSaveFailureKeepsOriginalTimes(); + else if (!strcmp(argv[1], "schedule_permutations")) schedulePermutations(); + else if (!strcmp(argv[1], "storage_isolation")) storageBackoffDoesNotDelayTemporaryWindows(); + else return 2; + return 0; + } for (bool damaged_primary : {false, true}) { Fixture f(true); f.cmd("set radio2 910.5,500,8,5,rxtx,80"); diff --git a/test/test_radio_profile_cli.py b/test/test_radio_profile_cli.py index 58fdc711..0204e0ed 100644 --- a/test/test_radio_profile_cli.py +++ b/test/test_radio_profile_cli.py @@ -19,7 +19,10 @@ class RadioProfileCLITest(unittest.TestCase): str(ROOT/'src/helpers/RadioProfileCLI.cpp'), str(ROOT/'test/fixtures/radio_profiles/cli_test.cpp'), '-o', str(exe)], check=True, capture_output=True, text=True) - subprocess.run([str(exe)], check=True) + for scenario in (None, 'permanent_order', 'temporary_boundary', 'lease_creation', + 'save_failure', 'schedule_permutations', 'storage_isolation'): + with self.subTest(scenario=scenario or 'commands_persistence_and_timers'): + subprocess.run([str(exe)] + ([scenario] if scenario else []), check=True) if __name__ == '__main__': unittest.main() diff --git a/test/test_repeater_radio_timing_integration.py b/test/test_repeater_radio_timing_integration.py index 8b5b2e2c..c8c472b0 100644 --- a/test/test_repeater_radio_timing_integration.py +++ b/test/test_repeater_radio_timing_integration.py @@ -23,6 +23,7 @@ HARNESS = r''' #include #include #include +#include #include #include #define MESH_DEBUG_PRINTLN(...) ((void)0) @@ -53,7 +54,9 @@ struct Board { int reboots = 0; void reboot() { ++reboots; } }; struct CLI { Board board; Board* getBoard() { return &board; } CLI& radioProfiles() { return *this; } - bool savePrimaryPreamble(uint16_t) { return true; } + bool preamble_save_success = true; + unsigned preamble_saves = 0; + bool savePrimaryPreamble(uint16_t) { ++preamble_saves; return preamble_save_success; } }; 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; } }; @@ -92,6 +95,7 @@ public: uint32_t scheduled_temp_radio_end_time = 0, scheduled_temp_radio_end_check_at = 0; bool scheduled_temp_radio_end_check_final = false; uint32_t scheduled_radio_retry_at = 0; + uint32_t scheduled_radio_save_retry_at = 0; uint8_t scheduled_radio_retry_failures = 0; bool pending_self_advert = false, pending_self_advert_flood = false; uint32_t pending_self_advert_delay = 0; @@ -153,6 +157,41 @@ static void startTemp(MyMesh& m, int minutes) { assert(m.temp_radio_applied); } int main() { + for (bool rollover : {false, true}) { + MyMesh m; + if (rollover) { now_ms = UINT32_MAX - 30000; m.last_millis = now_ms; } + char reply[160]; + m.addScheduledRadioParams(false, 912.5, 250, 5, 5, m.rtc.now + 2, 0, reply, 48); + assert(!strncmp(reply, "OK", 2)); + m._cli.preamble_save_success = false; + m.advance(2); m.servicePostMeshLoop(); + assert(m._cli.preamble_saves == 1 && m.saves == 0); + assert(m.next_scheduled_radio_check_at == m.scheduled_radio_save_retry_at); + assert(m._prefs.freq == 909.5f); + for (unsigned i = 0; i < 10; ++i) m.servicePostMeshLoop(); + assert(m._cli.preamble_saves == 1); // storage backoff survives the scheduler's cleanup + m._cli.preamble_save_success = true; + m.advance(59); m.servicePostMeshLoop(); + assert(m._cli.preamble_saves == 1); + m.advance(1); m.servicePostMeshLoop(); + assert(m._cli.preamble_saves == 2 && m.saves == 1); + assert(m._prefs.freq == 912.5f && !m.saved_radio_apply_pending); + } + { // Backing off a failed permanent save must not delay temporary start/end. + MyMesh m; + char reply[160]; + m.addScheduledRadioParams(false, 912.5, 250, 5, 5, m.rtc.now + 2, 0, reply, 48); + m.addScheduledRadioParams(true, 911.5, 250, 5, 5, m.rtc.now + 30, m.rtc.now + 90, reply, 80); + m._cli.preamble_save_success = false; + m.advance(2); m.servicePostMeshLoop(); + assert(m._cli.preamble_saves == 1); + m.advance(28); m.servicePostMeshLoop(); + assert(m.temp_radio_applied && m.radio_timing.isTemporary()); + assert(m._cli.preamble_saves == 1); + m.advance(60); m.servicePostMeshLoop(); + assert(!m.temp_radio_applied && !m.radio_timing.isTemporary()); + assert(m._prefs.freq == 909.5f); + } { // Pending schedules do not change current timings, even on an apply failure. MyMesh m; const uint32_t normal_local = m.next_local_advert; @@ -283,7 +322,7 @@ int main() { m.advance(23 * HOUR); m.servicePostMeshLoop(); assert(m._cli.board.reboots == 1); } - puts("13 production scheduler/watchdog/advert scenarios passed"); + puts("Production scheduler, storage-backoff, watchdog and advert scenarios passed"); } '''