Protocol layer: XML index, codec, safety tiers, expectations

Parse Parrot's common.xml and ardrone3.xml into CommandSpec objects rather
than transcribing 264 commands by hand. Direction is derived from
comment result=/triggered= with a class-name-suffix fallback for the 31
commands that state neither, giving 101 to-drone and 163 from-drone.
ardrone3.Piloting.FlatTrim is injected: the firmware accepts it but neither
the vendored snapshot nor upstream defines it.

codec encodes the <BBH header (the command id is 16 bits, not 8) and reads
enums as i32 and strings as NUL-terminated, both of which pyparrot gets
wrong. safety assigns the arming tier by class, with Mavlink and Calibration
counted as motion because they move the aircraft. expectations parses the
undocumented #p-c-m grammar including | alternatives and this.<arg> echoes.

90 tests, counts asserted so a bad parse fails loudly.

tests/test_protocol_index.py

@@ -0,0 +1,222 @@
+"""The index is derived, not transcribed, so these pin the derivation.
+
+Every count here was read off the two vendored XML files. A bad parse that
+still produces plausible-looking objects is the failure mode worth catching,
+so the numbers are asserted rather than the shapes.
+"""
+
+import collections
+import shutil
+
+import pytest
+
+from mcbebop.protocol import xml_index
+from mcbebop.protocol.types import Buffer, Direction, Tier
+
+TOTAL_IN_XML = 264
+TOTAL_WITH_FLAT_TRIM = 265
+
+
+def test_command_count():
+    assert len(xml_index.all_commands()) == TOTAL_WITH_FLAT_TRIM
+    from_xml = [c for c in xml_index.all_commands() if c.name != "FlatTrim"]
+    assert len(from_xml) == TOTAL_IN_XML
+
+
+def test_direction_split():
+    counts = collections.Counter(c.direction for c in xml_index.all_commands() if c.name != "FlatTrim")
+    assert counts[Direction.TO_DRONE] == 101
+    assert counts[Direction.FROM_DRONE] == 163
+    assert sum(counts.values()) == TOTAL_IN_XML
+
+
+def test_direction_fallback_count():
+    """31 commands state neither result nor triggered and fall back to the suffix."""
+    from xml.etree import ElementTree
+
+    directory = xml_index.xml_dir()
+    neither = 0
+    for name in xml_index.XML_FILES:
+        project = ElementTree.parse(directory / name).getroot()
+        for klass in project.iter("class"):
+            for cmd in klass.findall("cmd"):
+                comment = cmd.find("comment")
+                attrs = comment.attrib if comment is not None else {}
+                assert not ("result" in attrs and "triggered" in attrs), cmd.get("name")
+                if "result" not in attrs and "triggered" not in attrs:
+                    neither += 1
+    assert neither == 31
+
+
+def test_mixed_controller_class_is_not_decided_by_suffix():
+    # common.Controller holds one of each, which is why the suffix rule alone
+    # would be wrong there. Both state result/triggered, so it never applies.
+    assert xml_index.get("common.Controller.isPiloting").direction is Direction.TO_DRONE
+    assert xml_index.get("common.Controller.PeerStateChanged").direction is Direction.FROM_DRONE
+
+
+def test_buffers():
+    counts = collections.Counter(c.buffer for c in xml_index.all_commands() if c.name != "FlatTrim")
+    assert counts[Buffer.ACK] == 248
+    assert counts[Buffer.NON_ACK] == 15
+    assert counts[Buffer.HIGH_PRIO] == 1
+    high_prio = [c.full_name for c in xml_index.all_commands() if c.buffer is Buffer.HIGH_PRIO]
+    assert high_prio == ["ardrone3.Piloting.Emergency"]
+
+
+def test_deprecated_count():
+    assert sum(1 for c in xml_index.all_commands() if c.deprecated) == 39
+
+
+def test_expectation_block_count():
+    blocks = sum(1 for c in xml_index.all_commands() if c.expectations)
+    assert blocks == 82
+    assert all(c.direction is Direction.TO_DRONE for c in xml_index.all_commands() if c.expectations)
+
+
+def test_ids_are_unique_and_lookups_agree():
+    for command in xml_index.all_commands():
+        assert xml_index.by_ids(command.ids) is command
+        assert xml_index.get(command.full_name) is command
+    assert xml_index.get("ardrone3.Piloting.Nope") is None
+    assert xml_index.by_ids((9, 9, 9)) is None
+
+
+def test_known_ids_match_the_live_aircraft():
+    # Read off real traffic, see docs/notes/protocol.md.
+    assert xml_index.get("common.Common.AllStates").ids == (0, 4, 0)
+    assert xml_index.get("common.Settings.AllSettings").ids == (0, 2, 0)
+    assert xml_index.get("ardrone3.Piloting.TakeOff").ids == (1, 0, 1)
+    assert xml_index.get("ardrone3.Piloting.PCMD").ids == (1, 0, 2)
+
+
+def test_command_ids_have_gaps_and_document_order_is_not_id_order():
+    """Never infer an id from a command's position in the file."""
+    piloting = [c for c in xml_index.all_commands() if f"{c.project}.{c.klass}" == "ardrone3.Piloting"]
+    in_document_order = [c.ids[2] for c in piloting]
+    assert in_document_order != sorted(in_document_order)
+
+    # ardrone3.PilotingState runs 1..22 with no id 0, so counting is not enough.
+    state = [
+        c.ids[2] for c in xml_index.all_commands() if f"{c.project}.{c.klass}" == "ardrone3.PilotingState"
+    ]
+    assert len(state) < max(state) + 1
+
+
+def test_flat_trim_is_injected():
+    flat_trim = xml_index.get("ardrone3.Piloting.FlatTrim")
+    assert flat_trim is not None
+    assert flat_trim.ids == (1, 0, 0)
+    assert flat_trim.args == ()
+    assert flat_trim.buffer is Buffer.ACK
+    assert flat_trim.direction is Direction.TO_DRONE
+    assert flat_trim.tier is Tier.MOTION
+    assert flat_trim.support is None
+    assert "missing from Parrot's XML" in flat_trim.doc
+
+
+def test_commands_without_a_comment_keep_their_bare_text_as_doc():
+    bare = [
+        "common.CalibrationState.PitotCalibrationStateChanged",
+        "common.ARLibsVersionsState.ControllerLibARCommandsVersion",
+        "common.ARLibsVersionsState.SkyControllerLibARCommandsVersion",
+        "common.ARLibsVersionsState.DeviceLibARCommandsVersion",
+        "ardrone3.SettingsState.CPUID",
+        "ardrone3.MediaStreamingState.VideoStreamModeChanged",
+    ]
+    for name in bare:
+        command = xml_index.get(name)
+        assert command.title == ""
+        assert command.doc, name
+    assert xml_index.get("ardrone3.SettingsState.CPUID").doc == "Product main cpu id"
+
+
+def test_enum_values_are_positions_and_names_stay_verbatim():
+    band = xml_index.get("ardrone3.Network.WifiScan").args[0]
+    assert band.is_enum
+    assert [(m.name, m.value) for m in band.members] == [("2_4ghz", 0), ("5ghz", 1), ("all", 2)]
+    # Member names that are not valid identifiers must survive as strings.
+    framerate = xml_index.get("ardrone3.PictureSettings.VideoFramerate").args[0]
+    assert [m.name for m in framerate.members][:3] == ["24_FPS", "25_FPS", "30_FPS"]
+
+
+def test_only_name_and_type_on_args():
+    # Nothing in the parse may depend on an arg attribute Parrot does not set.
+    for command in xml_index.all_commands():
+        for arg in command.args:
+            assert arg.name and arg.type
+
+
+def test_search_ranks_the_command_above_the_prose():
+    results = xml_index.search("takeoff")
+    assert results
+    assert results[0].name == "TakeOff"
+    assert any(c.full_name == "ardrone3.PilotingState.FlyingStateChanged" for c in xml_index.search("flying"))
+    assert xml_index.search("   ") == []
+
+
+def test_myclass_spelling_also_loads(tmp_path, monkeypatch):
+    """pyparrot renamed <class> to <myclass>; a copy from it must still parse."""
+    source = xml_index.xml_dir()
+    for name in xml_index.XML_FILES:
+        text = (source / name).read_text()
+        text = text.replace("<class ", "<myclass ").replace("</class>", "</myclass>")
+        (tmp_path / name).write_text(text)
+
+    monkeypatch.setenv(xml_index.XML_DIR_ENV, str(tmp_path))
+    xml_index.index.cache_clear()
+    try:
+        assert len(xml_index.all_commands()) == TOTAL_WITH_FLAT_TRIM
+    finally:
+        monkeypatch.delenv(xml_index.XML_DIR_ENV)
+        xml_index.index.cache_clear()
+
+
+def test_missing_xml_dir_says_what_to_set(tmp_path, monkeypatch):
+    real = xml_index.xml_dir()
+    shutil.copy(real / "common.xml", tmp_path / "common.xml")  # one of the two, not both
+    monkeypatch.setenv(xml_index.XML_DIR_ENV, str(tmp_path))
+    xml_index.index.cache_clear()
+    try:
+        with pytest.raises(FileNotFoundError, match=xml_index.XML_DIR_ENV):
+            xml_index.all_commands()
+    finally:
+        monkeypatch.delenv(xml_index.XML_DIR_ENV)
+        xml_index.index.cache_clear()
+
+
+@pytest.mark.parametrize(
+    ("support", "expected"),
+    [
+        ("drones", True),
+        ("none", False),
+        ("0901;090c;090e", True),
+        ("0901;090e", False),
+        ("090c", True),
+        ("090c:4.3.0", True),  # firmware 4.7.1 clears a 4.3.0 minimum
+        ("090c:4.8.0", False),
+        ("0901:2.0.29;090c;090e", True),
+        ("0914:1.6.3;0919:1.6.3", False),
+        ("090e;090c:4.3.0", True),
+        (None, None),
+        ("", None),
+        ("   ", None),
+    ],
+)
+def test_supports_bebop2(support, expected):
+    assert xml_index.supports_bebop2(support) is expected
+
+
+def test_support_versions_compare_as_integers_not_strings():
+    # Lexically "2.0.29" < "2.0.3", which would make this unsupported.
+    assert xml_index.supports_bebop2("090c:2.0.29", firmware=(2, 0, 3)) is False
+    assert xml_index.supports_bebop2("090c:2.0.3", firmware=(2, 0, 29)) is True
+    assert xml_index.supports_bebop2("090c:10.0.0", firmware=(9, 0, 0)) is False
+
+
+def test_unknown_support_is_not_unsupported():
+    unknown = [c for c in xml_index.all_commands() if xml_index.supports_bebop2(c.support) is None]
+    # 30 commands carry a <comment> with no support attribute, 2 carry an empty
+    # one, 6 have no <comment> at all, and FlatTrim is hand-added.
+    assert len(unknown) == 39
+    assert xml_index.get("ardrone3.Piloting.FlatTrim") in unknown