feat: edit ops and the reads their forms need, for socket protocol 2

The edit routes become ops 26-44 (addRoom ... groupSortOrder): ProtoEdits
turns an op's payload into the route's request, full names and real
booleans, and loop() applies it with applyModelEdit() as the route
does. The reply goes to the client that sent it, with an add's new id,
or the route's reason and a shade's validation code; the HTTP routes
keep sending requestResult.

getNextRoom, getNextShade, getNextGroup and groupOptions (45-48) are
answered by loop() from the shade model, through a new LoopQueue item.

Messages that arrive in pieces, a frame over one TCP segment or several
frames, are put back together by ProtoAssembly: one at a time across
clients, up to 4 KB, in a buffer held only while one is on its way.
A message in pieces is now told from its first frame's opcode, which
also stops a fragmented message's last frame drawing a second reply.

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

test/e2e/README.md

@@ -11,8 +11,9 @@ Runs against a real device on your network, by hand, never in CI. It checks sock
 | `--device` | the device's address (required) |
 | `--password`, `--username` / `--pin` | the credentials of a Secured device |
 | `--radio --shade <id>` | also send real commands: that shade moves a little and back |
+| `--edit` | also add a room and delete it again |
 | `--record` | save hello, the sync and the schema to `test/fixtures/proto2/`, for the web UI and Home Assistant tests |
 
-A plain run sends nothing on air, changes no settings and doesn't reboot the device. The command tests send ops that are refused before they reach the radio, and one `setPositions` that sets a shade to the position the sync gave: that drops the fraction of a percent the shade's believed position may have had. It does fill every socket slot for a few seconds, and replays a page load.
+A plain run sends nothing on air, changes no settings and doesn't reboot the device. The command tests send ops that are refused before they reach the radio, and one `setPositions` that sets a shade to the position the sync gave: that drops the fraction of a percent the shade's believed position may have had. The edit tests send edits that are refused, and one shade sort order set to the order it has, which saves the shades again unchanged. It does fill every socket slot for a few seconds, and replays a page load.
 
 The tests run in whatever security mode the device is in; the suite never changes it. To test both modes, run the suite once in each.

test/e2e/conftest.py

@@ -17,6 +17,7 @@ def pytest_addoption(parser):
     parser.addoption("--record", action="store_true", help="save what the device sends to test/fixtures/proto2")
     parser.addoption("--radio", action="store_true", help="also run the tests that send commands on air")
     parser.addoption("--shade", type=int, default=None, help="with --radio: the id of the shade the commands may move")
+    parser.addoption("--edit", action="store_true", help="also run the edits that change the shade model, then undo them")
 
 
 @pytest.fixture(scope="session")
@@ -95,3 +96,11 @@ def radio_shade(request):
     if shade is None:
         pytest.skip("--radio needs --shade <id>, a shade that may move")
     return shade
+
+
+@pytest.fixture(scope="session")
+def editing(request):
+    """For tests that change the shade model and change it back, or a skip."""
+    if not request.config.getoption("--edit"):
+        pytest.skip("changes the shade model and back: pass --edit")
+    return True

test/e2e/test_edits.py

