From 993b7c1575b112c4d2a0a6332c8ab1171223cf52 Mon Sep 17 00:00:00 2001 From: Michael Sauer Date: Sun, 20 Sep 2026 01:16:42 +0200 Subject: [PATCH] fix(settings): neue Felder erst ab dem alten sizeof - sonst war sACN nach dem Update aus Ein Datensatz aus der Firmware vor diesem Zweig ist 208 Byte lang und traegt hinter wake_on_change (206) noch ein Fuellbyte. sacn_enabled lag auf genau diesem Byte (207), und read_rec() kopiert hdr.length Bytes ueber die Vorgaben - jede Lampe im Feld waere nach der OTA mit sacn_enabled == 0 hochgekommen und haette keinen Socket gebunden, genau das Gegenteil dessen, was Spezifikation und README versprechen. _legacy_pad schiebt das erste neue Feld auf 208; _padding schrumpft auf 1 Byte, damit sizeof ein Vielfaches von 4 bleibt (216). nl_settings_legacy_check pinnt den Offset zur Uebersetzungszeit fest, denn die bisherige Pruefung sah nur die Teilbarkeit durch 4 und liess diesen Fall durch. Der Test kuerzte auf offsetof(sacn_enabled) - eine Laenge, die es auf keiner Lampe je gab - und ging deshalb durch; er kuerzt jetzt auf die Literalzahl 208 und stellt das Fuellbyte nach. --- src/config/nl_settings.c | 18 +++++++++++++++--- src/config/nl_settings.h | 7 +++++-- test/test_settings.c | 11 +++++++++-- 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/src/config/nl_settings.c b/src/config/nl_settings.c index d33d40b..3a95fcd 100644 --- a/src/config/nl_settings.c +++ b/src/config/nl_settings.c @@ -29,9 +29,13 @@ typedef struct { /* Where read_rec() expects the CRC, and therefore where nl_settings_save() must put it: directly * behind the `length` payload bytes, not at offsetof(rec_t, crc). Both offsets are only equal - * while sizeof(nl_settings_t) is a multiple of 4 (204 today). Adding a single uint16_t would move - * offsetof(rec_t, crc) two bytes further than the reader looks and every stored record would - * silently fail its CRC on the next boot - Wi-Fi, token and name gone on every lamp. */ + * while sizeof(nl_settings_t) is a multiple of 4 (216 today, 208 before sACN). Adding a single + * uint16_t would move offsetof(rec_t, crc) two bytes further than the reader looks and every + * stored record would silently fail its CRC on the next boot - Wi-Fi, token and name gone on + * every lamp. Aus demselben Grund gilt fuer neue Felder: sie beginnen bei oder hinter dem + * vorherigen sizeof (also nicht unterhalb von 208), denn ein alter Datensatz ist genau + * sizeof lang und traegt seine Fuellbytes mit - ein Feld darunter wuerde beim Lesen von + * diesen Fuellbytes (Null) ueberschrieben statt seine Vorgabe zu behalten. */ #define NL_SETTINGS_CRC_OFF(len) (sizeof(rec_hdr_t) + (uint32_t)(len)) /* The CRC runs over header and payload as one contiguous block on both sides, so the payload @@ -44,6 +48,14 @@ typedef char nl_settings_layout_check[offsetof(rec_t, data) == sizeof(rec_hdr_t) * zu entfernen. */ typedef char nl_settings_size_check[(sizeof(nl_settings_t) % 4u == 0u) ? 1 : -1]; +/* Das erste mit sACN angehaengte Feld muss genau dort beginnen, wo der alte Satz aufhoerte: + * bei 208, dem sizeof(nl_settings_t) der Firmware vor sACN. Saetze, die eine solche Firmware + * geschrieben hat, sind 208 Byte lang und enthalten hinter wake_on_change (Offset 206) noch + * ein Fuellbyte. read_rec() kopiert hdr.length Bytes ueber die Vorgaben - ein Feld unterhalb + * von 208 wuerde also von diesem Fuellbyte (Null) ueberschrieben und seine Vorgabe verlieren. + * Bei sacn_enabled hiesse das: sACN nach dem Update auf jeder Lampe aus. */ +typedef char nl_settings_legacy_check[offsetof(nl_settings_t, sacn_enabled) == 208 ? 1 : -1]; + static uint32_t s_addr[2]; void nl_settings_defaults(nl_settings_t *s) diff --git a/src/config/nl_settings.h b/src/config/nl_settings.h index b246a57..bef8e92 100644 --- a/src/config/nl_settings.h +++ b/src/config/nl_settings.h @@ -28,11 +28,14 @@ typedef struct { * switching on or off. */ uint8_t wake_on_change; /* if set, such a change also switches the lamp on when it was * off, unless the same call says "on": false. */ - uint8_t sacn_enabled; /* sACN/E1.31-Empfang aktiv (Vorgabe 1) */ + uint8_t _legacy_pad; /* war Fuellbyte im 208-Byte-Satz: neue Felder muessen + * bei oder hinter dem vorherigen sizeof beginnen, sonst + * ueberschreibt das Fuellbyte eines alten Datensatzes sie */ + uint8_t sacn_enabled; /* Offset 208 - sACN/E1.31-Empfang aktiv (Vorgabe 1) */ uint16_t sacn_universe; /* 1..63999 (Vorgabe 1) */ uint16_t sacn_start_slot; /* 1..512 (Vorgabe 1) */ uint8_t sacn_hold; /* 0 = nach dem Timeout zurueck zum HA-Zustand, 1 = halten */ - uint8_t _padding[2]; /* padding for 4-byte alignment of sizeof(nl_settings_t) */ + uint8_t _padding[1]; /* padding for 4-byte alignment of sizeof(nl_settings_t) */ } nl_settings_t; void nl_settings_defaults(nl_settings_t *s); diff --git a/test/test_settings.c b/test/test_settings.c index 43f4e83..790c0ff 100644 --- a/test/test_settings.c +++ b/test/test_settings.c @@ -208,14 +208,21 @@ static void test_sacn_roundtrip(void) } /* Ein Datensatz aus einer Firmware VOR sACN: die vier Felder muessen als echte Vorgaben - * zurueckkommen, nicht als Null - sonst waere sACN auf jeder aktualisierten Lampe aus. */ + * zurueckkommen, nicht als Null - sonst waere sACN auf jeder aktualisierten Lampe aus. + * Die Laenge ist bewusst die Literalzahl 208: das war sizeof(nl_settings_t) vor diesem + * Zweig und damit die Laenge, die auf jeder ausgelieferten Lampe im Flash steht. Ein + * offsetof() auf ein heutiges Feld wuerde mit der Struktur mitwandern und genau den Fall + * verfehlen, um den es hier geht. Das letzte Byte (207) war im 208-Byte-Satz reines + * Fuellbyte und daher 0 - es wird hier nachgestellt, denn genau dieses Byte legt sich + * beim Lesen ueber das erste neue Feld. */ static void test_record_without_sacn_gets_defaults(void) { nl_settings_t in = sample("alt"), out; uint32_t crc; nl_settings_save(&in); { - uint16_t shorter = (uint16_t)offsetof(nl_settings_t, sacn_enabled); + uint16_t shorter = 208u; /* sizeof(nl_settings_t) vor sACN */ + fake_flash_mem[NL_FL_SETTINGS_A + 12 + 207] = 0; /* Fuellbyte des alten Satzes */ memcpy(&fake_flash_mem[NL_FL_SETTINGS_A + 6], &shorter, 2); crc = nl_crc32(0, &fake_flash_mem[NL_FL_SETTINGS_A], 12 + shorter); memcpy(&fake_flash_mem[NL_FL_SETTINGS_A + 12 + shorter], &crc, 4);