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);