diff --git a/src/ModelEditor.cpp b/src/ModelEditor.cpp index d7eeaf2..922159b 100644 --- a/src/ModelEditor.cpp +++ b/src/ModelEditor.cpp @@ -31,10 +31,9 @@ edit_changes_t editChanges(model_edit_type_t type, const edit_result_t &result) case model_edit_type_t::unlink_repeater: c.repeaters = true; break; - case model_edit_type_t::delete_room: - case model_edit_type_t::delete_shade: - case model_edit_type_t::delete_group: - break; + case model_edit_type_t::delete_room: c.removedRoomId = result.id; break; + case model_edit_type_t::delete_shade: c.removedShadeId = result.id; break; + case model_edit_type_t::delete_group: c.removedGroupId = result.id; break; } return c; } diff --git a/src/ModelEditor.h b/src/ModelEditor.h index ad374d9..c0b13d9 100644 --- a/src/ModelEditor.h +++ b/src/ModelEditor.h @@ -64,6 +64,10 @@ struct edit_changes_t { uint8_t roomId = 0; sort_list_t order = sort_list_t::none; bool repeaters = false; + // A delete: the id of what was removed. + uint8_t removedShadeId = 0; + uint8_t removedGroupId = 0; + uint8_t removedRoomId = 0; }; edit_changes_t editChanges(model_edit_type_t type, const edit_result_t &result); diff --git a/src/ShadeController.cpp b/src/ShadeController.cpp index e331c00..7b67354 100644 --- a/src/ShadeController.cpp +++ b/src/ShadeController.cpp @@ -6,7 +6,6 @@ #include "ConfigFile.h" #include "ModelEditor.h" #include "ShadeChanges.h" -#include "outputs/ProtoSocket.h" static const char *TAG = "Somfy"; @@ -291,7 +290,6 @@ bool SomfyShadeController::deleteShade(uint8_t shadeId) { for(uint8_t i = 0; i < SOMFY_MAX_SHADES; i++) { if(this->shades[i].getShadeId() == shadeId) { shades[i].emitState("shadeRemoved"); - protoSocket.shadeRemoved(shadeId); shades[i].unpublish(); this->shades[i].clear(); } @@ -322,7 +320,6 @@ bool SomfyShadeController::deleteRoom(uint8_t roomId) { } } rooms[i].emitState("roomRemoved"); - protoSocket.roomRemoved(roomId); this->rooms[i].clear(); } } @@ -338,7 +335,6 @@ bool SomfyShadeController::deleteGroup(uint8_t groupId) { if(shade) shade->configChanged(); } groups[i].emitState("groupRemoved"); - protoSocket.groupRemoved(groupId); groups[i].unpublish(); this->groups[i].clear(); } diff --git a/src/SomfyJson.cpp b/src/SomfyJson.cpp index a1d5f52..87f04b2 100644 --- a/src/SomfyJson.cpp +++ b/src/SomfyJson.cpp @@ -262,6 +262,9 @@ static void markEdited(const model_edit_t &edit, const edit_result_t &result) { if(room) protoSocket.room(*room); if(c.order != sort_list_t::none) protoSocket.order(c.order); if(c.repeaters) protoSocket.repeaters(); + if(c.removedShadeId) protoSocket.shadeRemoved(c.removedShadeId); + if(c.removedGroupId) protoSocket.groupRemoved(c.removedGroupId); + if(c.removedRoomId) protoSocket.roomRemoved(c.removedRoomId); } void runModelEdit(model_edit_t *edit) { const edit_result_t result = applyModelEdit(*edit); diff --git a/test/e2e/test_settings.py b/test/e2e/test_settings.py index 0f5dc04..414cdeb 100644 --- a/test/e2e/test_settings.py +++ b/test/e2e/test_settings.py @@ -118,10 +118,14 @@ def test_a_frequency_scan_starts_and_ends(connect, schema): def test_reboot(request, host, token, connect, schema): if not request.config.getoption("--reboot"): pytest.skip("reboots the device: pass --reboot") - c, _, _ = synced_model(connect, schema) + c = connect(schema=schema.hash) + _, frames, _ = run_sync(c) + uptime_before = uptime(schema, frames) assert is_ok(schema, op(c, schema, "reboot", {})) assert c.closed.wait(15), "the socket stayed open" - # Back once it answers a connect with hello again. + # The device closes its sockets just before it restarts, and may still + # take a connection in between: that one never gets its hello. + time.sleep(3) end = time.time() + 90 while True: assert time.time() < end, "the device didn't come back" @@ -132,9 +136,19 @@ def test_reboot(request, host, token, connect, schema): continue try: if back.status == 101: - hello, _, _ = run_sync(back) + hello, frames, _ = run_sync(back, timeout=10) assert hello["d"]["sh"] == schema.hash + assert uptime(schema, frames) < uptime_before, "the device didn't restart" return + except TimeoutError: + pass # not back yet finally: back.close() time.sleep(2) + + +def uptime(schema, frames): + """The uptime, in ms, from the status a sync ends with.""" + key = schema.keys["status"] + status = next(f["d"][key] for f in frames if key in f["d"]) + return schema.expand(status)["uptime"] diff --git a/test/fixtures/proto2/sync.json b/test/fixtures/proto2/sync.json index 14f4567..86fecdc 100644 --- a/test/fixtures/proto2/sync.json +++ b/test/fixtures/proto2/sync.json @@ -1515,13 +1515,15 @@ } ], "Z": { - "fr": 148772, + "fr": 148760, "mx": 77812, - "mn": 99332, + "mn": 102424, "to": 294616, - "up": 24633, + "up": 30144, + "lp": 2862, + "lx": 106737, "sd": "ssid 72", - "sg": -51, + "sg": -52, "ch": 6 } } diff --git a/test/test_modeleditor/test_modeleditor.cpp b/test/test_modeleditor/test_modeleditor.cpp index 3164b9f..5e7f989 100644 --- a/test/test_modeleditor/test_modeleditor.cpp +++ b/test/test_modeleditor/test_modeleditor.cpp @@ -205,16 +205,30 @@ static void test_a_repeater_edit_sends_the_repeaters() { TEST_ASSERT_TRUE(editChanges(model_edit_type_t::unlink_repeater, resultFor(0)).repeaters); TEST_ASSERT_FALSE(editChanges(model_edit_type_t::save_room, resultFor(1)).repeaters); } -static void test_deletes_send_nothing_more() { - // Their removals go out as they happen. - for(model_edit_type_t t : {model_edit_type_t::delete_room, model_edit_type_t::delete_shade, model_edit_type_t::delete_group}) { - const edit_changes_t c = editChanges(t, resultFor(4)); +static void test_deletes_send_their_removal() { + // After the reply, as every edit's events are, so a client waiting for the + // reply doesn't read past it. + const edit_changes_t room = editChanges(model_edit_type_t::delete_room, resultFor(4)); + const edit_changes_t shade = editChanges(model_edit_type_t::delete_shade, resultFor(5)); + const edit_changes_t group = editChanges(model_edit_type_t::delete_group, resultFor(6)); + TEST_ASSERT_EQUAL_UINT8(4, room.removedRoomId); + TEST_ASSERT_EQUAL_UINT8(5, shade.removedShadeId); + TEST_ASSERT_EQUAL_UINT8(6, group.removedGroupId); + for(const edit_changes_t &c : {room, shade, group}) { + // Nothing is left of it to send whole. TEST_ASSERT_EQUAL_UINT8(0, c.shadeId); TEST_ASSERT_EQUAL_UINT8(0, c.groupId); TEST_ASSERT_EQUAL_UINT8(0, c.roomId); TEST_ASSERT_EQUAL(sort_list_t::none, c.order); TEST_ASSERT_FALSE(c.repeaters); } + TEST_ASSERT_EQUAL_UINT8(0, room.removedShadeId + room.removedGroupId); + TEST_ASSERT_EQUAL_UINT8(0, shade.removedRoomId + shade.removedGroupId); + TEST_ASSERT_EQUAL_UINT8(0, group.removedRoomId + group.removedShadeId); +} +static void test_other_edits_remove_nothing() { + const edit_changes_t c = editChanges(model_edit_type_t::save_room, resultFor(4)); + TEST_ASSERT_EQUAL_UINT8(0, c.removedRoomId + c.removedShadeId + c.removedGroupId); } int main(int argc, char **argv) { @@ -249,6 +263,7 @@ int main(int argc, char **argv) { RUN_TEST(test_a_room_edit_sends_the_room); RUN_TEST(test_a_sort_edit_sends_its_list); RUN_TEST(test_a_repeater_edit_sends_the_repeaters); - RUN_TEST(test_deletes_send_nothing_more); + RUN_TEST(test_deletes_send_their_removal); + RUN_TEST(test_other_edits_remove_nothing); return UNITY_END(); }