fix: config serialization correctness — padding, mfr metadata, sizes, tx_power (C7/C8/M5/M6)

src/opendisplay/models/config.py

@@ -190,7 +190,7 @@ def from_bytes(cls, data: bytes) -> ManufacturerData:
 class PowerOption:
     """Power configuration (TLV packet type 0x04).
 
-    Size: 32 bytes (packed struct from firmware)
+    Size: 30 bytes (packed struct from firmware)
     """
 
     power_mode: int  # uint8
@@ -233,7 +233,7 @@ def has_battery_sense(self) -> bool:
         """Return True if device has a battery sense circuit."""
         return self.battery_sense_pin != 0xFF
 
-    SIZE: ClassVar[int] = 32
+    SIZE: ClassVar[int] = 30
 
     @classmethod
     def from_bytes(cls, data: bytes) -> PowerOption:
@@ -254,7 +254,7 @@ def from_bytes(cls, data: bytes) -> PowerOption:
             voltage_scaling_factor=int.from_bytes(data[12:14], "little"),
             deep_sleep_current_ua=int.from_bytes(data[14:18], "little"),
             deep_sleep_time_seconds=int.from_bytes(data[18:20], "little"),
-            reserved=data[20:32],
+            reserved=data[20:30],
         )
 
 
@@ -350,7 +350,7 @@ def rotation_enum(self) -> Rotation | int:
         except ValueError:
             return _INDEX_TO_ROTATION.get(self.rotation, self.rotation)
 
-    SIZE: ClassVar[int] = 66
+    SIZE: ClassVar[int] = 46
 
     @classmethod
     def from_bytes(cls, data: bytes) -> DisplayConfig:
@@ -379,7 +379,7 @@ def from_bytes(cls, data: bytes) -> DisplayConfig:
             clk_pin=data[23],
             reserved_pins=data[24:31],  # pins 2-8
             full_update_mC=int.from_bytes(data[31:33], "little"),
-            reserved=data[33:66],
+            reserved=data[33:46],
         )
 
 
@@ -471,7 +471,7 @@ def from_bytes(cls, data: bytes) -> SensorData:
 class DataBus:
     """Data bus configuration (TLV packet type 0x24, repeatable max 4).
 
-    Size: 28 bytes (packed struct from firmware)
+    Size: 30 bytes (packed struct from firmware)
     """
 
     instance_number: int  # uint8
@@ -489,7 +489,7 @@ class DataBus:
     pulldowns: int  # uint8 bitfield
     reserved: bytes  # 14 bytes
 
-    SIZE: ClassVar[int] = 28
+    SIZE: ClassVar[int] = 30
 
     @classmethod
     def from_bytes(cls, data: bytes) -> DataBus:
@@ -511,7 +511,7 @@ def from_bytes(cls, data: bytes) -> DataBus:
             bus_flags=data[13],
             pullups=data[14],
             pulldowns=data[15],
-            reserved=data[16:28],
+            reserved=data[16:30],
         )
 
     @property

src/opendisplay/protocol/config_parser.py

@@ -299,15 +299,11 @@ def _parse_manufacturer_data(data: bytes) -> ManufacturerData:
     if len(data) < 22:
         raise ConfigParseError(f"ManufacturerData too short: {len(data)} bytes (need 22)")
 
-    mfg_id, board_type, board_rev = struct.unpack_from("<HBB", data, 0)
-    reserved = data[4:22]
-
-    return ManufacturerData(
-        manufacturer_id=mfg_id,
-        board_type=board_type,
-        board_revision=board_rev,
-        reserved=reserved,
-    )
+    # Delegate to the model so the simple-config fields (driver/display/power
+    # index + configured_at at offsets 4-15) and the true 6-byte reserved
+    # (offsets 16-21) are parsed once. Storing data[4:22] into reserved here
+    # would drop that metadata and corrupt it on a read-modify-write.
+    return ManufacturerData.from_bytes(data)
 
 
 def _parse_power_option(data: bytes) -> PowerOption:
@@ -331,7 +327,7 @@ def _parse_power_option(data: bytes) -> PowerOption:
         voltage_scaling_factor,
         deep_sleep_current_ua,
         deep_sleep_time_seconds,
-    ) = struct.unpack_from("<HbBBBBBHIH", data, 4)
+    ) = struct.unpack_from("<HBBBBBBHIH", data, 4)  # tx_power is uint8, not int8
 
     reserved = data[20:30]  # 10 reserved bytes, not 12
 

src/opendisplay/protocol/config_serializer.py

@@ -99,7 +99,7 @@ def serialize_system_config(config: SystemConfig) -> bytes:
     )
 
     reserved = config.reserved if config.reserved else b"\x00" * 15
-    data += reserved[:15]
+    data += reserved[:15].ljust(15, b"\x00")
     data += bytes([config.pwr_pin_2 & 0xFF, config.pwr_pin_3 & 0xFF])
     return data
 
