From b768c5f987e2b77ff9c9a21b7ee4627ae1cb008b Mon Sep 17 00:00:00 2001 From: cjkas Date: Tue, 13 Oct 2026 15:03:17 +0200 Subject: [PATCH] fix: count the set-My settle delay from the end of the job ahead The delay was counted from when the job was queued. The stop at the target is queued just before the favorite and is on air for about 245ms, so the 200ms settle delay had run out before the favorite's turn and only the 100ms beat between jobs was left. A delayed job now waits its delay after whichever came later: being queued, or the job ahead of it going off air. Co-Authored-By: Claude Opus 5.5 --- GLOSSARY.md | 2 +- src/RadioScheduler.cpp | 22 ++++---- src/RadioScheduler.h | 8 ++- src/TxBuffer.h | 3 +- .../test_radioscheduler.cpp | 50 ++++++++++++++++++- 5 files changed, 69 insertions(+), 16 deletions(-) diff --git a/GLOSSARY.md b/GLOSSARY.md index cf91174..981e58f 100644 --- a/GLOSSARY.md +++ b/GLOSSARY.md @@ -17,7 +17,7 @@ What submitting a radio job returns: the job's sequence number, 0 when the radio A job normally joins the back of the queue. A *priority* job (a stop) goes to the front, unless an earlier job from the same remote is still waiting. Overtaking that job would put the remote's rolling codes out of order, and the motor would reject the older frame. **Delayed job** -A job that must not go on air before a given time (for example the long press that stores a favorite). It holds up the jobs behind it, but front jobs may overtake it while it waits. +A job that must wait a given time after it was queued and after the transmission ahead of it ended, so the motor has settled (for example the long press that stores a favorite). It holds up the jobs behind it, but front jobs may overtake it while it waits. **Flush** Drops every waiting job (each gets a *dropped* event) and stops the job on air after its current frame (it gets *aborted*). diff --git a/src/RadioScheduler.cpp b/src/RadioScheduler.cpp index 654bb72..7e1bfcf 100644 --- a/src/RadioScheduler.cpp +++ b/src/RadioScheduler.cpp @@ -26,11 +26,8 @@ uint32_t RadioScheduler::submit(somfy_tx_job_t &job, bool priority, uint16_t del job.seq = this->lastSeq; job.priority = priority; job.front = priority && !job.raw && !this->waitingFor(job.frame.remoteAddress); - job.notBefore = 0; - if(delayMs > 0) { - job.notBefore = now + delayMs; - if(job.notBefore == 0) job.notBefore = 1; // 0 means not delayed. - } + job.queuedAt = now; + job.delayMs = delayMs; // Front jobs go out in the order they came, ahead of the back. uint8_t at = this->count; if(job.front) { @@ -61,11 +58,14 @@ bool RadioScheduler::next(uint32_t now, somfy_tx_job_t &job, uint32_t &waitMs) { // Jobs go out in order, so a delayed job holds up the ones behind it. A // front job queued meanwhile is ahead of it. const somfy_tx_job_t &head = this->jobs[0]; - if(head.notBefore != 0) { - // Works across a millis() wrap. - const int32_t due = (int32_t)(head.notBefore - now); - if(due > 0) { - waitMs = (uint32_t)due; + if(head.delayMs != 0) { + // The delay lets the motor settle, so it counts from the job ahead of it + // going off air when that came after the job was queued. Ages are unsigned + // so this works across a millis() wrap. + uint32_t waited = now - head.queuedAt; + if(this->endedOnce && now - this->lastEnded < waited) waited = now - this->lastEnded; + if(waited < head.delayMs) { + waitMs = head.delayMs - waited; return false; } } @@ -94,6 +94,8 @@ void RadioScheduler::ended(uint32_t ticket, bool complete, uint32_t now) { if(ticket == 0 || ticket != this->taken) return; this->post(complete ? tx_event_t::finished : tx_event_t::aborted, this->takenJob, now); this->taken = 0; + this->lastEnded = now; + this->endedOnce = true; } void RadioScheduler::flush(uint32_t now) { // Every discarded job is reported as dropped. diff --git a/src/RadioScheduler.h b/src/RadioScheduler.h index 8ce7231..88808c0 100644 --- a/src/RadioScheduler.h +++ b/src/RadioScheduler.h @@ -21,8 +21,9 @@ class RadioScheduler { // the queue is full or suspended. A priority job (a stop) may use the // reserved slots and goes ahead of every job queued at the back, unless a // job of its remote is already waiting: overtaking that would put the - // rolling codes out of order. A delayed job doesn't go on air before - // delayMs from now and holds up the jobs queued behind it. + // rolling codes out of order. A delayed job doesn't go on air until + // delayMs after it was queued and after the job ahead of it ended, and holds + // up the jobs queued behind it. uint32_t submit(somfy_tx_job_t &job, bool priority, uint16_t delayMs, uint32_t now); // Whether a job would be taken right now. bool hasRoom(bool priority) const; @@ -59,6 +60,9 @@ class RadioScheduler { uint32_t taken = 0; somfy_tx_job_t takenJob; bool takenFlushed = false; + // When a job last went off air, for the delay of the job after it. + uint32_t lastEnded = 0; + bool endedOnce = false; somfy_tx_event_t events[TX_JOB_QUEUE_SIZE * 3]; uint8_t eventHead = 0; uint8_t eventCount = 0; diff --git a/src/TxBuffer.h b/src/TxBuffer.h index 9f4cc74..b5ac77f 100644 --- a/src/TxBuffer.h +++ b/src/TxBuffer.h @@ -62,7 +62,8 @@ struct somfy_tx_job_t { bool priority = false; // May use the reserved slots. bool front = false; // Queued ahead of the back. Only for priority jobs. uint32_t seq = 0; // Ticket the job's events are matched up by. - uint32_t notBefore = 0; // millis() before which the job must not go on air (0 = none). + uint32_t queuedAt = 0; // millis() when it was submitted. + uint16_t delayMs = 0; // How long it waits after it was queued and after the job ahead of it ended. }; enum class tx_event_t : uint8_t { started, // The job is going on air. diff --git a/test/test_radioscheduler/test_radioscheduler.cpp b/test/test_radioscheduler/test_radioscheduler.cpp index 9f29e36..3ff0887 100644 --- a/test/test_radioscheduler/test_radioscheduler.cpp +++ b/test/test_radioscheduler/test_radioscheduler.cpp @@ -191,8 +191,51 @@ static void test_stop_overtakes_a_delayed_job_of_another_remote() { const uint32_t a = submit(s, REMOTE_A, 0, false, 500); const uint32_t stop = submit(s, REMOTE_B, 100, true); TEST_ASSERT_EQUAL_UINT32(stop, runNext(s, 100)); - TEST_ASSERT_EQUAL_UINT32(0, runNext(s, 200)); - TEST_ASSERT_EQUAL_UINT32(a, runNext(s, 500)); + // The stop went off air at 100 so the delay starts again from there. + TEST_ASSERT_EQUAL_UINT32(0, runNext(s, 500)); + TEST_ASSERT_EQUAL_UINT32(a, runNext(s, 600)); +} +static void test_delay_counts_from_the_job_ahead_going_off_air() { + // The stop at the target is queued just before the favorite and is on air + // longer than the favorite's delay. + RadioScheduler s; + const uint32_t stop = submit(s, REMOTE_A, 0, true); + const uint32_t setMy = submit(s, REMOTE_A, 0, false, 200); + somfy_tx_job_t job; + uint32_t waitMs; + TEST_ASSERT_TRUE(s.next(0, job, waitMs)); + TEST_ASSERT_TRUE(s.start(stop, 0)); + s.ended(stop, true, 250); + TEST_ASSERT_FALSE(s.next(350, job, waitMs)); + TEST_ASSERT_EQUAL_UINT32(100, waitMs); + TEST_ASSERT_TRUE(s.next(450, job, waitMs)); + TEST_ASSERT_EQUAL_UINT32(setMy, job.seq); +} +static void test_delay_ignores_a_job_that_ended_before_it_was_queued() { + RadioScheduler s; + submit(s, REMOTE_B, 0); + runNext(s, 50); + const uint32_t a = submit(s, REMOTE_A, 100, false, 200); + somfy_tx_job_t job; + uint32_t waitMs; + TEST_ASSERT_FALSE(s.next(100, job, waitMs)); + TEST_ASSERT_EQUAL_UINT32(200, waitMs); + TEST_ASSERT_TRUE(s.next(300, job, waitMs)); + TEST_ASSERT_EQUAL_UINT32(a, job.seq); +} +static void test_delay_after_a_job_ended_across_millis_wrap() { + RadioScheduler s; + const uint32_t stop = submit(s, REMOTE_A, 0xFFFFFF00u, true); + const uint32_t setMy = submit(s, REMOTE_A, 0xFFFFFF00u, false, 0x100); + somfy_tx_job_t job; + uint32_t waitMs; + s.next(0xFFFFFF00u, job, waitMs); + s.start(stop, 0xFFFFFF00u); + s.ended(stop, true, 0x80); + TEST_ASSERT_FALSE(s.next(0x100, job, waitMs)); + TEST_ASSERT_EQUAL_UINT32(0x80, waitMs); + TEST_ASSERT_TRUE(s.next(0x180, job, waitMs)); + TEST_ASSERT_EQUAL_UINT32(setMy, job.seq); } static void test_stop_waits_behind_its_remotes_delayed_job() { // The favorite's long press holds a rolling code, so the stop can't go first. @@ -400,6 +443,9 @@ int main(int argc, char **argv) { RUN_TEST(test_delayed_job_waits_until_due); RUN_TEST(test_delayed_job_holds_up_the_jobs_behind_it); RUN_TEST(test_stop_overtakes_a_delayed_job_of_another_remote); + RUN_TEST(test_delay_counts_from_the_job_ahead_going_off_air); + RUN_TEST(test_delay_ignores_a_job_that_ended_before_it_was_queued); + RUN_TEST(test_delay_after_a_job_ended_across_millis_wrap); RUN_TEST(test_stop_waits_behind_its_remotes_delayed_job); RUN_TEST(test_delay_across_millis_wrap); RUN_TEST(test_job_reports_started_then_finished);