fix: refuse a radio job when its outcomes might not fit in the event ring

A full event ring threw an outcome away, breaking the rule that every job
we submit ends with exactly one of finished, aborted or dropped.  A lost
outcome left a favorite uncommitted and a held shade waiting out its 30s
timeout.  submit() and hasRoom() now keep room for two outcomes of every
job not yet ended, so an event can't be lost; if loop() stops collecting
them, new jobs are refused instead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

src/RadioScheduler.cpp

@@ -21,6 +21,10 @@ uint32_t RadioScheduler::submit(somfy_tx_job_t &job, bool priority, uint16_t del
     ESP_LOGE(TAG, "Radio TX queue full, dropping transmission");
     return 0;
   }
+  if(!this->outcomesFit()) {
+    ESP_LOGE(TAG, "Radio TX outcomes not collected, dropping transmission");
+    return 0;
+  }
   // 0 means no job so it is skipped on wrap.
   if(++this->lastSeq == 0) ++this->lastSeq;
   job.seq = this->lastSeq;
@@ -40,7 +44,13 @@ uint32_t RadioScheduler::submit(somfy_tx_job_t &job, bool priority, uint16_t del
   return job.seq;
 }
 bool RadioScheduler::hasRoom(bool priority) const {
-  return !this->suspendMask && admits(TX_JOB_QUEUE_SIZE - this->count, priority);
+  return !this->suspendMask && admits(TX_JOB_QUEUE_SIZE - this->count, priority) && this->outcomesFit();
+}
+bool RadioScheduler::outcomesFit() const {
+  // Each job not yet ended reports at most two outcomes: started, then how it
+  // ended.  Counting repeater frames too keeps this simple and errs safe.
+  const uint32_t unended = this->count + (this->taken ? 1u : 0u) + 1u;
+  return this->eventCount + 2u * unended <= TX_EVENT_QUEUE_SIZE;
 }
 bool RadioScheduler::waitingFor(uint32_t remoteAddress) const {
   for(uint8_t i = 0; i < this->count; i++) {
@@ -112,7 +122,8 @@ void RadioScheduler::post(tx_event_t type, const somfy_tx_job_t &job, uint32_t a
   // Repeater frames don't belong to any of our remotes.
   if(job.raw) return;
   if(this->eventCount >= TX_EVENT_QUEUE_SIZE) {
-    ESP_LOGW(TAG, "Radio TX event queue full");
+    // submit() leaves room for every outcome so this can't happen short of a bug.
+    ESP_LOGE(TAG, "Radio TX event queue full, outcome lost");
     return;
   }
   somfy_tx_event_t &evt = this->events[(this->eventHead + this->eventCount) % TX_EVENT_QUEUE_SIZE];

src/RadioScheduler.h

@@ -18,7 +18,8 @@
 class RadioScheduler {
   public:
     // Queues a job.  Its ticket comes back and is also set in job.seq, 0 when
-    // the queue is full or suspended.  A priority job (a stop) may use the
+    // the queue is full or suspended, or when outcomes aren't collected with
+    // popEvent() fast enough to leave room for its own.  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 until
@@ -66,6 +67,9 @@ class RadioScheduler {
     somfy_tx_event_t events[TX_JOB_QUEUE_SIZE * 3];
     uint8_t eventHead = 0;
     uint8_t eventCount = 0;
+    // Whether the outcomes of every job not yet ended, and of one more, still
+    // fit in events.  A job is refused otherwise so no outcome is ever lost.
+    bool outcomesFit() const;
     // Whether a job of the remote is still waiting.
     bool waitingFor(uint32_t remoteAddress) const;
     void remove(uint8_t i);

test/test_radioscheduler/test_radioscheduler.cpp

@@ -356,6 +356,54 @@ static void test_every_job_gets_one_outcome_across_a_flush() {
   }
   for(uint8_t i = 0; i < 6; i++) TEST_ASSERT_EQUAL_UINT8(1, outcomes[i]);
 }
+static void test_no_outcome_is_lost_when_events_arent_collected() {
+  // loop() collects the events.  If it stalls, jobs are refused rather than
+  // their outcomes thrown away.
+  RadioScheduler s;
+  uint32_t accepted = 0;
+  for(uint32_t i = 0; i < 200; i++) {
+    if(!submit(s, REMOTE_A)) break;
+    accepted++;
+    runNext(s);
+  }
+  TEST_ASSERT_TRUE(accepted > 0);
+  TEST_ASSERT_TRUE(accepted < 200);
+  TEST_ASSERT_FALSE(s.hasRoom(false));
+  uint32_t started = 0, finished = 0;
+  somfy_tx_event_t evt;
+  while(s.popEvent(evt)) {
+    if(evt.type == tx_event_t::started) started++;
+    else if(evt.type == tx_event_t::finished) finished++;
+  }
+  TEST_ASSERT_EQUAL_UINT32(accepted, started);
+  TEST_ASSERT_EQUAL_UINT32(accepted, finished);
+  TEST_ASSERT_TRUE(s.hasRoom(false));
+  TEST_ASSERT_NOT_EQUAL(0, submit(s, REMOTE_A));
+}
+static void test_flush_reports_every_drop_when_events_arent_collected() {
+  RadioScheduler s;
+  uint32_t tickets[64];
+  uint8_t n = 0;
+  // Outcomes of jobs already sent take up part of the event ring.
+  for(uint8_t i = 0; i < 20; i++) {
+    tickets[n] = submit(s, REMOTE_A);
+    TEST_ASSERT_NOT_EQUAL(0, tickets[n]);
+    n++;
+    runNext(s);
+  }
+  // Queue as many as are taken.
+  for(uint32_t t; n < 64 && (t = submit(s, REMOTE_B, 0, true)) != 0;) tickets[n++] = t;
+  TEST_ASSERT_TRUE(n > 20);
+  s.flush(0);
+  uint8_t outcomes[64] = {};
+  somfy_tx_event_t evt;
+  while(s.popEvent(evt)) {
+    if(evt.type == tx_event_t::started) continue;
+    for(uint8_t i = 0; i < n; i++)
+      if(tickets[i] == evt.seq) outcomes[i]++;
+  }
+  for(uint8_t i = 0; i < n; i++) TEST_ASSERT_EQUAL_UINT8(1, outcomes[i]);
+}
 static void test_unknown_ticket_is_ignored() {
   RadioScheduler s;
   TEST_ASSERT_FALSE(s.keepSending(0));
@@ -454,6 +502,8 @@ int main(int argc, char **argv) {
   RUN_TEST(test_flush_cuts_short_the_job_on_air);
   RUN_TEST(test_job_flushed_before_it_starts_is_dropped);
   RUN_TEST(test_every_job_gets_one_outcome_across_a_flush);
+  RUN_TEST(test_no_outcome_is_lost_when_events_arent_collected);
+  RUN_TEST(test_flush_reports_every_drop_when_events_arent_collected);
   RUN_TEST(test_unknown_ticket_is_ignored);
   RUN_TEST(test_suspend_flushes_and_refuses_jobs);
   RUN_TEST(test_resume_needs_every_reason_cleared);