@@ -138,7 +138,7 @@ def serialize_manufacturer_data(config: ManufacturerData) -> bytes:
 
     # Pad with reserved bytes to 22 total
     reserved = config.reserved if config.reserved else b"\x00" * 6
-    return data + reserved[:6]
+    return data + reserved[:6].ljust(6, b"\x00")
 
 
 def serialize_power_option(config: PowerOption) -> bytes:
@@ -148,7 +148,7 @@ def serialize_power_option(config: PowerOption) -> bytes:
     - power_mode: uint8
     - battery_capacity_mah: 3 bytes (24-bit LE)
     - sleep_timeout_ms: uint16
-    - tx_power: int8 (signed)
+    - tx_power: uint8
     - sleep_flags: uint8
     - battery_sense_pin: uint8
     - battery_sense_enable_pin: uint8
@@ -176,9 +176,9 @@ def serialize_power_option(config: PowerOption) -> bytes:
         capacity_bytes = config.battery_capacity_mah.to_bytes(3, byteorder="little")
         data += capacity_bytes
 
-    # Pack remaining fields
+    # Pack remaining fields (tx_power is uint8, not int8)
     data += struct.pack(
-        "<HbBBBBBHIH",
+        "<HBBBBBBHIH",
         config.sleep_timeout_ms,
         config.tx_power,
         config.sleep_flags,
@@ -193,7 +193,7 @@ def serialize_power_option(config: PowerOption) -> bytes:
 
     # Pad with reserved bytes to 30 total
     reserved = config.reserved if config.reserved else b"\x00" * 10
-    return data + reserved[:10]
+    return data + reserved[:10].ljust(10, b"\x00")
 
 
 def serialize_display_config(config: DisplayConfig) -> bytes:
@@ -253,7 +253,7 @@ def serialize_display_config(config: DisplayConfig) -> bytes:
 
     # Add reserved pins (7 bytes)
     reserved_pins = config.reserved_pins if config.reserved_pins else b"\xff" * 7
-    data += reserved_pins[:7]
+    data += reserved_pins[:7].ljust(7, b"\xff")
 
     # full_update_mC (uint16 LE) sits between reserved_pins and reserved in the
     # firmware struct; omitting it truncates the packet and the device drops the display.
@@ -297,7 +297,7 @@ def serialize_led_config(config: LedConfig) -> bytes:
 
     # Pad with reserved bytes to 22 total
     reserved = config.reserved if config.reserved else b"\x00" * 15
-    return data + reserved[:15]
+    return data + reserved[:15].ljust(15, b"\x00")
 
 
 def serialize_sensor_data(config: SensorData) -> bytes:
@@ -327,7 +327,7 @@ def serialize_sensor_data(config: SensorData) -> bytes:
 
     data += bytes([config.i2c_addr_7bit & 0xFF, config.msd_data_start_byte & 0xFF])
     reserved = config.reserved if config.reserved else b"\x00" * 24
-    return data + reserved[:24]
+    return data + reserved[:24].ljust(24, b"\x00")
 
 
 def serialize_data_bus(config: DataBus) -> bytes:
@@ -369,7 +369,7 @@ def serialize_data_bus(config: DataBus) -> bytes:
 
     # Pad with reserved bytes to 30 total
     reserved = config.reserved if config.reserved else b"\x00" * 14
-    return data + reserved[:14]
+    return data + reserved[:14].ljust(14, b"\x00")
 
 
 def serialize_binary_inputs(config: BinaryInputs) -> bytes:
@@ -403,7 +403,7 @@ def serialize_binary_inputs(config: BinaryInputs) -> bytes:
 
     # Add reserved pins (8 bytes)
     reserved_pins = config.reserved_pins if config.reserved_pins else b"\x00" * 8
-    data += reserved_pins[:8]
+    data += reserved_pins[:8].ljust(8, b"\x00")
 
     # Add flags
     data += struct.pack(
@@ -419,7 +419,7 @@ def serialize_binary_inputs(config: BinaryInputs) -> bytes:
 
     # Pad with reserved bytes to 30 total
     reserved = config.reserved if config.reserved else b"\x00" * 14
-    return data + reserved[:14]
+    return data + reserved[:14].ljust(14, b"\x00")
 
 
 def serialize_security_config(config: SecurityConfig) -> bytes:
@@ -429,7 +429,7 @@ def serialize_security_config(config: SecurityConfig) -> bytes:
     data += config.session_timeout_seconds.to_bytes(2, "little")
     data += bytes([config.flags & 0xFF, config.reset_pin & 0xFF])
     reserved = config.reserved if config.reserved else b"\x00" * 43
-    return data + reserved[:43]
+    return data + reserved[:43].ljust(43, b"\x00")
 
 
 def serialize_touch_controller(config: TouchController) -> bytes:
@@ -448,7 +448,7 @@ def serialize_touch_controller(config: TouchController) -> bytes:
     )
     data += bytes([config.touch_data_start_byte & 0xFF])
     reserved = config.reserved if config.reserved else b"\x00" * 21
-    return data + reserved[:21]
+    return data + reserved[:21].ljust(21, b"\x00")
 
 
 def serialize_passive_buzzer(config: PassiveBuzzer) -> bytes:
@@ -462,7 +462,7 @@ def serialize_passive_buzzer(config: PassiveBuzzer) -> bytes:
         config.duty_percent,
     )
     reserved = config.reserved if config.reserved else b"\x00" * 27
