diff --git a/src/ShadeMotion.cpp b/src/ShadeMotion.cpp index 39c9acb..da5328b 100644 --- a/src/ShadeMotion.cpp +++ b/src/ShadeMotion.cpp @@ -29,3 +29,13 @@ int8_t displayPosition(float pos, bool flip) { if(pos < 0) return -1; return static_cast(flip && pos >= 0.00f ? floor(100.0f - pos) : floor(pos)); } +void TxHold::mark(uint32_t jobSeq, bool moving, uint32_t now) { + if(jobSeq == 0 || this->seq != 0 || moving) return; + this->seq = jobSeq; + this->since = now; +} +bool TxHold::release(uint32_t jobSeq) { + if(jobSeq == 0 || jobSeq != this->seq) return false; + this->seq = 0; + return true; +} diff --git a/src/ShadeMotion.h b/src/ShadeMotion.h index 903d794..d45dc0e 100644 --- a/src/ShadeMotion.h +++ b/src/ShadeMotion.h @@ -12,4 +12,23 @@ float travelPosition(float startPos, int8_t dir, uint32_t elapsedMs, int32_t tra // A position as reported to clients: -1 when unset, otherwise the whole // percentage, counted from the other end when flipped. int8_t displayPosition(float pos, bool flip); + +// The radio job that set an idle shade moving. Its simulated movement is held +// until the job goes on air, since that is when the motor starts. +struct TxHold { + uint32_t seq = 0; // 0 when nothing is held. + uint32_t since = 0; // millis() when the hold started. + // Holds for job seq unless the shade was already moving (its motor carries on + // with the earlier command) or a job is already held. The first job held is + // the one that starts the motor. A command sent while processing another + // registers before it: for a simMy shade My/Favorite queues its Up/Down from + // moveToMyPosition() first, and that Up/Down is what starts a motor with no + // favorite of its own, so the outer My/Favorite is ignored on purpose. + void mark(uint32_t jobSeq, bool moving, uint32_t now); + // Ends the hold when jobSeq is the held job. True when it was. + bool release(uint32_t jobSeq); + bool held() const { return this->seq != 0; } + bool timedOut(uint32_t now, uint32_t timeoutMs) const { return now - this->since >= timeoutMs; } + void clear() { this->seq = 0; } +}; #endif diff --git a/src/Somfy.cpp b/src/Somfy.cpp index a22c5af..451ce0b 100644 --- a/src/Somfy.cpp +++ b/src/Somfy.cpp @@ -762,20 +762,20 @@ void SomfyShade::checkMovement() { uint8_t currPos = floor(this->currentPos); uint8_t currTiltPos = floor(this->currentTiltPos); if(this->direction != 0) this->lastMovement = this->direction; - if(this->txPendingSeq != 0 && this->direction == 0 && this->tiltDirection == 0) { + if(this->txHold.held() && this->direction == 0 && this->tiltDirection == 0) { // The command didn't set the shade moving so there is nothing to hold. - this->txPendingSeq = 0; + this->txHold.clear(); } - if(this->txPendingSeq != 0) { + if(this->txHold.held()) { // The command that set us moving hasn't gone on air yet so the motor hasn't // started. Hold the simulated position until it does. - if((uint32_t)curTime - this->txPendingSince < TX_PENDING_TIMEOUT) { + if(!this->txHold.timedOut((uint32_t)curTime, TX_PENDING_TIMEOUT)) { this->moveStart = this->tiltStart = curTime; if(currDir != this->direction || currTiltDir != this->tiltDirection) this->emitState(); return; } ESP_LOGW(TAG, "[Shade %u] Timed out waiting for the radio, moving anyway", this->shadeId); - this->txPendingSeq = 0; + this->txHold.clear(); } if (sunFlag) { if (isSunny && !isWindy) { // It is sunny and there is no wind so we should be extended @@ -2335,20 +2335,15 @@ void SomfyShade::setTiltMovement(int8_t dir) { } } void SomfyShade::markTxPending(uint32_t seq) { - // The motor starts on the first of several queued commands to go on air. - if(this->txPendingSeq != 0) return; // This runs before the caller sets the new target so direction still says - // whether the shade was moving. A moving motor carries on with its earlier - // command so its simulation must not be held. A reversal is simulated as - // soon as it is queued even though the motor turns once it is on air. - if(this->direction != 0 || this->tiltDirection != 0) return; - this->txPendingSeq = seq; - this->txPendingSince = millis(); + // whether the shade was moving. A reversal is simulated as soon as it is + // queued even though the motor turns once it is on air. See TxHold::mark + // for which of several queued commands is held. + this->txHold.mark(seq, this->direction != 0 || this->tiltDirection != 0, millis()); } void SomfyShade::processTxEvent(const somfy_tx_event_t &evt) { if(evt.seq == 0) return; - if(this->txPendingSeq == evt.seq) { - this->txPendingSeq = 0; + if(this->txHold.release(evt.seq)) { if(evt.type == tx_event_t::started) { // The motor starts now so the simulated movement does too. this->moveStart = this->tiltStart = evt.at; diff --git a/src/Somfy.h b/src/Somfy.h index 39cf4de..edd948b 100644 --- a/src/Somfy.h +++ b/src/Somfy.h @@ -23,6 +23,7 @@ #include "SomfyFrame.h" #include "TxBuffer.h" +#include "ShadeMotion.h" enum class group_types : byte { channel = 0x00 }; @@ -240,9 +241,8 @@ class SomfyShade : public SomfyRemote { bool settingTiltPos = false; uint32_t awaitMy = 0; // Radio job that set the shade moving. The simulated position is held - // until it goes on air. 0 when nothing is waiting. - uint32_t txPendingSeq = 0; - uint32_t txPendingSince = 0; + // until it goes on air. + TxHold txHold; // Stop sent when the simulated target was reached. If it never goes on // air the motor runs on to its end limit. 0 once it is on air. uint32_t txStopSeq = 0; diff --git a/test/test_motion/test_motion.cpp b/test/test_motion/test_motion.cpp index d17c8f2..92f4822 100644 --- a/test/test_motion/test_motion.cpp +++ b/test/test_motion/test_motion.cpp @@ -110,6 +110,74 @@ static void test_flipCommand_keeps_the_rest() { for(somfy_commands cmd : same) ASSERT_CMD(cmd, flipCommand(cmd)); } +/********************************************************************* + * TxHold + ********************************************************************/ +static void test_txhold_idle_shade_is_held() { + TxHold hold; + hold.mark(7, false, 1000); + TEST_ASSERT_TRUE(hold.held()); + TEST_ASSERT_EQUAL_UINT32(7, hold.seq); + TEST_ASSERT_EQUAL_UINT32(1000, hold.since); +} +static void test_txhold_moving_shade_is_not_held() { + TxHold hold; + hold.mark(7, true, 1000); + TEST_ASSERT_FALSE(hold.held()); +} +static void test_txhold_ignores_seq_zero() { + TxHold hold; + hold.mark(0, false, 1000); + TEST_ASSERT_FALSE(hold.held()); + hold.mark(7, false, 1000); + TEST_ASSERT_FALSE(hold.release(0)); + TEST_ASSERT_TRUE(hold.held()); +} +static void test_txhold_first_job_wins() { + // simMy Favorite: the Up/Down queued from inside processFrame (N+1) is + // marked before the outer Favorite (N), and the hold stays on the Up/Down. + TxHold hold; + hold.mark(11, false, 1000); + hold.mark(10, false, 1005); + TEST_ASSERT_EQUAL_UINT32(11, hold.seq); + TEST_ASSERT_EQUAL_UINT32(1000, hold.since); + TEST_ASSERT_FALSE(hold.release(10)); + TEST_ASSERT_TRUE(hold.held()); + TEST_ASSERT_TRUE(hold.release(11)); + TEST_ASSERT_FALSE(hold.held()); +} +static void test_txhold_release_only_matches_the_held_job() { + TxHold hold; + hold.mark(5, false, 0); + TEST_ASSERT_FALSE(hold.release(4)); + TEST_ASSERT_FALSE(hold.release(6)); + TEST_ASSERT_TRUE(hold.held()); + TEST_ASSERT_TRUE(hold.release(5)); + TEST_ASSERT_FALSE(hold.release(5)); +} +static void test_txhold_new_hold_after_release() { + TxHold hold; + hold.mark(5, false, 0); + hold.release(5); + hold.mark(6, false, 200); + TEST_ASSERT_EQUAL_UINT32(6, hold.seq); + TEST_ASSERT_EQUAL_UINT32(200, hold.since); +} +static void test_txhold_times_out() { + TxHold hold; + hold.mark(5, false, 1000); + TEST_ASSERT_FALSE(hold.timedOut(1000, 30000)); + TEST_ASSERT_FALSE(hold.timedOut(30999, 30000)); + TEST_ASSERT_TRUE(hold.timedOut(31000, 30000)); +} +static void test_txhold_times_out_across_millis_wrap() { + TxHold hold; + hold.mark(5, false, 0xFFFFF000u); + TEST_ASSERT_FALSE(hold.timedOut(0x00001000u, 30000)); // 8192ms later + TEST_ASSERT_FALSE(hold.timedOut(0x00006000u, 30000)); // 28672ms later + TEST_ASSERT_TRUE(hold.timedOut(0x00007000u, 30000)); // 32768ms later +} + int main(int argc, char **argv) { (void)argc; (void)argv; @@ -126,5 +194,13 @@ int main(int argc, char **argv) { RUN_TEST(test_displayPosition); RUN_TEST(test_flipCommand_swaps_directions); RUN_TEST(test_flipCommand_keeps_the_rest); + RUN_TEST(test_txhold_idle_shade_is_held); + RUN_TEST(test_txhold_moving_shade_is_not_held); + RUN_TEST(test_txhold_ignores_seq_zero); + RUN_TEST(test_txhold_first_job_wins); + RUN_TEST(test_txhold_release_only_matches_the_held_job); + RUN_TEST(test_txhold_new_hold_after_release); + RUN_TEST(test_txhold_times_out); + RUN_TEST(test_txhold_times_out_across_millis_wrap); return UNITY_END(); }