@@ -0,0 +1,115 @@
+"""The edit ops and the reads an edit form needs (docs/PROTOCOL.md).  Without
+--edit nothing changes: edits are refused, or set what is already there."""
+import pytest
+
+from test_commands import is_ok, op, pack, synced_model
+from test_messages import error_name, reply_to
+
+EDIT_OPS = ("addRoom", "addShade", "addGroup", "saveRoom", "saveShade", "saveGroup", "deleteRoom", "deleteShade",
+            "deleteGroup", "linkRemote", "unlinkRemote", "linkToGroup", "unlinkFromGroup", "linkRepeater",
+            "unlinkRepeater", "setPaired", "roomSortOrder", "shadeSortOrder", "groupSortOrder")
+READ_OPS = ("getNextRoom", "getNextShade", "getNextGroup", "groupOptions")
+
+
+def field(schema, reply, name):
+    return reply["d"].get(schema.keys[name], schema.default(name))
+
+
+def unused(items, key):
+    return next(i for i in range(250, 0, -1) if i not in {x[key] for x in items})
+
+
+def test_every_edit_and_read_op_is_in_the_schema(schema):
+    for name in EDIT_OPS + READ_OPS:
+        assert 16 <= schema.type_numbers[name] <= 127, name
+
+
+def test_the_next_shade_gets_a_free_id_and_address(connect, schema):
+    c, shades, groups = synced_model(connect, schema)
+    reply = op(c, schema, "getNextShade", {})
+    assert is_ok(schema, reply)
+    assert field(schema, reply, "shadeId") not in {s["shadeId"] for s in shades}
+    address = field(schema, reply, "remoteAddress")
+    assert address and address not in {s.get("remoteAddress") for s in shades + groups}
+
+
+def test_the_next_room_and_group(connect, schema):
+    c, _, groups = synced_model(connect, schema)
+    assert is_ok(schema, op(c, schema, "getNextRoom", {}))
+    reply = op(c, schema, "getNextGroup", {})
+    assert is_ok(schema, reply)
+    assert field(schema, reply, "groupId") not in {g["groupId"] for g in groups}
+
+
+def test_group_options_are_the_shades_it_lacks(connect, schema):
+    c, shades, groups = synced_model(connect, schema)
+    if not groups:
+        pytest.skip("the device has no groups")
+    group = groups[0]
+    reply = op(c, schema, "groupOptions", {"groupId": group["groupId"]})
+    assert is_ok(schema, reply)
+    expected = {s["shadeId"] for s in shades} - set(group.get("linkedShades", []))
+    assert set(field(schema, reply, "ids")) == expected
+
+
+def test_group_options_of_a_missing_group(connect, schema):
+    c, _, groups = synced_model(connect, schema)
+    reply = op(c, schema, "groupOptions", {"groupId": unused(groups, "groupId")})
+    assert error_name(schema, reply) == "notFound"
+
+
+@pytest.mark.parametrize("name, payload, why", [
+    ("saveRoom", {"zz": 1}, "Unknown field zz"),
+    ("setPaired", {"s": 1, "i": 2}, "paired is not a boolean (1 or 0)"),
+    ("shadeSortOrder", {}, "ids is missing"),
+])
+def test_a_bad_payload_is_refused_before_it_is_queued(connect, schema, name, payload, why):
+    c, _, _ = synced_model(connect, schema)
+    msg_id = c.send(schema.type_numbers[name], payload, with_id=True)
+    reply = reply_to(c, msg_id)
+    assert error_name(schema, reply) == "badRequest"
+    assert reply["d"][schema.keys["desc"]] == why
+
+
+def test_an_edit_of_a_missing_shade_says_why(connect, schema):
+    """Refused by the edit itself on loop(), as its route would be."""
+    c, shades, _ = synced_model(connect, schema)
+    reply = op(c, schema, "saveShade", {"shadeId": unused(shades, "shadeId"), "name": "x"})
+    assert error_name(schema, reply) == "badRequest"
+    assert reply["d"][schema.keys["desc"]] == "Shade Id not found."
+
+
+def test_a_sort_order_set_to_what_it_is(connect, schema):
+    c, shades, _ = synced_model(connect, schema)
+    if not shades:
+        pytest.skip("the device has no shades")
+    ids = [s["shadeId"] for s in sorted(shades, key=lambda s: s.get("sortOrder", 0))]
+    reply = op(c, schema, "shadeSortOrder", {"ids": ids})
+    assert is_ok(schema, reply)
+    order = c.recv_until(lambda f: f["t"] == schema.type_numbers["shadeOrder"])[-1]
+    assert order["d"][schema.keys["ids"]] == ids
+
+
+def test_a_long_edit_is_put_back_together(connect, schema):
+    """3 KB arrives in pieces: the reply carries its id, so it was read whole."""
+    c, shades, _ = synced_model(connect, schema)
+    padding = " " * 3000
+    msg_id = 4242
+    c.send_text('{"v":2,"t":%d,"id":%d,"d":{"%s":%d%s}}' % (
+        schema.type_numbers["saveShade"], msg_id, schema.keys["shadeId"], unused(shades, "shadeId"), padding))
+    reply = reply_to(c, msg_id)
+    assert reply["d"][schema.keys["desc"]] == "Shade Id not found."
+
+
+def test_adding_and_deleting_a_room(connect, schema, editing):
+    """With --edit: a room is added, then deleted again."""
+    c, _, _ = synced_model(connect, schema)
+    reply = op(c, schema, "addRoom", {"name": "e2e room"})
+    assert is_ok(schema, reply), reply
+    room_id = field(schema, reply, "roomId")
+    assert room_id
+    added = c.recv_until(lambda f: f["t"] == schema.type_numbers["room"])[-1]
+    assert schema.expand(added["d"])["name"] == "e2e room"
+    assert is_ok(schema, op(c, schema, "deleteRoom", {"roomId": room_id}))
+    removed = c.recv_until(lambda f: f["t"] == schema.type_numbers["roomRemoved"])[-1]
+    assert removed["d"][schema.keys["roomId"]] == room_id

