From 54b04b76534ace31963865f23e436ebf1adcb771 Mon Sep 17 00:00:00 2001 From: gabriel Date: Mon, 3 Aug 2026 14:14:41 +0200 Subject: [PATCH] fix(advertisement): stop tracking dynamic bytes that are not buttons AdvertisementTracker decoded all 11 bytes of the dynamic block as button reports and emitted a transition whenever any of them changed. Only the bytes a BinaryInputs packet claims are buttons; the rest belong to touch controllers and sensors, and every one of them decodes into a valid-looking button report. A moving touch coordinate or a refreshed sensor reading therefore produced phantom button_down / button_up / press_count_changed / button_slot_changed events. On a reTerminal E1003 (button byte 0, touch 1-5, SHT40 7-9) a 0.1 C change in the sensor reading was enough: unfiltered: 1 event(s) [('button_slot_changed', 7)] filtered : 0 event(s) [] AdvertisementTracker now accepts the byte indices it should watch. Home Assistant filters incoming events by the configured byte_index before firing an entity, which is why this was invisible there, but the events were still produced on every advertisement and any other consumer would have believed them. Add BinaryInputs.published_button_byte_index so callers derive those indices without re-implementing the firmware's rule that 0xFF (its default) means "not published" and indices past the block are ignored: AdvertisementTracker( idx for bi in config.binary_inputs if (idx := bi.published_button_byte_index) is not None ) The parameter is optional and defaults to watching every byte, so existing callers are unaffected until they opt in. --- src/opendisplay/models/advertisement.py | 31 ++++++++++- src/opendisplay/models/config.py | 14 +++++ tests/unit/test_models_advertisement.py | 74 +++++++++++++++++++++++++ tests/unit/test_models_config.py | 31 ++++++++++- 4 files changed, 146 insertions(+), 4 deletions(-) diff --git a/src/opendisplay/models/advertisement.py b/src/opendisplay/models/advertisement.py index 6fff71c..5ee62a6 100644 --- a/src/opendisplay/models/advertisement.py +++ b/src/opendisplay/models/advertisement.py @@ -4,6 +4,7 @@ import struct import time +from collections.abc import Iterable from dataclasses import dataclass, field @@ -78,7 +79,13 @@ def is_pressed(self, byte_index: int) -> bool | None: @property def button_events(self) -> list[ButtonEventData]: - """Decode all dynamic return bytes as button event data (v1 only).""" + """Decode all dynamic return bytes as button event data (v1 only). + + Every byte is decoded, including those owned by touch controllers and + sensors, which produce valid-looking but meaningless button reports. + Callers should keep only the indices their config assigns to buttons + (see ``BinaryInputs.published_button_byte_index``). + """ if self.format_version != "v1": return [] return [decode_button_event(raw, i) for i, raw in enumerate(self.dynamic_data)] @@ -193,11 +200,29 @@ class AdvertisementTracker: """Track per-device v1 advertisements and emit button transitions. This is best-effort only: BLE advertisements can be dropped. + + The 11-byte dynamic block is shared: buttons own the bytes their config + packets claim, and touch controllers and sensors own the rest. Decoding a + byte that belongs to something else yields a valid-looking button report, + so a tracker watching every byte emits phantom transitions whenever a + touch coordinate or a sensor reading changes. + + Pass ``byte_indices`` to watch only the bytes that are really buttons -- + ``BinaryInputs.published_button_byte_index`` for each configured input. + Omitting it keeps the historical behaviour of watching all 11 bytes. """ - def __init__(self) -> None: + def __init__(self, byte_indices: Iterable[int] | None = None) -> None: + """Initialize the tracker, optionally restricted to button bytes.""" + self._byte_indices = None if byte_indices is None else frozenset(byte_indices) self._last_by_address: dict[str, list[ButtonEventData]] = {} + def _watched(self, events: list[ButtonEventData]) -> list[ButtonEventData]: + """Drop dynamic bytes that no configured button reports into.""" + if self._byte_indices is None: + return events + return [event for event in events if event.byte_index in self._byte_indices] + def reset(self, address: str | None = None) -> None: """Reset tracker state for one device or all devices.""" if address is None: @@ -216,7 +241,7 @@ def update( self._last_by_address.pop(address, None) return [] - current = advertisement.button_events + current = self._watched(advertisement.button_events) previous = self._last_by_address.get(address) self._last_by_address[address] = current diff --git a/src/opendisplay/models/config.py b/src/opendisplay/models/config.py index 2b875f8..25c2d4d 100644 --- a/src/opendisplay/models/config.py +++ b/src/opendisplay/models/config.py @@ -603,6 +603,20 @@ class BinaryInputs: MAX_LADDER_BUTTONS: ClassVar[int] = 4 MAX_BUTTON_ID: ClassVar[int] = 7 # button id is a 3-bit field in the report byte MAX_BUTTON_DATA_BYTE_INDEX: ClassVar[int] = 10 # index into the 11-byte MSD block + BUTTON_DATA_NOT_PUBLISHED: ClassVar[int] = 0xFF # firmware default: report nothing + + @property + def published_button_byte_index(self) -> int | None: + """Dynamic block byte this input reports into, or None if it publishes none. + + The firmware treats 0xFF (its default) as "not published" and ignores + any index past the 11-byte block. Use this to decide which bytes of an + advertisement really carry button state -- the rest belong to touch + controllers and sensors, and decode into meaningless button reports. + """ + if self.button_data_byte_index > self.MAX_BUTTON_DATA_BYTE_INDEX: + return None + return self.button_data_byte_index @classmethod def adc_ladder( diff --git a/tests/unit/test_models_advertisement.py b/tests/unit/test_models_advertisement.py index 3e59292..6347603 100644 --- a/tests/unit/test_models_advertisement.py +++ b/tests/unit/test_models_advertisement.py @@ -428,3 +428,77 @@ def test_legacy_advertisement_returns_no_events(self) -> None: legacy = bytes([0x02, 0x36, 0x00, 0x6C, 0x00, 0xC3, 0x01, 0x55, 0x0F, 0x16, 0x4D]) adv = parse_advertisement(legacy) assert tracker.update(self.ADDRESS, adv, timestamp=1.0) == [] + + +# ── tracker byte filtering ──────────────────────────────────────────────────── + + +def _dynamic(**by_index: int) -> bytes: + """An 11-byte dynamic block with the given bytes set.""" + block = bytearray(11) + for index, value in by_index.items(): + block[int(index.removeprefix("b"))] = value + return bytes(block) + + +def test_tracker_ignores_bytes_no_button_reports_into() -> None: + """A sensor or touch byte changing must not look like a button press. + + Byte 1 here stands in for a slot owned by something else (an SHT40 block + or a touch coordinate). Only byte 0 is a configured button. + """ + tracker = AdvertisementTracker([0]) + tracker.update("AA", parse_advertisement(_v1_payload(_dynamic(b0=0x28, b1=0x54)))) + + events = tracker.update("AA", parse_advertisement(_v1_payload(_dynamic(b0=0x28, b1=0x4C)))) + + assert events == [] + + +def test_tracker_without_indices_still_watches_every_byte() -> None: + """Historical behaviour is preserved when no indices are given.""" + tracker = AdvertisementTracker() + tracker.update("AA", parse_advertisement(_v1_payload(_dynamic(b1=0x54)))) + + events = tracker.update("AA", parse_advertisement(_v1_payload(_dynamic(b1=0x4C)))) + + assert [e.byte_index for e in events] == [1] + + +def test_tracker_still_reports_configured_button() -> None: + """Filtering must not suppress the bytes that are real buttons.""" + tracker = AdvertisementTracker([0]) + tracker.update("AA", parse_advertisement(_v1_payload(_dynamic(b0=0x00)))) + + events = tracker.update("AA", parse_advertisement(_v1_payload(_dynamic(b0=0x88)))) + + assert [(e.event_type, e.byte_index) for e in events] == [ + ("button_down", 0), + ("press_count_changed", 0), + ] + + +def test_tracker_watches_several_button_bytes() -> None: + tracker = AdvertisementTracker([0, 5]) + tracker.update("AA", parse_advertisement(_v1_payload(_dynamic(b0=0x00, b3=0x11, b5=0x00)))) + + events = tracker.update("AA", parse_advertisement(_v1_payload(_dynamic(b0=0x88, b3=0x99, b5=0x88)))) + + assert sorted(e.byte_index for e in events) == [0, 0, 5, 5] + + +def test_tracker_accepts_any_iterable_of_indices() -> None: + tracker = AdvertisementTracker(i for i in (0,)) + tracker.update("AA", parse_advertisement(_v1_payload(_dynamic(b0=0x00)))) + + events = tracker.update("AA", parse_advertisement(_v1_payload(_dynamic(b0=0x88)))) + + assert [e.byte_index for e in events] == [0, 0] + + +def test_tracker_with_no_button_bytes_emits_nothing() -> None: + """A device whose inputs all publish nothing yields no events at all.""" + tracker = AdvertisementTracker([]) + tracker.update("AA", parse_advertisement(_v1_payload(_dynamic(b0=0x28)))) + + assert tracker.update("AA", parse_advertisement(_v1_payload(_dynamic(b0=0x4C)))) == [] diff --git a/tests/unit/test_models_config.py b/tests/unit/test_models_config.py index 7630d69..9587d81 100644 --- a/tests/unit/test_models_config.py +++ b/tests/unit/test_models_config.py @@ -2,7 +2,7 @@ import pytest -from opendisplay.models.config import DisplayConfig, ManufacturerData, PowerOption +from opendisplay.models.config import BinaryInputs, DisplayConfig, ManufacturerData, PowerOption from opendisplay.models.enums import ( BoardManufacturer, DIYBoardType, @@ -168,3 +168,32 @@ def test_supports_zip_false_when_only_raw_bit_set(self): def test_supports_zip_true_with_multiple_bits_set(self): assert self._display(transmission_modes=0x03).supports_zip is True + + +class TestBinaryInputsPublishedButtonByteIndex: + """0xFF means 'not published'; the firmware also ignores indices past the block.""" + + def _inputs(self, button_data_byte_index: int) -> BinaryInputs: + return BinaryInputs( + instance_number=0, + input_type=1, + display_as=1, + reserved_pins=b"\x00" * 8, + input_flags=0x01, + invert=0, + pullups=0, + pulldowns=0, + button_data_byte_index=button_data_byte_index, + ) + + def test_returns_index_when_published(self) -> None: + assert self._inputs(0).published_button_byte_index == 0 + + def test_returns_highest_valid_index(self) -> None: + assert self._inputs(10).published_button_byte_index == 10 + + def test_returns_none_for_not_published(self) -> None: + assert self._inputs(0xFF).published_button_byte_index is None + + def test_returns_none_for_index_past_block(self) -> None: + assert self._inputs(11).published_button_byte_index is None