fix: keep rolling codes in order when a stop is queued

A priority stop always went to the front of the radio queue, ahead of
an earlier frame of the same remote that was still waiting.  The older
rolling code then went out last and the motor rejected it.

Split "may use the reserved slots" (priority) from "goes to the front"
(front) and only send a stop to the front when its remote has nothing
queued at the back.  The radio task and flushes track which back jobs
have been taken so Transceiver::isQueued() can tell.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
cjkas 2026-10-11 12:46:40 +02:00
parent 130b5b6b0a
commit 44227bfd87
2 changed files with 30 additions and 5 deletions

View file

@ -63,6 +63,15 @@ static std::atomic<uint32_t> txPreemptWaiters{0};
static std::atomic<uint32_t> txSuspendMask{0}; static std::atomic<uint32_t> txSuspendMask{0};
// Last sequence number handed to a queued job. Only touched under TxQueueLock. // Last sequence number handed to a queued job. Only touched under TxQueueLock.
static uint32_t txSeq = 0; static uint32_t txSeq = 0;
// Sequence number of the last job queued at the back that the radio task took
// (or a flush dropped). Those jobs go out in order so any later one is still
// waiting in the queue.
static std::atomic<uint32_t> txBackTaken{0};
// The radio task and a flush both move txBackTaken. Never let it go backwards.
static void advanceBackTaken(uint32_t seq) {
uint32_t cur = txBackTaken;
while((int32_t)(seq - cur) > 0 && !txBackTaken.compare_exchange_weak(cur, seq)) {}
}
// How long a shade's simulated movement waits for its command to go on air // How long a shade's simulated movement waits for its command to go on air
// before it starts anyway in case the event was lost. // before it starts anyway in case the event was lost.
#define TX_PENDING_TIMEOUT 30000 #define TX_PENDING_TIMEOUT 30000
@ -4027,8 +4036,13 @@ bool SomfyRemote::sendCommandAfter(somfy_commands cmd, uint8_t repeat, uint8_t s
ESP_LOGD(TAG, "CMD:%s ADDR:%d RCODE:%d REPEAT:%d", ESP_LOGD(TAG, "CMD:%s ADDR:%d RCODE:%d REPEAT:%d",
translateSomfyCommand(frame.cmd).c_str(), translateSomfyCommand(frame.cmd).c_str(),
frame.remoteAddress, frame.rollingCode, repeat); frame.remoteAddress, frame.rollingCode, repeat);
// A stop jumps the queue unless an earlier frame of this remote is still
// waiting. Overtaking it would put the rolling codes out of order and the
// motor would reject the older one.
const bool front = priority && !somfy.transceiver.isQueued(this->lastBackTxSeq);
// Only simulate the command if it is actually going on air. // Only simulate the command if it is actually going on air.
if(!somfy.sendFrame(frame, repeat, delayMs, priority, &seq)) return false; if(!somfy.sendFrame(frame, repeat, delayMs, priority, front, &seq)) return false;
if(!front) this->lastBackTxSeq = seq;
} }
this->commitRollingCode(frame.rollingCode); this->commitRollingCode(frame.rollingCode);
this->lastFrame = frame; this->lastFrame = frame;
@ -4061,14 +4075,16 @@ bool SomfyRemote::repeatFrame(uint8_t repeat) {
// Only advance the counter for frames that will go on air. // Only advance the counter for frames that will go on air.
if(!somfy.transceiver.queueTx(job)) return false; if(!somfy.transceiver.queueTx(job)) return false;
this->lastFrame.repeats += repeat + 1; this->lastFrame.repeats += repeat + 1;
this->lastBackTxSeq = job.seq;
//somfy.processFrame(this->lastFrame, true); //somfy.processFrame(this->lastFrame, true);
return true; return true;
} }
bool SomfyShadeController::sendFrame(somfy_frame_t &frame, uint8_t repeat, uint16_t delayMs, bool priority, uint32_t *seq) { bool SomfyShadeController::sendFrame(somfy_frame_t &frame, uint8_t repeat, uint16_t delayMs, bool priority, bool front, uint32_t *seq) {
somfy_tx_job_t job; somfy_tx_job_t job;
encodeTxJob(job, frame, repeat); encodeTxJob(job, frame, repeat);
job.firstRepeat = 1; job.firstRepeat = 1;
job.priority = priority; job.priority = priority;
job.front = priority && front;
if(delayMs > 0) job.notBefore = millis() + delayMs; if(delayMs > 0) job.notBefore = millis() + delayMs;
if(!this->transceiver.queueTx(job)) return false; if(!this->transceiver.queueTx(job)) return false;
if(seq) *seq = job.seq; if(seq) *seq = job.seq;
@ -5218,13 +5234,18 @@ bool Transceiver::queueTx(somfy_tx_job_t &job) {
if(++txSeq == 0) ++txSeq; // 0 means no job. if(++txSeq == 0) ++txSeq; // 0 means no job.
job.seq = txSeq; job.seq = txSeq;
// We are the only producer while holding the lock so the space check holds. // We are the only producer while holding the lock so the space check holds.
if(job.priority) xQueueSendToFront(txJobQueue, &job, 0); if(job.front) xQueueSendToFront(txJobQueue, &job, 0);
else xQueueSendToBack(txJobQueue, &job, 0); else xQueueSendToBack(txJobQueue, &job, 0);
return true; return true;
} }
bool Transceiver::isQueued(uint32_t seq) {
// Only meaningful for jobs queued at the back.
return seq != 0 && (int32_t)(seq - txBackTaken) > 0;
}
void Transceiver::flushTx() { void Transceiver::flushTx() {
TxQueueLock lock; TxQueueLock lock;
++txGen; ++txGen;
advanceBackTaken(txSeq);
if(!txJobQueue) return; if(!txJobQueue) return;
// Drain rather than reset so every discarded job is reported as dropped. // Drain rather than reset so every discarded job is reported as dropped.
somfy_tx_job_t job; somfy_tx_job_t job;
@ -5262,6 +5283,7 @@ void Transceiver::txTask(void *arg) {
somfy_tx_job_t job; somfy_tx_job_t job;
for(;;) { for(;;) {
if(xQueueReceive(txJobQueue, &job, portMAX_DELAY) != pdTRUE) continue; if(xQueueReceive(txJobQueue, &job, portMAX_DELAY) != pdTRUE) continue;
if(!job.front) advanceBackTaken(job.seq);
// Flushed after we took it off the queue. // Flushed after we took it off the queue.
if(txSuspendMask || job.gen != txGen) { if(txSuspendMask || job.gen != txGen) {
postTxEvent(tx_event_t::dropped, job, millis()); postTxEvent(tx_event_t::dropped, job, millis());

View file

@ -216,7 +216,8 @@ struct somfy_tx_job_t {
uint8_t repeats = 0; // Number of repeat frames after the first frame. uint8_t repeats = 0; // Number of repeat frames after the first frame.
uint8_t firstRepeat = 1; // 80-bit repeat counter used for the first repeat frame. uint8_t firstRepeat = 1; // 80-bit repeat counter used for the first repeat frame.
bool raw = false; // Repeater frame. Waits for the receiver to be idle. bool raw = false; // Repeater frame. Waits for the receiver to be idle.
bool priority = false; // Goes to the front of the queue and may use the reserved slots. bool priority = false; // May use the reserved slots.
bool front = false; // Goes to the front of the queue. Only for priority jobs.
uint32_t gen = 0; // Flush generation when queued. Stale jobs are dropped. uint32_t gen = 0; // Flush generation when queued. Stale jobs are dropped.
uint32_t seq = 0; // Assigned when queued so the job's events can be matched up. uint32_t seq = 0; // Assigned when queued so the job's events can be matched up.
uint32_t notBefore = 0; // millis() before which the job must not go on air (0 = none). uint32_t notBefore = 0; // millis() before which the job must not go on air (0 = none).
@ -267,6 +268,7 @@ class SomfyRemote {
bool flipCommands = false; bool flipCommands = false;
uint16_t lastRollingCode = 0; uint16_t lastRollingCode = 0;
uint32_t lastTxSeq = 0; // Radio job of the last command sent, 0 if it didn't use the radio. uint32_t lastTxSeq = 0; // Radio job of the last command sent, 0 if it didn't use the radio.
uint32_t lastBackTxSeq = 0; // Last radio job queued at the back of the queue.
uint8_t flags = 0; uint8_t flags = 0;
uint8_t bitLength = 0; uint8_t bitLength = 0;
uint8_t repeats = 1; uint8_t repeats = 1;
@ -563,6 +565,7 @@ class Transceiver {
void flushTx(); void flushTx();
void suspendTx(uint8_t reason); void suspendTx(uint8_t reason);
void resumeTx(uint8_t reason); void resumeTx(uint8_t reason);
bool isQueued(uint32_t seq);
void beginTransmit(); void beginTransmit();
void endTransmit(); void endTransmit();
void emitFrame(somfy_frame_t *frame, somfy_rx_t *rx = nullptr); void emitFrame(somfy_frame_t *frame, somfy_rx_t *rx = nullptr);
@ -618,7 +621,7 @@ class SomfyShadeController {
SomfyGroup * getGroupById(uint8_t groupId); SomfyGroup * getGroupById(uint8_t groupId);
SomfyShade * findShadeByRemoteAddress(uint32_t address); SomfyShade * findShadeByRemoteAddress(uint32_t address);
SomfyGroup * findGroupByRemoteAddress(uint32_t address); SomfyGroup * findGroupByRemoteAddress(uint32_t address);
bool sendFrame(somfy_frame_t &frame, uint8_t repeats = 0, uint16_t delayMs = 0, bool priority = false, uint32_t *seq = nullptr); bool sendFrame(somfy_frame_t &frame, uint8_t repeats = 0, uint16_t delayMs = 0, bool priority = false, bool front = false, uint32_t *seq = nullptr);
void processFrame(somfy_frame_t &frame, bool internal = false); void processFrame(somfy_frame_t &frame, bool internal = false);
void markTxPending(uint32_t remoteAddress, uint32_t seq); void markTxPending(uint32_t remoteAddress, uint32_t seq);
void processTxEvent(const somfy_tx_event_t &evt); void processTxEvent(const somfy_tx_event_t &evt);