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