From e4ac7a29c64e38f49aa403939a87cd2c521b6f23 Mon Sep 17 00:00:00 2001 From: cjkas Date: Sat, 17 Oct 2026 23:03:16 +0200 Subject: [PATCH] fix: give out ids up to the maximum New ids stopped two short of the limits, at 30 shades, 14 groups and 14 rooms instead of 32, 16 and 16, while the web UI showed the full limits. The original loop ran i < MAX - 1, an off-by-one: nothing indexes by id or treats the last two ids as special. ModelSnapshot's next-id answers and SomfyShadeController's allocation now run from 1 to the maximum. Found during LOOP_QUEUE_PLAN.md's review. Co-Authored-By: Claude Opus 5.5 --- src/ModelSnapshot.cpp | 6 +++--- src/ShadeController.cpp | 8 ++++---- test/test_snapshot/test_snapshot.cpp | 11 ++++++++++- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/ModelSnapshot.cpp b/src/ModelSnapshot.cpp index ec56cd5..d40abae 100644 --- a/src/ModelSnapshot.cpp +++ b/src/ModelSnapshot.cpp @@ -126,18 +126,18 @@ void writeGroup(JsonFormatter &json, const group_snapshot_t &group, shade_lookup uint8_t nextRoomId(const model_snapshot_t &snap) { uint8_t ids[SOMFY_MAX_ROOMS]; for(uint8_t i = 0; i < snap.roomCount; i++) ids[i] = snap.rooms[i].roomId; - return firstFreeId(ids, snap.roomCount, SOMFY_MAX_ROOMS - 2); + return firstFreeId(ids, snap.roomCount, SOMFY_MAX_ROOMS); } uint8_t nextShadeId(const model_snapshot_t &snap) { uint8_t ids[SOMFY_MAX_SHADES]; for(uint8_t i = 0; i < snap.shadeCount; i++) ids[i] = snap.shades[i].shadeId; - const uint8_t id = firstFreeId(ids, snap.shadeCount, SOMFY_MAX_SHADES - 2); + const uint8_t id = firstFreeId(ids, snap.shadeCount, SOMFY_MAX_SHADES); return id == 0 ? 255 : id; } uint8_t nextGroupId(const model_snapshot_t &snap) { uint8_t ids[SOMFY_MAX_GROUPS]; for(uint8_t i = 0; i < snap.groupCount; i++) ids[i] = snap.groups[i].groupId; - const uint8_t id = firstFreeId(ids, snap.groupCount, SOMFY_MAX_GROUPS - 2); + const uint8_t id = firstFreeId(ids, snap.groupCount, SOMFY_MAX_GROUPS); return id == 0 ? 255 : id; } uint32_t nextRemoteAddress(const model_snapshot_t &snap, uint8_t id) { diff --git a/src/ShadeController.cpp b/src/ShadeController.cpp index 86fdd03..8d35053 100644 --- a/src/ShadeController.cpp +++ b/src/ShadeController.cpp @@ -149,11 +149,11 @@ void SomfyShadeController::processWaitingFrame() { if(this->shades[i].getShadeId() != 255) this->shades[i].processWaitingFrame(); } // A deleted shade, group or room leaves a gap, so a new one takes the lowest -// free id. Ids stop two short of the maximum, as they always have. +// free id, from 1 up to the maximum. uint8_t SomfyShadeController::getNextShadeId() { uint8_t ids[SOMFY_MAX_SHADES]; for(uint8_t i = 0; i < SOMFY_MAX_SHADES; i++) ids[i] = this->shades[i].getShadeId(); - const uint8_t id = firstFreeId(ids, SOMFY_MAX_SHADES, SOMFY_MAX_SHADES - 2); + const uint8_t id = firstFreeId(ids, SOMFY_MAX_SHADES, SOMFY_MAX_SHADES); return id == 0 ? 255 : id; } int8_t SomfyShadeController::getMaxShadeOrder() { @@ -173,13 +173,13 @@ int8_t SomfyShadeController::getMaxGroupOrder() { uint8_t SomfyShadeController::getNextGroupId() { uint8_t ids[SOMFY_MAX_GROUPS]; for(uint8_t i = 0; i < SOMFY_MAX_GROUPS; i++) ids[i] = this->groups[i].getGroupId(); - const uint8_t id = firstFreeId(ids, SOMFY_MAX_GROUPS, SOMFY_MAX_GROUPS - 2); + const uint8_t id = firstFreeId(ids, SOMFY_MAX_GROUPS, SOMFY_MAX_GROUPS); return id == 0 ? 255 : id; } uint8_t SomfyShadeController::getNextRoomId() { uint8_t ids[SOMFY_MAX_ROOMS]; for(uint8_t i = 0; i < SOMFY_MAX_ROOMS; i++) ids[i] = this->rooms[i].roomId; - return firstFreeId(ids, SOMFY_MAX_ROOMS, SOMFY_MAX_ROOMS - 2); + return firstFreeId(ids, SOMFY_MAX_ROOMS, SOMFY_MAX_ROOMS); } int8_t SomfyShadeController::getMaxRoomOrder() { int8_t orders[SOMFY_MAX_ROOMS]; diff --git a/test/test_snapshot/test_snapshot.cpp b/test/test_snapshot/test_snapshot.cpp index 4984b21..b308782 100644 --- a/test/test_snapshot/test_snapshot.cpp +++ b/test/test_snapshot/test_snapshot.cpp @@ -242,9 +242,17 @@ static void test_next_group() { TEST_ASSERT_EQUAL_STRING("{\"groupId\":1,\"remoteAddress\":78,\"bitLength\":0,\"proto\":0}", document(snapshot_doc_t::nextGroup).c_str()); } -static void test_no_room_for_another_shade() { +static void test_the_last_ids_are_given_out() { + // Ids used to stop two short: 30 shades where the limit says 32. snap.shadeCount = 0; for(uint8_t id = 1; id <= SOMFY_MAX_SHADES - 2; id++) snap.shades[snap.shadeCount++] = shade(id, "S"); + TEST_ASSERT_EQUAL_UINT8(SOMFY_MAX_SHADES - 1, nextShadeId(snap)); + snap.shades[snap.shadeCount++] = shade(SOMFY_MAX_SHADES - 1, "S"); + TEST_ASSERT_EQUAL_UINT8(SOMFY_MAX_SHADES, nextShadeId(snap)); +} +static void test_no_room_for_another_shade() { + snap.shadeCount = 0; + for(uint8_t id = 1; id <= SOMFY_MAX_SHADES; id++) snap.shades[snap.shadeCount++] = shade(id, "S"); TEST_ASSERT_EQUAL_UINT8(255, nextShadeId(snap)); } static void test_group_options_lists_the_shades_not_in_it() { @@ -334,6 +342,7 @@ int main(int argc, char **argv) { RUN_TEST(test_next_shade); RUN_TEST(test_next_address_skips_one_already_used); RUN_TEST(test_next_group); + RUN_TEST(test_the_last_ids_are_given_out); RUN_TEST(test_no_room_for_another_shade); RUN_TEST(test_group_options_lists_the_shades_not_in_it); RUN_TEST(test_group_options_of_a_group_gone_by_the_snapshot);