From 6f2c36936e8a3c83cfdcf42cdce578e9fd66435e Mon Sep 17 00:00:00 2001 From: Ryan Malloy Date: Fri, 2 Oct 2026 00:37:40 -0600 Subject: [PATCH] Keep every element of a list-shaped event 17 events are MAP_ITEM or LIST_ITEM: they arrive once per element, all carrying the same argument names, so a flat store kept only whichever landed last. SensorsStatesListChanged reports six sensors that way, and the simulator's deliberate magnetometer fault was invisible through state() as a result, which is exactly the fact preflight_check needs to see. Entries are now keyed Command[element]_arg. Parrot marks these events but never names the key; it is the first argument by convention. --- src/mcbebop/arsdk/session.py | 35 ++++++++++++++++++++- src/mcbebop/protocol/types.py | 9 ++++++ src/mcbebop/protocol/xml_index.py | 16 ++++++++++ tests/test_list_events.py | 52 +++++++++++++++++++++++++++++++ 4 files changed, 111 insertions(+), 1 deletion(-) create mode 100644 tests/test_list_events.py diff --git a/src/mcbebop/arsdk/session.py b/src/mcbebop/arsdk/session.py index d0e8440..d8ba8f4 100644 --- a/src/mcbebop/arsdk/session.py +++ b/src/mcbebop/arsdk/session.py @@ -154,6 +154,38 @@ def _as_event(decoded: Any, payload: bytes) -> Event | None: ) +def _expand_list_event(event: Event) -> dict[str, Any]: + """Give each element of a list-shaped event its own keys. + + MAP_ITEM and LIST_ITEM events arrive once per element and every one of them + carries the same argument names, so a flat store keeps only whichever + landed last. `SensorsStatesListChanged` reports six sensors that way: left + alone, a failing magnetometer vanishes behind the sensor that follows it, + which is precisely the fact a preflight check exists to surface. + + Entries become `Command[element]_arg`. The identifying argument is also + kept unsuffixed so that "what arrived most recently" is still answerable. + """ + from mcbebop.protocol import xml_index + + spec = xml_index.by_ids(event.ids) + if spec is None or not spec.list_key: + return event.values + + key_field = f"{spec.name}_{spec.list_key}" + element = event.values.get(key_field) + if element is None: + return event.values + + label = getattr(element, "value", element) + out: dict[str, Any] = {key_field: element} + for key, value in event.values.items(): + if key == key_field: + continue + out[f"{spec.name}[{label}]_{key.split('_', 1)[-1]}"] = value + return out + + class DroneSession: """What the tools layer holds. Implements the `Session` protocol.""" @@ -246,8 +278,9 @@ class DroneSession: self._record(event) def _record(self, event: Event) -> None: + values = _expand_list_event(event) with self._lock: - for key, value in event.values.items(): + for key, value in values.items(): prior = self._store.get(key) self._store[key] = Reading(value, event.at, (prior.count + 1) if prior else 1) hit = [w for w in self._waiters if event.ids in w.ids and w.predicate(event)] diff --git a/src/mcbebop/protocol/types.py b/src/mcbebop/protocol/types.py index 1082c20..7264dbe 100644 --- a/src/mcbebop/protocol/types.py +++ b/src/mcbebop/protocol/types.py @@ -117,6 +117,15 @@ class CommandSpec: title: str = "" doc: str = "" expectations: tuple[Expectation, ...] = () + list_key: str | None = None + """For a MAP_ITEM/LIST_ITEM event, the argument that identifies the entry. + + These events arrive once per element, so a flat `_` store + would let each one overwrite the last and leave only the final element + visible. `SensorsStatesListChanged` sends six, so a failing sensor + disappears behind whichever arrives last. Parrot names no key explicitly; + it is the first argument by convention. + """ @property def full_name(self) -> str: diff --git a/src/mcbebop/protocol/xml_index.py b/src/mcbebop/protocol/xml_index.py index 97d89d1..58a024e 100644 --- a/src/mcbebop/protocol/xml_index.py +++ b/src/mcbebop/protocol/xml_index.py @@ -174,6 +174,7 @@ def _parse_command(project: str, project_id: int, klass: str, class_id: int, cmd title=title, doc=doc, expectations=expectations_for(cmd), + list_key=_list_key(cmd), ) return replace(spec, tier=tier_for(spec)) @@ -202,6 +203,21 @@ def xml_dir() -> Path: ) +LIST_TYPES = ("MAP_ITEM", "LIST_ITEM") + + +def _list_key(cmd: ET.Element) -> str | None: + """Which argument identifies one entry of a list-shaped event. + + Parrot marks these with type="MAP_ITEM" or "LIST_ITEM" and never names the + key; by convention it is the first argument. 17 events are shaped this way. + """ + if cmd.get("type") not in LIST_TYPES: + return None + first = cmd.find("arg") + return first.get("name") if first is not None else None + + def _flat_trim(takeoff: CommandSpec) -> CommandSpec: """Hand-add `ardrone3.Piloting.FlatTrim`. diff --git a/tests/test_list_events.py b/tests/test_list_events.py new file mode 100644 index 0000000..e223389 --- /dev/null +++ b/tests/test_list_events.py @@ -0,0 +1,52 @@ +"""List-shaped events must not overwrite each other. + +MAP_ITEM and LIST_ITEM events arrive once per element with identical argument +names. A flat store keeps only the last, which silently hides exactly the kind +of fact a preflight check looks for. +""" + +import asyncio + +import pytest + +from mcbebop.arsdk.session import DroneSession +from mcbebop.protocol import xml_index +from mcbebop.sim import FakeBebop + +SENSORS = "common.CommonState.SensorsStatesListChanged" + + +def test_list_events_are_indexed(): + listed = [c for c in xml_index.all_commands() if c.list_key] + assert len(listed) == 17, "both XML files together define 17 MAP_ITEM/LIST_ITEM events" + assert xml_index.get(SENSORS).list_key == "sensorName" + # A scalar event must not be treated as a list. + assert xml_index.get("common.CommonState.BatteryStateChanged").list_key is None + + +def test_every_sensor_survives_and_a_fault_is_visible(): + async def run(): + with FakeBebop() as sim: + session = DroneSession(ip=sim.host, discovery_port=sim.discovery_port) + await session.connect() + await session.request_full_state() + await asyncio.sleep(2.0) + values = session.values() + await session.disconnect() + return values + + values = asyncio.run(run()) + sensors = { + key.split("[", 1)[1].split("]", 1)[0]: value + for key, value in values.items() + if key.startswith("SensorsStatesListChanged[") and key.endswith("_sensorState") + } + assert len(sensors) == 6, f"expected all six self-tests, got {sorted(sensors)}" + faults = sorted(name for name, ok in sensors.items() if not ok) + assert faults == ["magnetometer"], "the simulator's deliberate fault must be reachable" + + +@pytest.mark.parametrize("name", [SENSORS, "common.CommonState.MassStorageInfoStateListChanged"]) +def test_list_key_is_the_first_argument(name): + spec = xml_index.get(name) + assert spec.list_key == spec.args[0].name