test/test_loopqueue/test_loopqueue.cpp

@@ -20,7 +20,9 @@ static const shade_request_type_t RADIO_REQUESTS[] = {
 static const shade_request_type_t OFF_RADIO_REQUESTS[] = {
   shade_request_type_t::set_positions, shade_request_type_t::set_rolling_code
 };
-static const loop_item_type_t OTHER_ITEMS[] = { loop_item_type_t::restore_shades, loop_item_type_t::load_shades };
+static const loop_item_type_t OTHER_ITEMS[] = {
+  loop_item_type_t::restore_shades, loop_item_type_t::load_shades, loop_item_type_t::model_edit, loop_item_type_t::proto_read
+};
 
 void setUp() {}
 void tearDown() {}

test/test_protoassembly/test_protoassembly.cpp

@@ -0,0 +1,100 @@
+#include <unity.h>
+#include <string.h>
+#include "ProtoAssembly.h"
+
+// Putting protocol 2 messages back together from their pieces: one message at
+// a time, one reply-worthy result per message.
+
+void setUp() {}
+void tearDown() {}
+
+static proto_piece_t piece(uint32_t client, const char *text, bool first, bool last, bool isText = true) {
+  proto_piece_t p;
+  p.clientId = client;
+  p.first = first;
+  p.last = last;
+  p.text = isText;
+  p.data = reinterpret_cast<const uint8_t *>(text);
+  p.len = strlen(text);
+  return p;
+}
+
+static void test_pieces_make_the_message() {
+  MessageAssembler a(64);
+  TEST_ASSERT_EQUAL(assembly_t::more, a.add(piece(1, "{\"v\":2,", true, false), 0));
+  TEST_ASSERT_EQUAL(assembly_t::more, a.add(piece(1, "\"t\":3", false, false), 0));
+  TEST_ASSERT_EQUAL(assembly_t::complete, a.add(piece(1, "}", false, true), 0));
+  TEST_ASSERT_EQUAL_STRING("{\"v\":2,\"t\":3}", a.text());
+  TEST_ASSERT_EQUAL_UINT32(13, a.length());
+  a.done();
+  TEST_ASSERT_FALSE(a.holding());
+}
+static void test_nothing_is_held_between_messages() {
+  MessageAssembler a(64);
+  TEST_ASSERT_FALSE(a.holding());
+  a.add(piece(1, "ab", true, false), 0);
+  TEST_ASSERT_TRUE(a.holding());
+  a.add(piece(1, "c", false, true), 0);
+  a.done();
+  TEST_ASSERT_FALSE(a.holding());
+  TEST_ASSERT_NULL(a.text());
+}
+static void test_another_clients_message_waits() {
+  MessageAssembler a(64);
+  a.add(piece(1, "ab", true, false), 0);
+  // One busy for the other client's message, then its pieces are let go.
+  TEST_ASSERT_EQUAL(assembly_t::busy, a.add(piece(2, "xy", true, false), 10));
+  TEST_ASSERT_EQUAL(assembly_t::ignored, a.add(piece(2, "z", false, true), 10));
+  TEST_ASSERT_EQUAL(assembly_t::complete, a.add(piece(1, "c", false, true), 20));
+  TEST_ASSERT_EQUAL_STRING("abc", a.text());
+}
+static void test_too_large_is_said_once() {
+  MessageAssembler a(4);
+  TEST_ASSERT_EQUAL(assembly_t::more, a.add(piece(1, "abc", true, false), 0));
+  TEST_ASSERT_EQUAL(assembly_t::tooLarge, a.add(piece(1, "de", false, false), 0));
+  TEST_ASSERT_EQUAL(assembly_t::ignored, a.add(piece(1, "f", false, true), 0));
+  TEST_ASSERT_FALSE(a.holding());
+}
+static void test_a_binary_message_is_refused_once() {
+  MessageAssembler a(64);
+  TEST_ASSERT_EQUAL(assembly_t::notText, a.add(piece(1, "ab", true, false, false), 0));
+  TEST_ASSERT_EQUAL(assembly_t::ignored, a.add(piece(1, "c", false, true, false), 0));
+}
+static void test_a_stalled_message_gives_the_buffer_up() {
+  MessageAssembler a(64);
+  a.add(piece(1, "ab", true, false), 0);
+  TEST_ASSERT_EQUAL(assembly_t::busy, a.add(piece(2, "x", true, true), PROTO_ASSEMBLY_TIMEOUT_MS - 1));
+  TEST_ASSERT_EQUAL(assembly_t::complete, a.add(piece(2, "x", true, true), PROTO_ASSEMBLY_TIMEOUT_MS));
+  a.done();
+  // The stalled client is told once, when its next piece comes.
+  TEST_ASSERT_EQUAL(assembly_t::timedOut, a.add(piece(1, "c", false, false), PROTO_ASSEMBLY_TIMEOUT_MS + 1));
+  TEST_ASSERT_EQUAL(assembly_t::ignored, a.add(piece(1, "d", false, true), PROTO_ASSEMBLY_TIMEOUT_MS + 1));
+}
+static void test_a_client_that_leaves_lets_go() {
+  MessageAssembler a(64);
+  a.add(piece(1, "ab", true, false), 0);
+  a.drop(1);
+  TEST_ASSERT_FALSE(a.holding());
+  TEST_ASSERT_EQUAL(assembly_t::complete, a.add(piece(2, "xy", true, true), 1));
+}
+static void test_a_message_of_one_piece_is_whole() {
+  // A whole binary frame, or one piece too large for the rest of the code.
+  MessageAssembler a(64);
+  TEST_ASSERT_EQUAL(assembly_t::complete, a.add(piece(1, "{}", true, true), 0));
+  TEST_ASSERT_EQUAL_STRING("{}", a.text());
+}
+
+int main(int argc, char **argv) {
+  (void)argc;
+  (void)argv;
+  UNITY_BEGIN();
+  RUN_TEST(test_pieces_make_the_message);
+  RUN_TEST(test_nothing_is_held_between_messages);
+  RUN_TEST(test_another_clients_message_waits);
+  RUN_TEST(test_too_large_is_said_once);
+  RUN_TEST(test_a_binary_message_is_refused_once);
+  RUN_TEST(test_a_stalled_message_gives_the_buffer_up);
+  RUN_TEST(test_a_client_that_leaves_lets_go);
+  RUN_TEST(test_a_message_of_one_piece_is_whole);
+  return UNITY_END();
+}

test/test_protoedits/test_protoedits.cpp

@@ -0,0 +1,109 @@
+#include <unity.h>
+#include <string>
+#include <ArduinoJson.h>
+#include "ProtoEdits.h"
+
+// Socket protocol 2's edit ops: which edit each is, and the edit route's
+// request that its payload becomes (docs/PROTOCOL.md).
+
+void setUp() {}
+void tearDown() {}
+
+static JsonDocument payload;
+static JsonDocument doc;
+
+static const char *expand(model_edit_type_t type, const char *json) {
+  payload.clear();
+  TEST_ASSERT_EQUAL(DeserializationError::Ok, deserializeJson(payload, json).code());
+  return expandEditPayload(type, payload.as<JsonObjectConst>(), doc);
+}
+static std::string written() {
+  std::string out;
+  serializeJson(doc, out);
+  return out;
+}
+#define TEST_ASSERT_GOOD(why) TEST_ASSERT_NULL_MESSAGE(why, why)
+
+/*********************************************************************
+ * Which edit an op is
+ ********************************************************************/
+static void test_every_edit_op_is_an_edit() {
+  model_edit_type_t t;
+  TEST_ASSERT_TRUE(protoEditType(static_cast<uint8_t>(proto_type_t::addShade), t));
+  TEST_ASSERT_EQUAL(model_edit_type_t::add_shade, t);
+  TEST_ASSERT_TRUE(protoEditType(static_cast<uint8_t>(proto_type_t::groupSortOrder), t));
+  TEST_ASSERT_EQUAL(model_edit_type_t::group_order, t);
+  TEST_ASSERT_TRUE(protoEditType(static_cast<uint8_t>(proto_type_t::setPaired), t));
+  TEST_ASSERT_EQUAL(model_edit_type_t::set_paired, t);
+  // One op for each edit.
+  int count = 0;
+  for(int type = 16; type <= 127; type++)
+    if(protoEditType(static_cast<uint8_t>(type), t)) count++;
+  TEST_ASSERT_EQUAL_INT(static_cast<int>(model_edit_type_t::group_order) + 1, count);
+}
+static void test_other_types_are_not_edits() {
+  model_edit_type_t t;
+  TEST_ASSERT_FALSE(protoEditType(static_cast<uint8_t>(proto_type_t::shadeCommand), t));
+  TEST_ASSERT_FALSE(protoEditType(static_cast<uint8_t>(proto_type_t::getNextShade), t));
+  TEST_ASSERT_FALSE(protoEditType(static_cast<uint8_t>(proto_type_t::shade), t));
+}
+static void test_the_read_ops() {
+  TEST_ASSERT_TRUE(protoIsReadOp(static_cast<uint8_t>(proto_type_t::getNextRoom)));
+  TEST_ASSERT_TRUE(protoIsReadOp(static_cast<uint8_t>(proto_type_t::getNextShade)));
+  TEST_ASSERT_TRUE(protoIsReadOp(static_cast<uint8_t>(proto_type_t::getNextGroup)));
+  TEST_ASSERT_TRUE(protoIsReadOp(static_cast<uint8_t>(proto_type_t::groupOptions)));
+  TEST_ASSERT_FALSE(protoIsReadOp(static_cast<uint8_t>(proto_type_t::addRoom)));
+}
+
+/*********************************************************************
+ * The payload as the edit route's request
+ ********************************************************************/
+static void test_short_keys_become_the_route_names() {
+  TEST_ASSERT_GOOD(expand(model_edit_type_t::save_shade, R"({"s":3,"n":"Office","U":12000,"a":1193046})"));
+  TEST_ASSERT_EQUAL_STRING(R"({"shadeId":3,"name":"Office","upTime":12000,"remoteAddress":1193046})", written().c_str());
+}
+static void test_booleans_become_true_and_false() {
+  // set_paired reads paired | false, which takes only a boolean.
+  TEST_ASSERT_GOOD(expand(model_edit_type_t::set_paired, R"({"s":3,"i":1})"));
+  TEST_ASSERT_EQUAL_STRING(R"({"shadeId":3,"paired":true})", written().c_str());
+  TEST_ASSERT_GOOD(expand(model_edit_type_t::save_shade, R"({"s":3,"fp":0})"));
+  TEST_ASSERT_EQUAL_STRING(R"({"shadeId":3,"flipPosition":false})", written().c_str());
+}
+static void test_only_what_was_sent_is_in_the_request() {
+  TEST_ASSERT_GOOD(expand(model_edit_type_t::save_room, R"({"r":2})"));
+  TEST_ASSERT_EQUAL_STRING(R"({"roomId":2})", written().c_str());
+  TEST_ASSERT_GOOD(expand(model_edit_type_t::add_room, R"({})"));
+  TEST_ASSERT_EQUAL_STRING("{}", written().c_str());
+}
+static void test_a_sort_order_is_its_list_of_ids() {
+  TEST_ASSERT_GOOD(expand(model_edit_type_t::shade_order, R"({"is":[3,1,2]})"));
+  TEST_ASSERT_EQUAL_STRING("[3,1,2]", written().c_str());
+  TEST_ASSERT_EQUAL_STRING("ids is missing", expand(model_edit_type_t::room_order, R"({})"));
+  TEST_ASSERT_EQUAL_STRING("ids must be a list of ids", expand(model_edit_type_t::group_order, R"({"is":[1,"x"]})"));
+  TEST_ASSERT_EQUAL_STRING("ids must be a list of ids", expand(model_edit_type_t::group_order, R"({"is":[1,300]})"));
+}
+static void test_values_are_checked_against_their_kind() {
+  TEST_ASSERT_EQUAL_STRING("paired is not a boolean (1 or 0)", expand(model_edit_type_t::set_paired, R"({"s":3,"i":true})"));
+  TEST_ASSERT_EQUAL_STRING("paired is not a boolean (1 or 0)", expand(model_edit_type_t::set_paired, R"({"s":3,"i":2})"));
+  TEST_ASSERT_EQUAL_STRING("shadeId is not a whole number", expand(model_edit_type_t::delete_shade, R"({"s":"3"})"));
+  TEST_ASSERT_EQUAL_STRING("name is not a string", expand(model_edit_type_t::save_room, R"({"r":1,"n":5})"));
+}
+static void test_an_unknown_key_is_refused() {
+  TEST_ASSERT_EQUAL_STRING("Unknown field zz", expand(model_edit_type_t::save_room, R"({"r":1,"zz":5})"));
+}
+
+int main(int argc, char **argv) {
+  (void)argc;
+  (void)argv;
+  UNITY_BEGIN();
+  RUN_TEST(test_every_edit_op_is_an_edit);
+  RUN_TEST(test_other_types_are_not_edits);
+  RUN_TEST(test_the_read_ops);
+  RUN_TEST(test_short_keys_become_the_route_names);
+  RUN_TEST(test_booleans_become_true_and_false);
+  RUN_TEST(test_only_what_was_sent_is_in_the_request);
+  RUN_TEST(test_a_sort_order_is_its_list_of_ids);
+  RUN_TEST(test_values_are_checked_against_their_kind);
+  RUN_TEST(test_an_unknown_key_is_refused);
+  return UNITY_END();
+}