-    return data + reserved[:27]
+    return data + reserved[:27].ljust(27, b"\x00")
 
 
 def serialize_nfc_config(config: NfcConfig) -> bytes:
@@ -487,7 +487,7 @@ def serialize_nfc_config(config: NfcConfig) -> bytes:
         config.reserved_pin_2,
     )
     reserved = config.reserved if config.reserved else b"\x00" * 16
-    return data + reserved[:16]
+    return data + reserved[:16].ljust(16, b"\x00")
 
 
 def serialize_flash_config(config: FlashConfig) -> bytes:
@@ -508,7 +508,7 @@ def serialize_flash_config(config: FlashConfig) -> bytes:
         config.mode,
     )
     reserved = config.reserved if config.reserved else b"\x00" * 20
-    return data + reserved[:20]
+    return data + reserved[:20].ljust(20, b"\x00")
 
 
 def serialize_wifi_config(config: WifiConfig) -> bytes:

tests/unit/test_config_serialization_fixes.py

@@ -0,0 +1,80 @@
+"""Regression tests for config serialization correctness (C7, C8, M5, M6)."""
+
+from __future__ import annotations
+
+import dataclasses
+
+from opendisplay.models.config import DataBus, DisplayConfig, ManufacturerData, PowerOption
+from opendisplay.protocol.config_parser import (
+    _parse_manufacturer_data,
+    _parse_power_option,
+)
+from opendisplay.protocol.config_serializer import (
+    serialize_data_bus,
+    serialize_manufacturer_data,
+    serialize_power_option,
+)
+
+
+# ── C7: ManufacturerData round-trip preserves simple-config metadata ──────────
+def test_manufacturer_data_roundtrip_preserves_simple_config() -> None:
+    m = ManufacturerData(
+        manufacturer_id=0x2446,
+        board_type=1,
+        board_revision=2,
+        reserved=b"\xaa" * 6,
+        simple_config_driver_index=5,
+        simple_config_display_index=6,
+        simple_config_power_index=7,
+        simple_config_configured_at=123456,
+    )
+    parsed = _parse_manufacturer_data(serialize_manufacturer_data(m))
+    assert parsed.simple_config_driver_index == 5
+    assert parsed.simple_config_display_index == 6
+    assert parsed.simple_config_power_index == 7
+    assert parsed.simple_config_configured_at == 123456
+    assert parsed.reserved == b"\xaa" * 6
+
+
+# ── M5: public SIZE constants match the firmware structs ──────────────────────
+def test_public_sizes_match_firmware() -> None:
+    assert PowerOption.SIZE == 30
+    assert DisplayConfig.SIZE == 46
+    assert DataBus.SIZE == 30
+
+
+def test_display_config_from_bytes_parses_46_byte_packet() -> None:
+    data = bytes(range(46))
+    cfg = DisplayConfig.from_bytes(data)
+    assert cfg.reserved == data[33:46]
+    assert len(cfg.reserved) == 13
+
+
+def test_data_bus_from_bytes_yields_full_14_byte_reserved() -> None:
+    data = bytes(range(30))
+    bus = DataBus.from_bytes(data)
+    assert len(bus.reserved) == 14
+    assert bus.reserved == data[16:30]
+
+
+# ── C8: serializers pad short reserved buffers to the fixed size ──────────────
+def test_data_bus_serialize_pads_short_reserved_to_full_length() -> None:
+    bus = DataBus.from_bytes(bytes(30))
+    short = dataclasses.replace(bus, reserved=b"\x01\x02")  # only 2 bytes
+    out = serialize_data_bus(short)
+    assert len(out) == 30  # would be 18 without padding, desyncing the stream
+
+
+# ── M6: tx_power is treated as unsigned end-to-end ────────────────────────────
+def test_tx_power_unsigned_roundtrip() -> None:
+    raw = bytearray(30)
+    raw[6] = 0xF4  # 244 unsigned; would be -12 if parsed signed
+    po = PowerOption.from_bytes(bytes(raw))
+    assert po.tx_power == 244
+
+    parsed = _parse_power_option(bytes(raw))
+    assert parsed.tx_power == 244
+
+    out = serialize_power_option(po)  # must not raise struct.error
+    assert out[6] == 0xF4
+    assert len(out) == 30