From 9d7134e70a1cbbd6db5537877aae6367b6c2b3c3 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Sat, 4 Jul 2026 03:18:40 -0400 Subject: [PATCH] fix: validate LED group_repeats 1-254 and tolerate raw 0xFF on parse (M11) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit group_repeats is encoded as group_repeats-1, and raw 0xFE is the firmware's infinite sentinel. The old 1-255 range let group_repeats=255 encode to 0xFE, so a request for 255 finite repeats looped forever. Cap the finite range at 254. Conversely, from_bytes decoded raw 0xFF to 256, which the constructor then rejected — so parsing a firmware-valid payload raised. Treat 0xFF (never emitted by our encoder) like the 0xFE unbounded sentinel so from_bytes never raises. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01JRrm95f1qNZzDM9r2SB6KW --- src/opendisplay/models/led_flash.py | 11 ++++++++--- tests/unit/test_models_led_flash.py | 17 +++++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/opendisplay/models/led_flash.py b/src/opendisplay/models/led_flash.py index 3f4aaf2..bebdb75 100644 --- a/src/opendisplay/models/led_flash.py +++ b/src/opendisplay/models/led_flash.py @@ -47,8 +47,10 @@ def __post_init__(self) -> None: _check_nibble("mode", self.mode) if not 1 <= self.brightness <= 16: raise ValueError(f"brightness out of range: {self.brightness} (must be 1-16)") - if self.group_repeats is not None and not 1 <= self.group_repeats <= 255: - raise ValueError(f"group_repeats out of range: {self.group_repeats} (must be 1-255 or None for infinite)") + # Encoded as group_repeats-1; raw 0xFE is the firmware's infinite + # sentinel, so 255 finite repeats would encode to 0xFE and loop forever. + if self.group_repeats is not None and not 1 <= self.group_repeats <= 254: + raise ValueError(f"group_repeats out of range: {self.group_repeats} (must be 1-254 or None for infinite)") _check_u8("reserved", self.reserved) @classmethod @@ -125,7 +127,10 @@ def from_bytes(cls, data: bytes) -> LedFlashConfig: mode = data[0] & 0x0F brightness = ((data[0] >> 4) & 0x0F) + 1 group_raw = data[10] - group_repeats = None if group_raw == 0xFE else (group_raw + 1) + # 0xFE is the firmware's infinite sentinel; 0xFF (which our encoder never + # emits) is a firmware-valid unbounded value — treat both as infinite so + # parsing a device's payload never raises. Finite values decode as raw+1. + group_repeats = None if group_raw in (0xFE, 0xFF) else (group_raw + 1) return cls( mode=mode, diff --git a/tests/unit/test_models_led_flash.py b/tests/unit/test_models_led_flash.py index 40423b9..5fa344c 100644 --- a/tests/unit/test_models_led_flash.py +++ b/tests/unit/test_models_led_flash.py @@ -72,3 +72,20 @@ def test_led_flash_config_rejects_invalid_values() -> None: with pytest.raises(ValueError, match="color out of range"): LedFlashStep(color=256) + + +def test_group_repeats_255_rejected_to_avoid_infinite_sentinel() -> None: + # 255 would encode to raw 0xFE (the infinite sentinel) and loop forever (M11). + with pytest.raises(ValueError, match="group_repeats out of range"): + LedFlashConfig(group_repeats=255) + + +def test_group_repeats_254_is_max_finite() -> None: + cfg = LedFlashConfig(group_repeats=254) + assert cfg.to_bytes()[10] == 253 # raw = group_repeats - 1 + + +def test_from_bytes_accepts_raw_0xff_without_raising() -> None: + payload = bytes([0x70, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xFF, 0]) + cfg = LedFlashConfig.from_bytes(payload) # must not raise + assert cfg.group_repeats is None