diff --git a/docs/KNOWN_ISSUES.md b/docs/KNOWN_ISSUES.md index ae40e6d..5d5bd1e 100644 --- a/docs/KNOWN_ISSUES.md +++ b/docs/KNOWN_ISSUES.md @@ -32,12 +32,11 @@ The new `ControllerChunker` reads state as the response drains. On a slow link o - Same exposure exists in any other endpoint converted to chunked responses next (`/discovery`, `/shades`). Resolve this issue before expanding the pattern. - Other handlers on the web server task still change the shade model that `loop()` owns: - `/addRoom`, `/saveRoom`, `/deleteRoom`, `/addShade`, `/saveShade`, `/deleteShade`, `/addGroup`, `/saveGroup` and `/deleteGroup` add, change and remove rooms, shades and groups. - - `/linkRemote` and `/unlinkRemote` change a shade's linked remotes, and `/linkRemote` also sets a linked remote's rolling code. + - `/linkRemote` and `/unlinkRemote` change a shade's linked remotes. `/linkRemote` also sets a linked remote's rolling code, which `loop()` updates whenever it receives a frame from that remote: the last write wins, so a received frame can undo the code just set. - `/linkToGroup` and `/unlinkFromGroup` change a group's linked shades. - `/setPaired` sets `paired`. - - `/setRollingCode` and a shades restore change rolling codes (see [Rolling codes written from two tasks](#rolling-codes-written-from-two-tasks)). - Shade commands, set-My and unpair already go through the web command queue to `loop()`. + Shade commands, set-My, unpair, `/setRollingCode`, a shades restore and `/updateShadeConfig` already go through the web command queue to `loop()`. - `/controller` leaves out a room, shade, group or linked shade that doesn't fit in its chunk buffer, see [Oversized `/controller` items are left out](#oversized-controller-items-are-left-out). ## Oversized `/controller` items are left out @@ -91,40 +90,3 @@ Unlike the `/controller` HTTP crash, this path does **not** abort — there is n - Not the same code path as the `/controller` crash. Solve independently. - Worth grepping for `JsonSockEvent` usages that iterate collections (see references in `SomfyEvents.cpp`, `Transceiver.cpp`, `ESPNetwork.cpp`, `GitOTA.cpp`) to identify the most at-risk events. - -## Rolling codes written from two tasks - -**Status:** open -**Location:** `SomfyRemote::sendCommandAfter` and `setRollingCode` in [src/SomfyRemote.cpp](../src/SomfyRemote.cpp), the `/setRollingCode` handlers in [src/Web.cpp](../src/Web.cpp), and `ShadeConfigFile::readShadeRecord`/`readGroupRecord` in [src/ConfigFile.cpp](../src/ConfigFile.cpp). - -### Summary - -Sending a command reads the next rolling code, queues the frame, then commits the code (`peekNextRollingCode` → `submitTx` → `commitRollingCode`), all on `loop()`. `/setRollingCode` and a shades restore change the same remote's `lastRollingCode` and its `ShadeCodes` NVS key from the async web server task, with no lock against that sequence. Each NVS write is safe on its own, since every function opens its own handle (see [ADR-0001](adr/0001-local-nvs-handles-no-wrapper.md)), but the read-then-commit sequence is not. - -### Concrete failure modes - -1. `/setRollingCode` lands between the peek and the commit of a command. The command's commit overwrites the code just set, so the set has no effect. -2. A restore reads `ShadeCodes` while a command commits a newer code. The restored shade can come back with the older code, and the motor rejects its frames until the code passes the newer one. - -### Fix options (pick one later) - -- **(a) Run these handlers on `loop()`.** Queue `/setRollingCode` and restore through the web command queue like shade commands, so `loop()` is the only task that touches a remote's rolling code. Part of making `loop()` the only owner of the shade model. -- **(b) A rolling-code lock.** Hold a mutex across peek → queue → commit, and in `setRollingCode` and restore. Narrower, but it adds a lock to the command path. - -### Related - -- Same root cause as [Concurrent mutation during chunked `/controller` response](#concurrent-mutation-during-chunked-controller-response): the web server task changes the shade model that `loop()` owns. -- `/linkRemote` sets a linked remote's rolling code on the web server task, while `loop()` updates the same code when it receives a frame from that remote. The last write wins, so a received frame can undo the code just set. - -## `/setMyPosition` without a tilt uses the lift favorite - -**Status:** open -**Location:** the `/setMyPosition` handlers in [src/Web.cpp](../src/Web.cpp). This was already in the code before the radio task work. - -### Summary - -When a request for a shade with tilt leaves out `tilt`, the handler fills it in with `shade->myPos`, the lift favorite, where `shade->myTiltPos` looks intended. The favorite tilt is then set to the favorite lift position. The web UI always sends `tilt`, so only API callers that leave it out see this. - -### Fix - -Use `shade->myTiltPos` in both handlers, after checking that no API client depends on the current behaviour. diff --git a/src/ShadeMotion.cpp b/src/ShadeMotion.cpp index 54a8f75..6fd006b 100644 --- a/src/ShadeMotion.cpp +++ b/src/ShadeMotion.cpp @@ -29,6 +29,11 @@ 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)); } +int8_t requestedFavoriteTilt(int8_t requestTilt, tilt_types tilt, float myTiltPos, bool flip) { + if(tilt == tilt_types::none) return -1; + if(requestTilt >= 0) return requestTilt; + return displayPosition(myTiltPos, flip); +} float limitTarget(int8_t dir) { return dir > 0 ? 100.0f : 0.0f; } bool stepPosition(float pos, int8_t dir, uint32_t travelMs, uint16_t stepSize, uint8_t frameStepSize, float &out) { if(travelMs == 0 || stepSize == 0) return false; diff --git a/src/ShadeMotion.h b/src/ShadeMotion.h index 1fa10bd..0c801ab 100644 --- a/src/ShadeMotion.h +++ b/src/ShadeMotion.h @@ -14,6 +14,10 @@ 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 tilt a request to set the favorite asks for, as a display position: the +// requested tilt, or the favorite tilt when the request leaves it out. -1 for +// a shade without tilt or with no favorite tilt. +int8_t requestedFavoriteTilt(int8_t requestTilt, tilt_types tilt, float myTiltPos, bool flip); // Where a shade (or its tilt) moving in dir ends up when nothing stops it. float limitTarget(int8_t dir); // Where a step command moves a shade (or its tilt) from pos, in out. dir > 0 diff --git a/src/Web.cpp b/src/Web.cpp index 72a4052..f4d4025 100644 --- a/src/Web.cpp +++ b/src/Web.cpp @@ -51,12 +51,12 @@ static QueueHandle_t webCmdQueue = nullptr; static void sendQueueFull(AsyncWebServerRequest *request); static void sendRadioUnavailable(AsyncWebServerRequest *request); -// Hands a command that transmits to the main loop. When it can't be the -// request is answered with the reason and false is returned. -static bool queueRadioCommand(AsyncWebServerRequest *request, const web_command_t &cmd) { +// Hands a command to the main loop, which owns the shades. When it can't be +// the request is answered with the reason and false is returned. +static bool queueLoopCommand(AsyncWebServerRequest *request, const web_command_t &cmd) { // Nothing can go on air during a frequency scan so tell the client now rather // than dropping the command once it reaches the radio. - if(somfy.transceiver.txSuspended()) { sendRadioUnavailable(request); return false; } + if(webCommandUsesRadio(cmd.type) && somfy.transceiver.txSuspended()) { sendRadioUnavailable(request); return false; } if(!webServer.queueCommand(cmd)) { sendQueueFull(request); return false; } return true; } @@ -67,7 +67,7 @@ static bool queueSetMyPosition(AsyncWebServerRequest *request, SomfyShade *shade cmd.shadeId = shade->getShadeId(); cmd.position = pos; cmd.tiltPosition = tilt; - return queueRadioCommand(request, cmd); + return queueLoopCommand(request, cmd); } // The main loop clears paired and saves it once the Prog is queued. The // shade belongs to the main loop so it isn't changed here. @@ -75,7 +75,32 @@ static bool queueUnpairShade(AsyncWebServerRequest *request, SomfyShade *shade) web_command_t cmd = {}; cmd.type = web_cmd_t::unpair_shade; cmd.shadeId = shade->getShadeId(); - return queueRadioCommand(request, cmd); + return queueLoopCommand(request, cmd); +} +// Sending a command reads and commits the rolling code on the main loop, so +// a new code is set there too. +static bool queueSetRollingCode(AsyncWebServerRequest *request, SomfyShade *shade, uint16_t rollingCode) { + web_command_t cmd = {}; + cmd.type = web_cmd_t::set_rolling_code; + cmd.shadeId = shade->getShadeId(); + cmd.rollingCode = rollingCode; + return queueLoopCommand(request, cmd); +} +// A restore's options travel through the command queue as RESTORE_* bits. +static uint8_t restoreParts(const restore_options_t &opts) { + return (opts.settings ? RESTORE_SETTINGS : 0) | (opts.shades ? RESTORE_SHADES : 0) | + (opts.network ? RESTORE_NETWORK : 0) | (opts.transceiver ? RESTORE_TRANSCEIVER : 0) | + (opts.repeaters ? RESTORE_REPEATERS : 0) | (opts.mqtt ? RESTORE_MQTT : 0); +} +static restore_options_t restoreOptions(uint8_t parts) { + restore_options_t opts; + opts.settings = parts & RESTORE_SETTINGS; + opts.shades = parts & RESTORE_SHADES; + opts.network = parts & RESTORE_NETWORK; + opts.transceiver = parts & RESTORE_TRANSCEIVER; + opts.repeaters = parts & RESTORE_REPEATERS; + opts.mqtt = parts & RESTORE_MQTT; + return opts; } // The radio and MQTT are shut down once an upload starts and only a reboot // brings them back. The completion handler schedules it but never runs if the @@ -220,6 +245,22 @@ void Web::processQueue() { } break; } + case web_cmd_t::set_rolling_code: { + SomfyShade *shade = somfy.getShadeById(cmd.shadeId); + if(shade) shade->setRollingCode(cmd.rollingCode); + break; + } + case web_cmd_t::restore_shades: { + restore_options_t opts = restoreOptions(cmd.restoreParts); + ShadeConfigFile::restore(&somfy, "/shades.tmp", opts); + ESP_LOGI(TAG, "Rebooting ESP for restored settings..."); + rebootDelay.reboot = true; + rebootDelay.rebootTime = millis() + 1000; + break; + } + case web_cmd_t::load_shades: + somfy.loadShadesFile("/shades.tmp"); + break; } if(!sent) ESP_LOGW(TAG, "Radio didn't take web command %u", static_cast(cmd.type)); } @@ -310,10 +351,10 @@ static void serializeShadeRef(SomfyShade *shade, JsonFormatter &json) { json.addElem("hasLight", shade->hasLight()); json.addElem("repeats", shade->repeats); } -// paired is passed in for a response that reports the change the main loop -// is about to make. The unpair is held until it goes on air (holdWebCommand), -// so the answer comes true. -static void serializeShade(SomfyShade *shade, JsonFormatter &json, bool paired) { +// paired and lastRollingCode are passed in for a response that reports the +// change the main loop is about to make. An unpair is held until it goes on +// air (holdWebCommand), so the answer comes true. +static void serializeShade(SomfyShade *shade, JsonFormatter &json, bool paired, uint16_t lastRollingCode) { json.addElem("shadeId", shade->getShadeId()); json.addElem("roomId", shade->roomId); json.addElem("name", shade->name); @@ -321,7 +362,7 @@ static void serializeShade(SomfyShade *shade, JsonFormatter &json, bool paired) json.addElem("upTime", (uint32_t)shade->upTime); json.addElem("downTime", (uint32_t)shade->downTime); json.addElem("paired", paired); - json.addElem("lastRollingCode", (uint32_t)shade->lastRollingCode); + json.addElem("lastRollingCode", (uint32_t)lastRollingCode); json.addElem("position", shade->transformPosition(shade->currentPos)); json.addElem("tiltType", static_cast(shade->tiltType)); json.addElem("tiltPosition", shade->transformPosition(shade->currentTiltPos)); @@ -361,7 +402,7 @@ static void serializeShade(SomfyShade *shade, JsonFormatter &json, bool paired) } json.endArray(); } -static void serializeShade(SomfyShade *shade, JsonFormatter &json) { serializeShade(shade, json, shade->paired); } +static void serializeShade(SomfyShade *shade, JsonFormatter &json) { serializeShade(shade, json, shade->paired, shade->lastRollingCode); } static void serializeGroupRef(SomfyGroup *group, JsonFormatter &json) { group->updateFlags(); json.addElem("groupId", group->getGroupId()); @@ -890,7 +931,7 @@ void Web::handleShadeCommand(AsyncWebServerRequest *request, JsonVariant &json) cmd.command = command; cmd.repeat = repeat; cmd.stepSize = stepSize; - if(!queueRadioCommand(request, cmd)) return; + if(!queueLoopCommand(request, cmd)) return; AsyncJsonResp resp; resp.beginResponse(request, g_async_content, sizeof(g_async_content)); resp.beginObject(); @@ -931,7 +972,7 @@ void Web::handleGroupCommand(AsyncWebServerRequest *request, JsonVariant &json) cmd.command = command; cmd.repeat = repeat; cmd.stepSize = stepSize; - if(!queueRadioCommand(request, cmd)) return; + if(!queueLoopCommand(request, cmd)) return; AsyncJsonResp resp; resp.beginResponse(request, g_async_content, sizeof(g_async_content)); resp.beginObject(); @@ -968,7 +1009,7 @@ void Web::handleTiltCommand(AsyncWebServerRequest *request, JsonVariant &json) { cmd.shadeId = shadeId; cmd.target = target; cmd.command = command; - if(!queueRadioCommand(request, cmd)) return; + if(!queueLoopCommand(request, cmd)) return; AsyncJsonResp resp; resp.beginResponse(request, g_async_content, sizeof(g_async_content)); resp.beginObject(); @@ -1009,7 +1050,7 @@ void Web::handleRepeatCommand(AsyncWebServerRequest *request, JsonVariant &json) cmd.command = command; cmd.repeat = repeat; cmd.stepSize = stepSize; - if(!queueRadioCommand(request, cmd)) return; + if(!queueLoopCommand(request, cmd)) return; AsyncJsonResp resp; resp.beginResponse(request, g_async_content, sizeof(g_async_content)); resp.beginObject(); @@ -1027,7 +1068,7 @@ void Web::handleRepeatCommand(AsyncWebServerRequest *request, JsonVariant &json) cmd.command = command; cmd.repeat = repeat; cmd.stepSize = stepSize; - if(!queueRadioCommand(request, cmd)) return; + if(!queueLoopCommand(request, cmd)) return; AsyncJsonResp resp; resp.beginResponse(request, g_async_content, sizeof(g_async_content)); resp.beginObject(); @@ -1166,7 +1207,7 @@ void Web::handleSetSensor(AsyncWebServerRequest *request, JsonVariant &json) { cmd.sunny = sunny; cmd.windy = windy; cmd.repeat = repeat; - if(!queueRadioCommand(request, cmd)) return; + if(!queueLoopCommand(request, cmd)) return; // The main loop applies the command after we answer so this is the state // before it. Clients get the result from the shadeState socket event. AsyncJsonResp resp; @@ -1187,7 +1228,7 @@ void Web::handleSetSensor(AsyncWebServerRequest *request, JsonVariant &json) { cmd.sunny = sunny; cmd.windy = windy; cmd.repeat = repeat; - if(!queueRadioCommand(request, cmd)) return; + if(!queueLoopCommand(request, cmd)) return; // As above, this is the state before the command and groupState follows. AsyncJsonResp resp; resp.beginResponse(request, g_async_content, sizeof(g_async_content)); @@ -1449,7 +1490,6 @@ void Web::begin() { asyncServer.on("/restore", HTTP_POST, [](AsyncWebServerRequest *request) { if(webServer.uploadSuccess) { - request->send(200, _encoding_json, "{\"status\":\"Success\",\"desc\":\"Restoring Shade settings\"}"); restore_options_t opts; if(asyncHasParam(request, "data")) { String dataStr = asyncParam(request, "data"); @@ -1469,10 +1509,13 @@ void Web::begin() { ESP_LOGD(TAG, "No restore options sent. Using defaults..."); opts.shades = true; } - ShadeConfigFile::restore(&somfy, "/shades.tmp", opts); - ESP_LOGI(TAG, "Rebooting ESP for restored settings..."); - rebootDelay.reboot = true; - rebootDelay.rebootTime = millis() + 1000; + // The main loop restores, since it owns the shades and their rolling + // codes, and then reboots. + web_command_t cmd = {}; + cmd.type = web_cmd_t::restore_shades; + cmd.restoreParts = restoreParts(opts); + if(!queueLoopCommand(request, cmd)) return; + request->send(200, _encoding_json, "{\"status\":\"Success\",\"desc\":\"Restoring Shade settings\"}"); } }, [](AsyncWebServerRequest *request, String filename, size_t index, uint8_t *data, size_t len, bool final) { @@ -1733,8 +1776,7 @@ void Web::begin() { } SomfyShade* shade = somfy.getShadeById(shadeId); if(shade) { - if(tilt < 0) tilt = shade->myPos; - if(shade->tiltType == tilt_types::none) tilt = -1; + tilt = requestedFavoriteTilt(tilt, shade->tiltType, shade->myTiltPos, shade->flipPosition); if(pos >= 0 && pos <= 100) if(!queueSetMyPosition(request, shade, shade->transformPosition(pos), shade->transformPosition(tilt))) return; AsyncJsonResp resp; @@ -1760,8 +1802,7 @@ void Web::begin() { else { request->send(500, _encoding_json, F("{\"status\":\"ERROR\",\"desc\":\"No shade object supplied.\"}")); return; } SomfyShade* shade = somfy.getShadeById(shadeId); if(shade) { - if(tilt < 0) tilt = shade->myPos; - if(shade->tiltType == tilt_types::none) tilt = -1; + tilt = requestedFavoriteTilt(tilt, shade->tiltType, shade->myTiltPos, shade->flipPosition); if(pos >= 0 && pos <= 100) if(!queueSetMyPosition(request, shade, shade->transformPosition(pos), shade->transformPosition(tilt))) return; AsyncJsonResp resp; @@ -1792,11 +1833,11 @@ void Web::begin() { request->send(500, _encoding_json, F("{\"status\":\"ERROR\",\"desc\":\"Shade not found to set rolling code\"}")); } else { - shade->setRollingCode(rollingCode); + if(!queueSetRollingCode(request, shade, rollingCode)) return; AsyncJsonResp resp; resp.beginResponse(request, g_async_content, sizeof(g_async_content)); resp.beginObject(); - serializeShade(shade, resp); + serializeShade(shade, resp, shade->paired, rollingCode); resp.endObject(); resp.endResponse(); } @@ -1814,11 +1855,11 @@ void Web::begin() { request->send(500, _encoding_json, F("{\"status\":\"ERROR\",\"desc\":\"Shade not found to set rolling code\"}")); } else { - shade->setRollingCode(rollingCode); + if(!queueSetRollingCode(request, shade, rollingCode)) return; AsyncJsonResp resp; resp.beginResponse(request, g_async_content, sizeof(g_async_content)); resp.beginObject(); - serializeShade(shade, resp); + serializeShade(shade, resp, shade->paired, rollingCode); resp.endObject(); resp.endResponse(); } @@ -1892,7 +1933,7 @@ void Web::begin() { AsyncJsonResp resp; resp.beginResponse(request, g_async_content, sizeof(g_async_content)); resp.beginObject(); - serializeShade(shade, resp, false); + serializeShade(shade, resp, false, shade->lastRollingCode); resp.endObject(); resp.endResponse(); } @@ -1911,7 +1952,7 @@ void Web::begin() { AsyncJsonResp resp; resp.beginResponse(request, g_async_content, sizeof(g_async_content)); resp.beginObject(); - serializeShade(shade, resp, false); + serializeShade(shade, resp, false, shade->lastRollingCode); resp.endObject(); resp.endResponse(); } @@ -2284,7 +2325,10 @@ void Web::begin() { fup.close(); } if(final) { - somfy.loadShadesFile("/shades.tmp"); + // The main loop loads it, since it owns the shades. + web_command_t cmd = {}; + cmd.type = web_cmd_t::load_shades; + webServer.queueCommand(cmd); } }); diff --git a/src/WebCommand.h b/src/WebCommand.h index 982a4c8..580c0aa 100644 --- a/src/WebCommand.h +++ b/src/WebCommand.h @@ -22,8 +22,19 @@ enum class web_cmd_t : uint8_t { group_sensor, // group sensor command set_my_position, // shade setMyPosition (position/tiltPosition) unpair_shade, // shade unpair (Prog) and clear paired flag + set_rolling_code, // set a shade's rolling code + restore_shades, // restore from /shades.tmp, then reboot + load_shades, // replace the shades with /shades.tmp }; +// What a restore_shades command restores, as bits of restoreParts. +#define RESTORE_SETTINGS 0x01 +#define RESTORE_SHADES 0x02 +#define RESTORE_NETWORK 0x04 +#define RESTORE_TRANSCEIVER 0x08 +#define RESTORE_REPEATERS 0x10 +#define RESTORE_MQTT 0x20 + struct web_command_t { web_cmd_t type; uint8_t shadeId; @@ -36,10 +47,22 @@ struct web_command_t { int8_t tiltPosition; // for setPositions/tilt int8_t sunny; // for sensor int8_t windy; // for sensor + uint16_t rollingCode; // for set_rolling_code + uint8_t restoreParts; // for restore_shades: RESTORE_* bits }; -// Only setting positions never goes on air. -inline bool webCommandUsesRadio(web_cmd_t type) { return type != web_cmd_t::set_positions; } +// Whether the command sends anything on air. +inline bool webCommandUsesRadio(web_cmd_t type) { + switch(type) { + case web_cmd_t::set_positions: + case web_cmd_t::set_rolling_code: + case web_cmd_t::restore_shades: + case web_cmd_t::load_shades: + return false; + default: + return true; + } +} // Whether the command at the head of the queue waits for the radio. The // client was already told it was accepted so it stays queued until the radio // has room rather than have the radio drop it. While TX is suspended nothing diff --git a/test/test_motion/test_motion.cpp b/test/test_motion/test_motion.cpp index 6996a42..744147b 100644 --- a/test/test_motion/test_motion.cpp +++ b/test/test_motion/test_motion.cpp @@ -285,6 +285,25 @@ static void test_stepTargets_without_a_travel_time_or_step_size_is_ignored() { ASSERT_ACTION(StepAction::ignore, stepOf(tilt_types::none, DOWN, 100, 0, 1, 20000, 10000, 5000, 0).action); } +/********************************************************************* + * requestedFavoriteTilt + ********************************************************************/ +static void test_favorite_tilt_keeps_the_requested_tilt() { + TEST_ASSERT_EQUAL_INT8(25, requestedFavoriteTilt(25, tilt_types::integrated, 70.0f, false)); +} +static void test_favorite_tilt_defaults_to_the_favorite_tilt() { + // Not the lift favorite: that was the old default. + TEST_ASSERT_EQUAL_INT8(70, requestedFavoriteTilt(-1, tilt_types::tiltmotor, 70.0f, false)); + // The request is in display positions, so a flipped shade gets its favorite flipped. + TEST_ASSERT_EQUAL_INT8(30, requestedFavoriteTilt(-1, tilt_types::tiltmotor, 70.0f, true)); +} +static void test_favorite_tilt_without_a_favorite_is_unset() { + TEST_ASSERT_EQUAL_INT8(-1, requestedFavoriteTilt(-1, tilt_types::integrated, -1.0f, false)); +} +static void test_favorite_tilt_of_a_shade_without_tilt_is_unset() { + TEST_ASSERT_EQUAL_INT8(-1, requestedFavoriteTilt(25, tilt_types::none, 70.0f, false)); +} + int main(int argc, char **argv) { (void)argc; (void)argv; @@ -312,6 +331,10 @@ int main(int argc, char **argv) { RUN_TEST(test_stepTargets_integrated_at_both_ends_is_ignored); RUN_TEST(test_stepTargets_without_a_travel_time_or_step_size_is_ignored); RUN_TEST(test_displayPosition); + RUN_TEST(test_favorite_tilt_keeps_the_requested_tilt); + RUN_TEST(test_favorite_tilt_defaults_to_the_favorite_tilt); + RUN_TEST(test_favorite_tilt_without_a_favorite_is_unset); + RUN_TEST(test_favorite_tilt_of_a_shade_without_tilt_is_unset); RUN_TEST(test_flipCommand_swaps_directions); RUN_TEST(test_flipCommand_keeps_the_rest); RUN_TEST(test_txhold_idle_shade_is_held); diff --git a/test/test_webcmd/test_webcmd.cpp b/test/test_webcmd/test_webcmd.cpp index 7073ad2..40c86d3 100644 --- a/test/test_webcmd/test_webcmd.cpp +++ b/test/test_webcmd/test_webcmd.cpp @@ -10,17 +10,24 @@ static const web_cmd_t RADIO_COMMANDS[] = { void setUp() {} void tearDown() {} -static void test_only_set_positions_stays_off_the_radio() { +static const web_cmd_t LOOP_COMMANDS[] = { + web_cmd_t::set_positions, web_cmd_t::set_rolling_code, web_cmd_t::restore_shades, web_cmd_t::load_shades +}; + +static void test_only_loop_commands_stay_off_the_radio() { int offRadio = 0; - for(uint8_t t = 0; t <= static_cast(web_cmd_t::unpair_shade); t++) { + for(uint8_t t = 0; t <= static_cast(web_cmd_t::load_shades); t++) { if(!webCommandUsesRadio(static_cast(t))) offRadio++; } - TEST_ASSERT_EQUAL_INT(1, offRadio); - TEST_ASSERT_FALSE(webCommandUsesRadio(web_cmd_t::set_positions)); + TEST_ASSERT_EQUAL_INT(4, offRadio); + for(web_cmd_t t : LOOP_COMMANDS) TEST_ASSERT_FALSE(webCommandUsesRadio(t)); } -static void test_set_positions_is_never_held() { - TEST_ASSERT_FALSE(holdWebCommand(web_cmd_t::set_positions, false, false)); - TEST_ASSERT_FALSE(holdWebCommand(web_cmd_t::set_positions, true, false)); +static void test_loop_commands_are_never_held() { + // Nothing goes on air, so a full or suspended radio doesn't matter. + for(web_cmd_t t : LOOP_COMMANDS) { + TEST_ASSERT_FALSE(holdWebCommand(t, false, false)); + TEST_ASSERT_FALSE(holdWebCommand(t, true, false)); + } } static void test_radio_command_waits_for_room() { for(web_cmd_t t : RADIO_COMMANDS) { @@ -46,8 +53,8 @@ int main(int argc, char **argv) { (void)argc; (void)argv; UNITY_BEGIN(); - RUN_TEST(test_only_set_positions_stays_off_the_radio); - RUN_TEST(test_set_positions_is_never_held); + RUN_TEST(test_only_loop_commands_stay_off_the_radio); + RUN_TEST(test_loop_commands_are_never_held); RUN_TEST(test_radio_command_waits_for_room); RUN_TEST(test_commands_are_not_held_while_suspended); RUN_TEST(test_unpair_waits_out_a_suspend);