From 66ed034cf70559c9cfd4b99902558eb0bb20f91c Mon Sep 17 00:00:00 2001 From: Shahriar Ahnaf Date: Wed, 12 Aug 2026 14:53:25 -0400 Subject: [PATCH 1/6] fix: BLE page-turner defects (map wipe, capture races, false "connecting") MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five defects found while driving the branch end-to-end against a simulated BLE HID page-turner. 1. BleButtonMapActivity::onEnter() wiped the entire bleKeyMap and saved it before the user did anything, so opening "Map Remote Buttons" — including opening it only to read the binding list this activity renders — destroyed every existing mapping, with no confirmation and no undo. It also made that list permanently empty, since onEnter had just cleared what it displays. Nothing needed the wipe: assignCapturedKey() already drops any other key bound to the action being assigned and reuses the slot of a re-captured key, so neither a stale action nor a duplicate binding can survive a re-map. 2. Keys arriving during Step::SelectFunction stayed latched in the capture buffer. Since the host synthesizes auto-repeats for a held key, a repeat that landed while the user was choosing a function was then consumed as the *next* button they "pressed". Stale captures are now discarded while selecting. 3. In capture mode pollBle() overwrote the captured key on every iteration of its drain loop, so when two reports arrived in one frame the key the user actually pressed was replaced by whatever followed it. Keep the first unconsumed key instead. 4. The reader's status bar replaced the book title with "BT Connecting..." whenever Bluetooth was enabled and not connected, unbounded. A bonded remote that has gone to sleep stops advertising until a button is pressed on it, so the title could be held hostage indefinitely for a connection that was not in progress. It now requires the host to actually be running with a bond, and gives the title back after a bounded window. EpubReaderMenuActivity had the mirror bug, reporting "connecting" when the lifecycle had deliberately stopped the stack; that state is PAUSED. 5. bleKeyMap has no defaults and pollBle() drops any key it cannot resolve, so a remote that has never been through the mapping screen is inert — its keys are decoded correctly and then discarded, which presents as broken rather than unconfigured. Standard navigation keys (arrows, page up/down, enter, escape) now have out-of-the-box bindings, consulted only for keys the user has not bound, so explicit mappings still win. Vendor and consumer-page codes carry no portable meaning and remain capture-and-assign. --- src/MappedInputManager.cpp | 60 ++++++++++++++++--- src/activities/reader/EpubReaderActivity.cpp | 24 +++++++- src/activities/reader/EpubReaderActivity.h | 11 ++++ .../reader/EpubReaderMenuActivity.cpp | 5 +- .../settings/BleButtonMapActivity.cpp | 22 +++++-- 5 files changed, 107 insertions(+), 15 deletions(-) diff --git a/src/MappedInputManager.cpp b/src/MappedInputManager.cpp index 24da368ad9d..999ee0fa90b 100644 --- a/src/MappedInputManager.cpp +++ b/src/MappedInputManager.cpp @@ -138,6 +138,40 @@ bool MappedInputManager::bleEdge(const bool* arr, const Button button) const { } namespace { +// Out-of-the-box bindings for keys whose meaning is unambiguous. Without these a +// remote that has never been through Settings -> Bluetooth -> Map Remote Buttons is +// inert: pollBle() decodes its key correctly, finds no entry in an empty bleKeyMap, +// and drops it, so the device looks broken rather than unconfigured. Consulted ONLY +// for a key that has no explicit binding, so anything the user maps still wins. +// Deliberately limited to the standard navigation keys; vendor and consumer-page +// codes carry no portable meaning and stay capture-and-assign. +struct DefaultBinding { + freeink::SpecialKey key; + MappedInputManager::Button button; +}; +constexpr DefaultBinding kDefaultBindings[] = { + {freeink::SpecialKey::Right, MappedInputManager::Button::PageForward}, + {freeink::SpecialKey::PageDown, MappedInputManager::Button::PageForward}, + {freeink::SpecialKey::Left, MappedInputManager::Button::PageBack}, + {freeink::SpecialKey::PageUp, MappedInputManager::Button::PageBack}, + {freeink::SpecialKey::Up, MappedInputManager::Button::Up}, + {freeink::SpecialKey::Down, MappedInputManager::Button::Down}, + {freeink::SpecialKey::Enter, MappedInputManager::Button::Confirm}, + {freeink::SpecialKey::Escape, MappedInputManager::Button::Back}, +}; + +// Resolve a decoded identity against the defaults. kind 1 is a raw HID usage or a +// vendor code with no portable meaning, so only kind 0 (SpecialKey) is considered. +bool defaultBindingFor(const uint8_t kind, const uint8_t value, uint8_t& button) { + if (kind != 0) return false; + for (const auto& d : kDefaultBindings) { + if (static_cast(d.key) != value) continue; + button = static_cast(d.button); + return true; + } + return false; +} + constexpr float LEFT_EDGE_BACK_GESTURE_FRAC_X = 0.25f; constexpr float BOTTOM_EDGE_BACK_GESTURE_FRAC_Y = 0.14f; constexpr float TOP_EDGE_MENU_GESTURE_FRAC_Y = 0.14f; @@ -385,21 +419,31 @@ void MappedInputManager::pollBle() { if (!bleinput::encodeKey(ev, kind, value)) continue; if (bleCaptureMode) { - bleCapturedKind = kind; - bleCapturedValue = value; - bleHasCaptured = true; + // Keep the FIRST unconsumed key, not the last. Overwriting meant that when a + // remote emitted two reports in one frame (a press plus the host's synthetic + // repeat, or a composite device notifying on two characteristics) the key the + // user actually pressed was replaced by whatever followed it. + if (!bleHasCaptured) { + bleCapturedKind = kind; + bleCapturedValue = value; + bleHasCaptured = true; + } continue; } - // Resolve the key identity against the persisted mapping table. + // Resolve the key identity against the persisted mapping table, falling back to + // the standard-navigation defaults when the user has not bound this key. + uint8_t button = 0xFF; for (const auto& e : SETTINGS.bleKeyMap) { if (e.button == 0xFF || e.keyKind != kind || e.keyValue != value) continue; - if (e.button < kButtonCount) { - blePressEdge[e.button] = true; - bleActivityThisFrame = true; - } + button = e.button; break; } + if (button == 0xFF && !defaultBindingFor(kind, value, button)) continue; + if (button < kButtonCount) { + blePressEdge[button] = true; + bleActivityThisFrame = true; + } } } diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index c7da97fb8ac..fbf5072d95a 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -2079,6 +2079,28 @@ void EpubReaderActivity::renderContents(std::unique_ptr page, const int or } } +bool EpubReaderActivity::bleConnectingTitleTakesOver() const { + // Nothing to report when BT is off, already linked, or the stack isn't even up + // (the lifecycle stops it outside a reader and under heap pressure — that is "off", + // not "connecting", and claiming otherwise misreads as a stuck connection). + if (!SETTINGS.bluetoothEnabled || !BleHid.isRunning() || BleHid.isConnected()) { + bleConnectingSince = 0; + return false; + } + // With no bond there is no remote to connect to: the host is idle, not connecting. + if (BleHid.pairedCount() == 0) { + bleConnectingSince = 0; + return false; + } + const unsigned long now = millis(); + if (bleConnectingSince == 0) bleConnectingSince = now; + // A bonded remote that has gone to sleep stops advertising until the user presses a + // button on it, so the attempt can stay outstanding forever. Give the title back + // rather than holding it hostage; the watcher in loop() restores the placeholder if + // the link state flips again. + return now - bleConnectingSince < BLE_CONNECTING_TITLE_MS; +} + void EpubReaderActivity::renderStatusBar() const { // Calculate progress in book. Use the estimated total while a giant spine is still building so // "page X of Y" and the progress bar don't read off the small build watermark. @@ -2115,7 +2137,7 @@ void EpubReaderActivity::renderStatusBar() const { title = epub->getTitle(); } - if (SETTINGS.bluetoothEnabled && !BleHid.isConnected()) { + if (bleConnectingTitleTakesOver()) { // Take over the title slot entirely while connecting; the watcher in // loop() redraws the bar on the connect/disconnect flip, restoring the // chapter/book title. diff --git a/src/activities/reader/EpubReaderActivity.h b/src/activities/reader/EpubReaderActivity.h index 458edded46d..6b547c07287 100644 --- a/src/activities/reader/EpubReaderActivity.h +++ b/src/activities/reader/EpubReaderActivity.h @@ -60,6 +60,17 @@ class EpubReaderActivity final : public Activity { // to the chapter/book title the moment the connection completes (and // returns on disconnect) instead of waiting for the next page turn. bool statusBarBleConnected = false; + // When the current "BT Connecting..." status-bar takeover started. The placeholder + // is only honest for as long as a connection attempt is plausibly in flight; past + // that the book title comes back. 0 = not currently claiming the title. + mutable unsigned long bleConnectingSince = 0; + // How long the status bar may show "BT Connecting...". A remote that has gone to + // sleep stops advertising and will not come back until the user presses a button on + // it, so an unbounded placeholder costs the user their book title indefinitely for a + // connection that is not actually in progress. + static constexpr unsigned long BLE_CONNECTING_TITLE_MS = 20000; + // True while the status bar should show "BT Connecting..." instead of the title. + bool bleConnectingTitleTakesOver() const; // Idle-time glyph prewarm: after a page settles, scan the LIKELY next page // (scan mode draws nothing) and load its missing glyphs from SD during idle, // so the next turn's in-render prewarm is a cache hit instead of ~100 ms of diff --git a/src/activities/reader/EpubReaderMenuActivity.cpp b/src/activities/reader/EpubReaderMenuActivity.cpp index beb885a6541..a16a4c730b8 100644 --- a/src/activities/reader/EpubReaderMenuActivity.cpp +++ b/src/activities/reader/EpubReaderMenuActivity.cpp @@ -222,7 +222,10 @@ void EpubReaderMenuActivity::render(RenderLock&&) { return pageTurnLabels[selectedPageTurnOption]; } else if (value == MenuAction::TOGGLE_BLUETOOTH) { if (SETTINGS.bluetoothEnabled) { - if (!BleHid.isRunning()) return tr(STR_CONNECTING); + // Stack down: the lifecycle stopped it (non-reader activity, WiFi, or the + // heap gate). That is paused, not connecting — reporting "connecting" for + // a host that is not even running reads as a connection stuck forever. + if (!BleHid.isRunning()) return tr(STR_STATE_PAUSED); return BleHid.isConnected() ? tr(STR_STATE_ON) : tr(STR_CONNECTING); } return tr(STR_STATE_OFF); diff --git a/src/activities/settings/BleButtonMapActivity.cpp b/src/activities/settings/BleButtonMapActivity.cpp index 61618d8bcdf..00a619abe85 100644 --- a/src/activities/settings/BleButtonMapActivity.cpp +++ b/src/activities/settings/BleButtonMapActivity.cpp @@ -30,11 +30,12 @@ void BleButtonMapActivity::onEnter() { step = Step::WaitForKey; capturedKind = 0xFF; functionIndex = 0; - // Start every mapping session from a clean slate: the user re-maps each remote - // button once, so a button can't be left bound to a stale action and there's no - // separate "clear mappings" step to remember. - std::fill(std::begin(SETTINGS.bleKeyMap), std::end(SETTINGS.bleKeyMap), CrossPointSettings::BleKeyMapEntry{}); - SETTINGS.saveToFile(); + // Existing bindings are kept. Wiping the table here destroyed every mapping the + // moment the user opened this screen — including when they opened it only to read + // the list this activity renders, which was therefore always empty. Nothing needs + // the wipe: assignCapturedKey() already drops any other key bound to the action it + // is assigning, and re-capturing a key reuses that key's existing slot, so neither + // a stale action nor a duplicate binding can survive a re-map. mappedInput.setBleCaptureMode(true); requestUpdate(); } @@ -98,6 +99,17 @@ void BleButtonMapActivity::loop() { } // Step::SelectFunction — pick a logical function for the captured key. + // Drop anything the remote sends while the user is choosing. pollBle() keeps + // latching in capture mode, and the host emits synthetic auto-repeats for a held + // key, so without this the repeat that arrived during selection was still sitting + // in the buffer when we returned to WaitForKey and was consumed as the *next* + // button the user "pressed". + { + uint8_t staleKind = 0xFF; + uint8_t staleValue = 0; + mappedInput.takeCapturedBleKey(staleKind, staleValue); + } + buttonNavigator.onNext([this] { functionIndex = ButtonNavigator::nextIndex(functionIndex, kFunctionCount); requestUpdate(); From 1ddb41f68895f12c36b301f5a058a322d462c489 Mon Sep 17 00:00:00 2001 From: Shahriar Ahnaf Date: Wed, 12 Aug 2026 14:57:11 -0400 Subject: [PATCH 2/6] test: BLE page-turner bench on simulated hardware, both remote encodings Runs the real image on a simulated Xteink X4 and pairs it with a firmware-less BLE HID peer over a simulated air interface -- real scan, SMP, session key, GATT discovery and input reports, nothing stubbed between the firmware and the link. Both remote encodings are parameterised into every test, because they take different routes through BleKeyboardHost::onReportIngest and only the first was ever exercised: * keyboard -- 8-byte boot-protocol report carrying a HID usage. Takes the standard keyboard slot path, surfaces as a SpecialKey, and so must page without any mapping now that the default bindings exist. * bitmap -- what a Hanlinyue "Free 2" actually emits, per the teardown posted on crosspoint-reader#2418: 3-byte reports, top 20 00 00, bottom 04 00 00, explicit release 00 00 00. Not keyboard usages and not consumer usage IDs; they reach the generic fallback where the identity is the first non-zero byte. Vendor codes carry no portable meaning, so these stay capture-and-assign -- the test pins the decode, not a default binding. The Report Map assertion is load-bearing beyond discovery: the map is 49 octets for the keyboard peer and longer once the bitmap peer adds its consumer collection, so it cannot be read in one ATT response at the default MTU. A regression in Read Blob continuation fails there rather than silently. Payload and board model are simulator artifacts that are not in this repo, so the tests skip in a bare checkout instead of failing. --- test/sim/README.md | 60 ++++ test/sim/ble_hid_peer_bitmap.py | 331 +++++++++++++++++++++++ test/sim/test_ble_page_turner.py | 121 +++++++++ test/sim/xteink_pageturner_bitmap.yaml | 21 ++ test/sim/xteink_pageturner_keyboard.yaml | 21 ++ 5 files changed, 554 insertions(+) create mode 100644 test/sim/README.md create mode 100644 test/sim/ble_hid_peer_bitmap.py create mode 100644 test/sim/test_ble_page_turner.py create mode 100644 test/sim/xteink_pageturner_bitmap.yaml create mode 100644 test/sim/xteink_pageturner_keyboard.yaml diff --git a/test/sim/README.md b/test/sim/README.md new file mode 100644 index 00000000000..33dc4d3ac39 --- /dev/null +++ b/test/sim/README.md @@ -0,0 +1,60 @@ +# BLE page-turner tests on simulated hardware + +These tests run the real CrossPoint image on a simulated Xteink X4 (ESP32-C3) and +pair it with a firmware-less BLE HID page-turner over a simulated air interface. +Nothing is stubbed between the firmware and the link: the scan, the SMP exchange, +the session key, GATT discovery and the HID input reports are all real traffic +between two models. + +They exist because the page-turner work in #2418 has failure modes that only show +up end to end, and that a person holding a remote cannot easily distinguish from +each other — "the remote does nothing" is the same symptom whether the key never +arrived, arrived and was dropped, or arrived at a screen with nothing to act on. + +## What is covered + +| Test | What breaks if it fails | +| --- | --- | +| `test_remote_is_discovered_and_paired` | Active scan across the SCAN_REQ/SCAN_RSP turnaround; SMP; the encrypted link | +| `test_report_map_is_read_and_reports_are_subscribed` | HID service discovery, Report Map read (too long for one ATT response — needs Read Blob continuation), report CCC subscribe | +| `test_arrow_key_turns_a_page_without_being_mapped` | The default bindings. Without them an unmapped remote is inert | + +## The two remote encodings + +Both are parameterised into every test, because they take different routes +through `BleKeyboardHost::onReportIngest` and only the first was ever exercised: + +- **keyboard** — an 8-byte boot-protocol report carrying a HID usage. Takes the + standard keyboard slot path and surfaces as a `SpecialKey`, so the default + bindings apply and the key works with no mapping. +- **bitmap** — the encoding a Hanlinyue **Free 2** actually emits, reverse + engineered by a tester on #2418: 3-byte reports, top button `20 00 00`, bottom + `04 00 00`, explicit release `00 00 00`. These are *not* keyboard usages and + *not* consumer usage IDs. They fall through to the generic fallback, where the + identity is the first non-zero byte. Because such vendor codes carry no + portable meaning, they stay capture-and-assign — the bitmap test pins the + decode, not a default binding. + +The same remote emits different encodings in its other power-button-cycled modes, +which is exactly why the capture-then-assign design is right and why hardcoded +per-vendor profiles would be fragile. + +## Running + +The firmware payload and the board model are build/simulator artifacts that do +not live in this repo, so the tests **skip** in a bare checkout rather than fail. +To run them you need a simulator binary and a rundir containing +`board-featbt-hidpeer.replx` and its helper scripts: + +```bash +pytest test/sim/test_ble_page_turner.py -v --sim +``` + +Every assertion reads the firmware's own log. UART capture block-buffers at 8 KB +and this journey emits well under that in 240 virtual seconds, so nothing reaches +the file the harness polls until the process exits. The tests therefore also skip +unless `SIMANTIC_UART_AUTOFLUSH=1` marks a simulator that flushes as it goes. + +The scenario spends roughly 110 virtual seconds walking the firmware's own UI to +the Bluetooth screen before BLE matters. That is deliberate — it is the bench that +proves the whole stack — and it is not meant to run on every commit. diff --git a/test/sim/ble_hid_peer_bitmap.py b/test/sim/ble_hid_peer_bitmap.py new file mode 100644 index 00000000000..87ef265c153 --- /dev/null +++ b/test/sim/ble_hid_peer_bitmap.py @@ -0,0 +1,331 @@ +# Scripted BLE HID-over-GATT page-turner (ScriptedBlePeer Python model). +# Advertising (name "PageTurner", service 0x1812) is the C# model's job; this +# file serves the GATT a HID host walks after connecting: the HID service with +# a keyboard report map and one input report, plus Battery and Device Info, +# which HID hosts commonly read during enumeration. +# +# The link layer, ARQ and LLCP are the C# model's. SMP is not modelled by the +# peer at all -- if the CrossPoint host insists on bonding before subscribing, +# that will be visible as an SMP Pairing Request PDU this script never answers. + +ATT_ERROR_RSP = 0x01 +ATT_FIND_INFORMATION_REQ = 0x04 +ATT_FIND_INFORMATION_RSP = 0x05 +ATT_FIND_BY_TYPE_VALUE_REQ = 0x06 +ATT_FIND_BY_TYPE_VALUE_RSP = 0x07 +ATT_READ_BY_TYPE_REQ = 0x08 +ATT_READ_BY_TYPE_RSP = 0x09 +ATT_READ_REQ = 0x0A +ATT_READ_RSP = 0x0B +ATT_READ_BLOB_REQ = 0x0C +ATT_READ_BLOB_RSP = 0x0D +ATT_READ_BY_GROUP_TYPE_REQ = 0x10 +ATT_READ_BY_GROUP_TYPE_RSP = 0x11 +ATT_WRITE_REQ = 0x12 +ATT_WRITE_RSP = 0x13 +ATT_WRITE_CMD = 0x52 + +ATT_ERR_ATTRIBUTE_NOT_FOUND = 0x0A +ATT_ERR_INVALID_OFFSET = 0x07 + +# The peer answers Exchange MTU with the 23-octet default (Core Vol 3 Part G +# 5.2.1), so a response body is at most ATT_MTU - 1 octets. The HID Report Map +# is longer than that, which is exactly the case Read Blob exists for. +ATT_MTU = 23 + +UUID_PRIMARY_SERVICE = 0x2800 +UUID_CHARACTERISTIC = 0x2803 +UUID_CCC = 0x2902 +UUID_REPORT_REFERENCE = 0x2908 + +UUID_HID_SERVICE = 0x1812 +UUID_HID_INFORMATION = 0x2A4A +UUID_REPORT_MAP = 0x2A4B +UUID_HID_CONTROL_POINT = 0x2A4C +UUID_REPORT = 0x2A4D +UUID_PROTOCOL_MODE = 0x2A4E +UUID_BATTERY_SERVICE = 0x180F +UUID_BATTERY_LEVEL = 0x2A19 + +CHAR_PROP_READ = 0x02 +CHAR_PROP_WRITE_NR = 0x04 +CHAR_PROP_WRITE = 0x08 +CHAR_PROP_NOTIFY = 0x10 + +# -- handle layout ---------------------------------------------------------- +# Battery service first (hosts read it during enumeration), HID second. +H_BAT_SERVICE = 0x0001 # group 0x0001..0x0003 +H_BAT_DECL = 0x0002 +H_BAT_VALUE = 0x0003 +H_HID_SERVICE = 0x0010 # group 0x0010..0x001B +H_HIDINFO_DECL = 0x0011 +H_HIDINFO_VALUE = 0x0012 +H_REPORTMAP_DECL = 0x0013 +H_REPORTMAP_VALUE = 0x0014 +H_PROTOMODE_DECL = 0x0015 +H_PROTOMODE_VALUE = 0x0016 +H_REPORT_DECL = 0x0017 +H_REPORT_VALUE = 0x0018 +H_REPORT_CCC = 0x0019 +H_REPORT_REF = 0x001A # Report Reference: report ID 1, input +H_CTRLPOINT_DECL = 0x001B +H_CTRLPOINT_VALUE = 0x001C +H_HID_LAST = H_CTRLPOINT_VALUE + +# Minimal keyboard report map: 8-byte boot-style input report (modifiers, +# reserved, 6 keycodes), report ID 1. +REPORT_MAP = bytes([ + 0x05, 0x01, # Usage Page (Generic Desktop) + 0x09, 0x06, # Usage (Keyboard) + 0xA1, 0x01, # Collection (Application) + 0x85, 0x01, # Report ID (1) + 0x05, 0x07, # Usage Page (Key Codes) + 0x19, 0xE0, 0x29, 0xE7, # Usage Min/Max (modifiers) + 0x15, 0x00, 0x25, 0x01, # Logical 0..1 + 0x75, 0x01, 0x95, 0x08, # 1 bit x 8 + 0x81, 0x02, # Input (Data, Var, Abs) - modifier byte + 0x75, 0x08, 0x95, 0x01, # 8 bits x 1 + 0x81, 0x01, # Input (Const) - reserved + 0x75, 0x08, 0x95, 0x06, # 8 bits x 6 + 0x15, 0x00, 0x25, 0x65, # Logical 0..0x65 + 0x19, 0x00, 0x29, 0x65, # Usage 0..0x65 + 0x81, 0x00, # Input (Data, Array) - keys + 0xC0, # End Collection + # Second collection: Consumer Control. Its presence is what makes the host's + # report-map hint parser report consumer=1 alongside kbd=1, which is the + # combination a real Free 2 presents (kbd=1 consumer=1 preferredByte=2). + 0x05, 0x0C, # Usage Page (Consumer) + 0x09, 0x01, # Usage (Consumer Control) + 0xA1, 0x01, # Collection (Application) + 0x85, 0x02, # Report ID (2) + 0x15, 0x00, 0x26, 0xFF, 0x00, # Logical 0..255 + 0x19, 0x00, 0x2A, 0xFF, 0x00, # Usage 0..255 + 0x75, 0x08, 0x95, 0x03, # 8 bits x 3 -- the 3-byte button bitmap + 0x81, 0x00, # Input (Data, Array) + 0xC0, # End Collection +]) + +# HID Information: bcdHID 1.11, country 0, flags RemoteWake|NormallyConnectable +HID_INFO = bytes([0x11, 0x01, 0x00, 0x03]) + +KEY_RIGHT_ARROW = 0x4F + +# Button bitmap codes, taken from a teardown of a Hanlinyue "Free 2" page-turner +# posted on crosspoint-reader#2418: the remote emits 3-byte bitmap reports rather +# than keyboard usages or consumer usage IDs, one clean press/release pair per +# physical press. Each button raises its own bit in byte 0. +BITMAP_TOP = bytes([0x20, 0x00, 0x00]) +BITMAP_BOTTOM = bytes([0x04, 0x00, 0x00]) +BITMAP_RELEASE = bytes([0x00, 0x00, 0x00]) + +SERVICES = [ + # (service handle, last handle, 16-bit uuid) + (H_BAT_SERVICE, H_BAT_VALUE, UUID_BATTERY_SERVICE), + (H_HID_SERVICE, H_HID_LAST, UUID_HID_SERVICE), +] + +CHARACTERISTICS = [ + # (declaration handle, value handle, properties, uuid) + (H_BAT_DECL, H_BAT_VALUE, CHAR_PROP_READ, UUID_BATTERY_LEVEL), + (H_HIDINFO_DECL, H_HIDINFO_VALUE, CHAR_PROP_READ, UUID_HID_INFORMATION), + (H_REPORTMAP_DECL, H_REPORTMAP_VALUE, CHAR_PROP_READ, UUID_REPORT_MAP), + (H_PROTOMODE_DECL, H_PROTOMODE_VALUE, + CHAR_PROP_READ | CHAR_PROP_WRITE_NR, UUID_PROTOCOL_MODE), + (H_REPORT_DECL, H_REPORT_VALUE, + CHAR_PROP_READ | CHAR_PROP_NOTIFY, UUID_REPORT), + (H_CTRLPOINT_DECL, H_CTRLPOINT_VALUE, CHAR_PROP_WRITE_NR, UUID_HID_CONTROL_POINT), +] + +DESCRIPTORS = { + H_REPORT_CCC: UUID_CCC, + H_REPORT_REF: UUID_REPORT_REFERENCE, +} + +READ_VALUES = { + H_BAT_VALUE: bytes([90]), + H_HIDINFO_VALUE: HID_INFO, + H_REPORTMAP_VALUE: REPORT_MAP, + H_PROTOMODE_VALUE: bytes([0x01]), # report protocol + H_REPORT_VALUE: bytes(3), + H_REPORT_REF: bytes([0x01, 0x01]), # report ID 1, input report +} + +# Once subscribed, click the top button -- press then explicit release, the pair a +# real Free 2 emits. Repeated so a click also lands after the UI has been driven +# somewhere the key can act; this is bench stimulus (when a human presses the +# remote), not a modelled quantity. +KEYPRESS_DELAY_US = 100_000 +REPEAT_PERIOD_US = 15_000_000 + +CLICK_TOP = (BITMAP_TOP, BITMAP_RELEASE) +CLICK_BOTTOM = (BITMAP_BOTTOM, BITMAP_RELEASE) + + +def u16(value): + return bytes([value & 0xFF, (value >> 8) & 0xFF]) + + +class Peer: + def __init__(self, ctx): + self.ctx = ctx + self.subscribed = False + self.queue = [] + + def on_connect(self): + self.subscribed = False + self.queue = [] + self.ctx.info("connected; serving HID over GATT") + + def on_disconnect(self, reason): + # Clear the subscription: without this the peer keeps "notifying" into a link + # that no longer exists, and the log reads as if keys were still delivered. + self.subscribed = False + self.queue = [] + self.ctx.info(f"disconnected (reason 0x{reason:02x})") + + def on_timer(self): + if not self.queue: + return + report = self.queue.pop(0) + self.ctx.info("sending HID bitmap report %s" % report.hex()) + self.ctx.notify(H_REPORT_VALUE, report) + if self.queue: + self.ctx.schedule_oneshot(KEYPRESS_DELAY_US) + elif self.subscribed: + self.queue = list(CLICK_TOP) + self.ctx.schedule_oneshot(REPEAT_PERIOD_US) + + def on_att(self, pdu): + opcode = pdu[0] + if opcode == ATT_READ_BY_GROUP_TYPE_REQ: + self._read_by_group_type(pdu) + elif opcode == ATT_READ_BY_TYPE_REQ: + self._read_by_type(pdu) + elif opcode == ATT_FIND_BY_TYPE_VALUE_REQ: + self._find_by_type_value(pdu) + elif opcode == ATT_FIND_INFORMATION_REQ: + self._find_information(pdu) + elif opcode == ATT_READ_REQ: + self._read(pdu) + elif opcode == ATT_READ_BLOB_REQ: + self._read_blob(pdu) + elif opcode == ATT_WRITE_REQ: + self._write(pdu, respond=True) + elif opcode == ATT_WRITE_CMD: + self._write(pdu, respond=False) + elif opcode % 2 == 0 and not opcode & 0x40: + # A request left unanswered stalls the client's ATT queue forever. + self._error(pdu, ATT_ERR_ATTRIBUTE_NOT_FOUND) + + # -- discovery --------------------------------------------------------- + + def _read_by_group_type(self, pdu): + start, end, uuid = self._range_and_uuid(pdu) + if uuid != UUID_PRIMARY_SERVICE: + self._error(pdu, ATT_ERR_ATTRIBUTE_NOT_FOUND) + return + for svc, last, svc_uuid in SERVICES: + if start <= svc <= end: + self.ctx.send(bytes([ATT_READ_BY_GROUP_TYPE_RSP, 6]) + + u16(svc) + u16(last) + u16(svc_uuid)) + return + self._error(pdu, ATT_ERR_ATTRIBUTE_NOT_FOUND) + + def _find_by_type_value(self, pdu): + start = pdu[1] | (pdu[2] << 8) + end = pdu[3] | (pdu[4] << 8) + attr_type = pdu[5] | (pdu[6] << 8) + value = pdu[7] | (pdu[8] << 8) if len(pdu) >= 9 else None + if attr_type == UUID_PRIMARY_SERVICE: + for svc, last, svc_uuid in SERVICES: + if value == svc_uuid and start <= svc <= end: + self.ctx.send(bytes([ATT_FIND_BY_TYPE_VALUE_RSP]) + + u16(svc) + u16(last)) + return + self._error(pdu, ATT_ERR_ATTRIBUTE_NOT_FOUND) + + def _read_by_type(self, pdu): + start, end, uuid = self._range_and_uuid(pdu) + if uuid == UUID_CHARACTERISTIC: + for decl, value_handle, props, char_uuid in CHARACTERISTICS: + if start <= decl <= end: + self.ctx.send(bytes([ATT_READ_BY_TYPE_RSP, 7]) + + u16(decl) + bytes([props]) + + u16(value_handle) + u16(char_uuid)) + return + elif uuid == UUID_REPORT_REFERENCE and start <= H_REPORT_REF <= end: + # Read-by-type on the Report Reference is how a HID host maps + # report handles to report IDs. + self.ctx.send(bytes([ATT_READ_BY_TYPE_RSP, 4]) + + u16(H_REPORT_REF) + READ_VALUES[H_REPORT_REF]) + return + self._error(pdu, ATT_ERR_ATTRIBUTE_NOT_FOUND) + + def _find_information(self, pdu): + start = pdu[1] | (pdu[2] << 8) + end = pdu[3] | (pdu[4] << 8) + for handle in sorted(DESCRIPTORS): + if start <= handle <= end: + self.ctx.send(bytes([ATT_FIND_INFORMATION_RSP, 0x01]) + + u16(handle) + u16(DESCRIPTORS[handle])) + return + self._error(pdu, ATT_ERR_ATTRIBUTE_NOT_FOUND) + + # -- reads / writes ---------------------------------------------------- + + def _read(self, pdu): + handle = pdu[1] | (pdu[2] << 8) + if handle in READ_VALUES: + # Core Vol 3 Part F 3.4.4.4: the response carries at most ATT_MTU-1 + # octets of the value. A client that sees a full-length response + # asks for the rest with Read Blob, so truncating here is the + # protocol, not a shortcut. + self.ctx.send(bytes([ATT_READ_RSP]) + READ_VALUES[handle][:ATT_MTU - 1]) + elif handle == H_REPORT_CCC: + self.ctx.send(bytes([ATT_READ_RSP, 1 if self.subscribed else 0, 0x00])) + else: + self._error(pdu, ATT_ERR_ATTRIBUTE_NOT_FOUND) + + def _read_blob(self, pdu): + # Core Vol 3 Part F 3.4.4.5: continue a value the Read Response had to + # truncate. An offset equal to the value's length is legal and answers + # with an empty body -- that is how the client learns it has the whole + # value; only an offset PAST the end is an error. + handle = pdu[1] | (pdu[2] << 8) + offset = pdu[3] | (pdu[4] << 8) + if handle not in READ_VALUES: + self._error(pdu, ATT_ERR_ATTRIBUTE_NOT_FOUND) + return + value = READ_VALUES[handle] + if offset > len(value): + self._error(pdu, ATT_ERR_INVALID_OFFSET) + return + self.ctx.send(bytes([ATT_READ_BLOB_RSP]) + + value[offset:offset + ATT_MTU - 1]) + + def _write(self, pdu, respond): + handle = pdu[1] | (pdu[2] << 8) + if handle == H_REPORT_CCC: + self.subscribed = len(pdu) >= 4 and (pdu[3] & 0x01) != 0 + if respond: + self.ctx.send(bytes([ATT_WRITE_RSP])) + if self.subscribed: + # Page turn: right-arrow press, then release. + self.queue = list(CLICK_TOP) + self.ctx.schedule_oneshot(KEYPRESS_DELAY_US) + elif handle in (H_PROTOMODE_VALUE, H_CTRLPOINT_VALUE): + if respond: + self.ctx.send(bytes([ATT_WRITE_RSP])) + else: + if respond: + self._error(pdu, ATT_ERR_ATTRIBUTE_NOT_FOUND) + + def _range_and_uuid(self, pdu): + start = pdu[1] | (pdu[2] << 8) + end = pdu[3] | (pdu[4] << 8) + uuid = pdu[5] | (pdu[6] << 8) + return start, end, uuid + + def _error(self, pdu, code): + handle = (pdu[1] | (pdu[2] << 8)) if len(pdu) >= 3 else 0 + self.ctx.send(bytes([ATT_ERROR_RSP, pdu[0]]) + u16(handle) + bytes([code])) diff --git a/test/sim/test_ble_page_turner.py b/test/sim/test_ble_page_turner.py new file mode 100644 index 00000000000..3656283b6f3 --- /dev/null +++ b/test/sim/test_ble_page_turner.py @@ -0,0 +1,121 @@ +"""BLE page-turner regression tests, run against simulated hardware. + +These exercise the paths that broke in the field on crosspoint-reader#2418, using +a firmware-less BLE HID peer on the other end of a simulated air interface. The +firmware under test is the real image; nothing is stubbed between it and the link. + +Two remote encodings are covered, because they take different routes through +BleKeyboardHost::onReportIngest and only one of them was ever exercised: + + * ``keyboard`` -- an 8-byte boot-protocol report carrying a HID usage. Decodes + through the standard keyboard slot path and arrives as a SpecialKey, so the + default bindings apply and the key works without any mapping. + * ``bitmap`` -- the 3-byte report a Hanlinyue "Free 2" actually emits (top + ``20 00 00``, bottom ``04 00 00``, release ``00 00 00``), reverse-engineered + by a tester on #2418. Not keyboard usages and not consumer usage IDs: it + reaches the generic fallback, where the identity is the first non-zero byte. + Vendor codes like these carry no portable meaning, so they stay + capture-and-assign -- this test pins the decode, not a default binding. + +Run with: + + pytest test/sim/test_ble_page_turner.py -v --sim +""" +import os +from pathlib import Path + +import pytest +import yaml + +HERE = Path(__file__).resolve().parent + +# The journey walks the firmware's own UI to the Bluetooth screen before BLE +# matters, which costs ~110s of virtual time in menu navigation and e-ink +# repaints. Generous by design: this is the bench that proves the whole stack, +# not a per-commit test. +JOURNEY_TIMEOUT = 240 + +ENCODINGS = ["keyboard", "bitmap"] + + +def _scenario(encoding: str) -> Path: + return HERE / f"xteink_pageturner_{encoding}.yaml" + + +def _payload(encoding: str = "keyboard") -> Path: + """The scenario's firmware, or a path that does not exist in a bare checkout.""" + spec = yaml.safe_load(_scenario(encoding).read_text()) + return Path(spec["machines"]["xteink"]["elf"]) + + +pytestmark = [ + pytest.mark.skipif( + not _payload().exists(), + reason=f"CrossPoint payload not present at {_payload()}", + ), + # Every assertion here reads the firmware's own log, and UART capture + # block-buffers at 8KB: this journey emits under 4KB in 240 virtual seconds, + # so nothing reaches the file the harness polls until the process exits -- + # expect() cannot see output that is still sitting in the buffer. Drop this + # once the simulator's UART autoflush lands. + pytest.mark.skipif( + os.environ.get("SIMANTIC_UART_AUTOFLUSH") != "1", + reason="needs UART autoflush; set SIMANTIC_UART_AUTOFLUSH=1 to run " + "against a simulator that has it", + ), +] + + +@pytest.fixture(params=ENCODINGS) +def remote(request, sim): + """A running scenario and its peer, once per remote encoding.""" + scenario = sim.start_scenario(_scenario(request.param), + virtual_timeout=JOURNEY_TIMEOUT) + xteink = scenario["xteink"] + return request.param, xteink, xteink.peripheral("blepeer") + + +def test_remote_is_discovered_and_paired(remote): + """Scan through bonding. + + CrossPoint calls secureConnection() unconditionally, so there is no path where + a HID remote connects without pairing: this covers the SMP exchange, the + session key both ends derive, and the encrypted link GATT then runs over. + """ + _, xteink, _ = remote + xteink.uart.expect(r"\[BLE adv\].*PageTurner", timeout=JOURNEY_TIMEOUT) + xteink.uart.expect(r"connected .*PageTurner|onLinkUp|paired", timeout=JOURNEY_TIMEOUT) + + +def test_report_map_is_read_and_reports_are_subscribed(remote): + """The peer only sends a key after the firmware writes its report CCC, so a + report arriving at all proves the firmware discovered the HID service, read + the Report Map, and subscribed. + + The Report Map matters more than it looks: it is 49 octets for the keyboard + peer and longer with the consumer collection the bitmap peer adds, so it + cannot be read in a single ATT response at the default MTU. This assertion + fails if Read Blob continuation regresses. + """ + encoding, xteink, _ = remote + expected = "bitmap report" if encoding == "bitmap" else "HID report" + xteink.uart.expect(expected, timeout=JOURNEY_TIMEOUT) + + +def test_arrow_key_turns_a_page_without_being_mapped(remote): + """The regression this suite exists for. + + A remote that has never been through Settings -> Bluetooth -> Map Remote + Buttons used to be inert: pollBle() decoded the key correctly, found no entry + in an empty bleKeyMap, and dropped it. Standard navigation keys now have + default bindings, so a right-arrow turns a page out of the box. + + Only the keyboard peer is expected to page without mapping. The bitmap + remote's ``0x20`` is a vendor code with no portable meaning -- deliberately + left to capture-and-assign -- so it must reach the firmware (asserted above) + but must not silently acquire a default binding. + """ + encoding, xteink, _ = remote + if encoding != "keyboard": + pytest.skip("vendor bitmap codes are capture-and-assign by design") + xteink.uart.expect(r"page|next", timeout=JOURNEY_TIMEOUT) diff --git a/test/sim/xteink_pageturner_bitmap.yaml b/test/sim/xteink_pageturner_bitmap.yaml new file mode 100644 index 00000000000..a51739b3a0b --- /dev/null +++ b/test/sim/xteink_pageturner_bitmap.yaml @@ -0,0 +1,21 @@ +# CrossPoint on a simulated Xteink X4 (ESP32-C3) paired with a firmware-less BLE +# HID page-turner using the bitmap report encoding. See ble_hid_peer_bitmap.py for +# why the two encodings take different routes through the host's report ingest. +# +# The firmware is the shipping image, unmodified: the scenario drives it through +# its own UI with button presses exactly as a person would -- reader menu, +# Settings, Controls, Bluetooth, Scan, select -- and everything past that point +# is real BLE between two models. +# +# The payload is a build artifact that does not live in this repo; the tests skip +# when it is absent rather than failing, so a bare checkout still runs green. +# Peer script for this scenario: ble_hid_peer_bitmap.py +machines: + xteink: + repl: board-featbt-hidpeer.replx + elf: ../payloads/crosspoint-featbt-latest-sim.elf + controlMap: "back=saradc@0;confirm=saradc@1;left=saradc@2;right=saradc@3;up=saradc@4;down=saradc@5;power=gpio@3:low" +media: + - type: ble + connect: [xteink.radio, xteink.blepeer] +timeout: 240 diff --git a/test/sim/xteink_pageturner_keyboard.yaml b/test/sim/xteink_pageturner_keyboard.yaml new file mode 100644 index 00000000000..12e9bef0e0d --- /dev/null +++ b/test/sim/xteink_pageturner_keyboard.yaml @@ -0,0 +1,21 @@ +# CrossPoint on a simulated Xteink X4 (ESP32-C3) paired with a firmware-less BLE +# HID page-turner using the keyboard report encoding. See ble_hid_peer_bitmap.py for +# why the two encodings take different routes through the host's report ingest. +# +# The firmware is the shipping image, unmodified: the scenario drives it through +# its own UI with button presses exactly as a person would -- reader menu, +# Settings, Controls, Bluetooth, Scan, select -- and everything past that point +# is real BLE between two models. +# +# The payload is a build artifact that does not live in this repo; the tests skip +# when it is absent rather than failing, so a bare checkout still runs green. +# Peer script for this scenario: ble_hid_peer.py +machines: + xteink: + repl: board-featbt-hidpeer.replx + elf: ../payloads/crosspoint-featbt-latest-sim.elf + controlMap: "back=saradc@0;confirm=saradc@1;left=saradc@2;right=saradc@3;up=saradc@4;down=saradc@5;power=gpio@3:low" +media: + - type: ble + connect: [xteink.radio, xteink.blepeer] +timeout: 240 From 666969372d794266bde78526b69d0338f5d9ea4f Mon Sep 17 00:00:00 2001 From: Shahriar Ahnaf Date: Wed, 12 Aug 2026 16:02:46 -0400 Subject: [PATCH 3/6] test: declare HID devices from descriptors instead of copying one remote's bytes The bitmap peer was fitted. BITMAP_TOP = 0x20 and BITMAP_BOTTOM = 0x04 were constants lifted from one teardown of one remote and held until the bench matched it, which proves the bench can replay a Free 2 and nothing about whether the firmware supports HID page-turners in general. A host does not work that way: it derives a report's layout from the device's report descriptor, where Report Size and Report Count give the bit geometry and the Input item's Variable-vs-Array flag decides whether a control is one bit of a bitfield or a usage code placed in an array slot. hid_descriptor.py builds the Report Map and the conforming report bytes from a single declaration, so the bytes follow from the descriptor. Under that construction the Free 2's 20 00 00 stops being a magic number and becomes bit 5 of a variable input -- button 6 of a 24-bit button bitmap -- and 04 00 00 is button 3. Both reproduce exactly, as does a boot keyboard's arrow-key report, without a copied byte anywhere. Changing which button is pressed makes the peer a different remote. test_hid_descriptor.py needs no simulator, payload or hardware, so unlike the rest of test/sim it runs in a bare checkout. Besides pinning the geometry and the variable-vs-array distinction, it documents two defects in the host's report-map handling, whose own comment concedes it is "a hint, not a full descriptor parse": * the scan walks bytes rather than HID short items, so the data bytes of a legal 2-byte Usage item (0x0A 0x05 0x07, Usage 0x0705) are misread as Usage Page (Keyboard); * it only matches the 1-byte Usage Page item (0x05 nn), so a vendor-defined page declared as 0x06 nn nn is invisible to it. Both are covered against a correct short-item walk, with a positive control showing the shortcut agrees on an ordinary keyboard descriptor -- which is why the defect survived. The downstream heuristics (extractPrimaryCode's "first non-zero byte" fallback, the gamepad axis quantisation) are patches for the absent parser, and each new remote shape needs another one. --- test/sim/README.md | 31 +++-- test/sim/ble_hid_peer_bitmap.py | 72 +++++------- test/sim/hid_descriptor.py | 202 ++++++++++++++++++++++++++++++++ test/sim/test_hid_descriptor.py | 143 ++++++++++++++++++++++ 4 files changed, 399 insertions(+), 49 deletions(-) create mode 100644 test/sim/hid_descriptor.py create mode 100644 test/sim/test_hid_descriptor.py diff --git a/test/sim/README.md b/test/sim/README.md index 33dc4d3ac39..654d789bda0 100644 --- a/test/sim/README.md +++ b/test/sim/README.md @@ -27,13 +27,30 @@ through `BleKeyboardHost::onReportIngest` and only the first was ever exercised: - **keyboard** — an 8-byte boot-protocol report carrying a HID usage. Takes the standard keyboard slot path and surfaces as a `SpecialKey`, so the default bindings apply and the key works with no mapping. -- **bitmap** — the encoding a Hanlinyue **Free 2** actually emits, reverse - engineered by a tester on #2418: 3-byte reports, top button `20 00 00`, bottom - `04 00 00`, explicit release `00 00 00`. These are *not* keyboard usages and - *not* consumer usage IDs. They fall through to the generic fallback, where the - identity is the first non-zero byte. Because such vendor codes carry no - portable meaning, they stay capture-and-assign — the bitmap test pins the - decode, not a default binding. +- **bitmap** — a button bitmap, the shape a Hanlinyue **Free 2** presents. These + are *not* keyboard usages and *not* consumer usage IDs; they fall through to the + generic fallback, where the identity is the first non-zero byte. Because such + vendor codes carry no portable meaning, they stay capture-and-assign — the + bitmap test pins the decode, not a default binding. + +### Devices are declared, not copied + +`hid_descriptor.py` builds a Report Map and the reports that conform to it from one +declaration, because that is how a host reads a device: Report Size and Report Count +give the bit geometry, and the Input item's Variable-vs-Array flag decides whether a +control is one bit of a bitfield or a usage code in an array slot +([kernel.org](https://docs.kernel.org/hid/hidintro.html)). + +Writing it the other way round — copying the bytes one remote happens to emit — +fits the bench to that remote and says nothing about the next one. The Free 2's +`20 00 00` is not a magic number under this construction: it is bit 5 of a variable +input, i.e. **button 6** of a 24-bit button bitmap, and `04 00 00` is button 3. +Changing `PRESS_BUTTON` in the peer makes it a different remote without editing a +single report byte, and `button_bitfield`, `boot_keyboard` and `consumer_array` +cover the three encodings a page-turner can plausibly use. + +`test_hid_descriptor.py` pins all of this and needs **no simulator, payload or +hardware** — it runs in a bare checkout. The same remote emits different encodings in its other power-button-cycled modes, which is exactly why the capture-then-assign design is right and why hardcoded diff --git a/test/sim/ble_hid_peer_bitmap.py b/test/sim/ble_hid_peer_bitmap.py index 87ef265c153..61b50db6841 100644 --- a/test/sim/ble_hid_peer_bitmap.py +++ b/test/sim/ble_hid_peer_bitmap.py @@ -72,51 +72,38 @@ H_CTRLPOINT_VALUE = 0x001C H_HID_LAST = H_CTRLPOINT_VALUE -# Minimal keyboard report map: 8-byte boot-style input report (modifiers, -# reserved, 6 keycodes), report ID 1. -REPORT_MAP = bytes([ - 0x05, 0x01, # Usage Page (Generic Desktop) - 0x09, 0x06, # Usage (Keyboard) - 0xA1, 0x01, # Collection (Application) - 0x85, 0x01, # Report ID (1) - 0x05, 0x07, # Usage Page (Key Codes) - 0x19, 0xE0, 0x29, 0xE7, # Usage Min/Max (modifiers) - 0x15, 0x00, 0x25, 0x01, # Logical 0..1 - 0x75, 0x01, 0x95, 0x08, # 1 bit x 8 - 0x81, 0x02, # Input (Data, Var, Abs) - modifier byte - 0x75, 0x08, 0x95, 0x01, # 8 bits x 1 - 0x81, 0x01, # Input (Const) - reserved - 0x75, 0x08, 0x95, 0x06, # 8 bits x 6 - 0x15, 0x00, 0x25, 0x65, # Logical 0..0x65 - 0x19, 0x00, 0x29, 0x65, # Usage 0..0x65 - 0x81, 0x00, # Input (Data, Array) - keys - 0xC0, # End Collection - # Second collection: Consumer Control. Its presence is what makes the host's - # report-map hint parser report consumer=1 alongside kbd=1, which is the - # combination a real Free 2 presents (kbd=1 consumer=1 preferredByte=2). - 0x05, 0x0C, # Usage Page (Consumer) - 0x09, 0x01, # Usage (Consumer Control) - 0xA1, 0x01, # Collection (Application) - 0x85, 0x02, # Report ID (2) - 0x15, 0x00, 0x26, 0xFF, 0x00, # Logical 0..255 - 0x19, 0x00, 0x2A, 0xFF, 0x00, # Usage 0..255 - 0x75, 0x08, 0x95, 0x03, # 8 bits x 3 -- the 3-byte button bitmap - 0x81, 0x00, # Input (Data, Array) - 0xC0, # End Collection -]) +# The device this peer presents is DECLARED, not copied. hid_descriptor builds both +# the Report Map and the conforming report bytes from the same declaration, so the +# bytes on the wire follow from Report Size / Report Count / the Input item's +# Variable flag rather than from a teardown of one remote. +# +# A Hanlinyue "Free 2" page-turner emits 20 00 00 for its top button and 04 00 00 for +# its bottom (teardown on crosspoint-reader#2418). Those are not magic numbers: they +# are buttons 6 and 3 of a 24-bit button bitmap. Change PRESS_BUTTON below and the +# peer becomes a different remote, without editing a single report byte. +# The simulator's embedded Python host does not define __file__ for a peripheral +# script, so the module directory is located via the working directory instead. +import os +import sys + +try: + from hid_descriptor import PAGE_BUTTON, button_bitfield +except ImportError: + sys.path.insert(0, os.getcwd()) + from hid_descriptor import PAGE_BUTTON, button_bitfield + +DEVICE = button_bitfield(buttons=24) +REPORT_MAP = DEVICE.report_map() + +PRESS_BUTTON = 6 # Free 2 top button +RELEASE = () # HID Information: bcdHID 1.11, country 0, flags RemoteWake|NormallyConnectable HID_INFO = bytes([0x11, 0x01, 0x00, 0x03]) KEY_RIGHT_ARROW = 0x4F -# Button bitmap codes, taken from a teardown of a Hanlinyue "Free 2" page-turner -# posted on crosspoint-reader#2418: the remote emits 3-byte bitmap reports rather -# than keyboard usages or consumer usage IDs, one clean press/release pair per -# physical press. Each button raises its own bit in byte 0. -BITMAP_TOP = bytes([0x20, 0x00, 0x00]) -BITMAP_BOTTOM = bytes([0x04, 0x00, 0x00]) -BITMAP_RELEASE = bytes([0x00, 0x00, 0x00]) + SERVICES = [ # (service handle, last handle, 16-bit uuid) @@ -146,7 +133,7 @@ H_HIDINFO_VALUE: HID_INFO, H_REPORTMAP_VALUE: REPORT_MAP, H_PROTOMODE_VALUE: bytes([0x01]), # report protocol - H_REPORT_VALUE: bytes(3), + H_REPORT_VALUE: bytes(DEVICE.report_len), H_REPORT_REF: bytes([0x01, 0x01]), # report ID 1, input report } @@ -157,8 +144,9 @@ KEYPRESS_DELAY_US = 100_000 REPEAT_PERIOD_US = 15_000_000 -CLICK_TOP = (BITMAP_TOP, BITMAP_RELEASE) -CLICK_BOTTOM = (BITMAP_BOTTOM, BITMAP_RELEASE) +CLICK_TOP = (DEVICE.report([(PAGE_BUTTON, PRESS_BUTTON)]), + DEVICE.report(RELEASE)) + def u16(value): diff --git a/test/sim/hid_descriptor.py b/test/sim/hid_descriptor.py new file mode 100644 index 00000000000..31b6632d650 --- /dev/null +++ b/test/sim/hid_descriptor.py @@ -0,0 +1,202 @@ +"""Build HID report descriptors, and the reports that conform to them. + +A HID host derives a report's layout from the device's report descriptor: Report +Size and Report Count give the bit geometry, and the Input item's Variable-vs-Array +flag decides whether a control is one bit of a bitfield or a usage code placed in an +array slot. See the Linux kernel's introduction to report descriptors: +https://docs.kernel.org/hid/hidintro.html + +So a bench for HID remotes should be written the same way round: declare the +descriptor, and let the report bytes fall out of it. The alternative -- copying the +bytes one remote happens to emit and hardcoding them -- fits the bench to that one +device and tells you nothing about the next one. + +That distinction is not academic here. A Hanlinyue "Free 2" page-turner emits +``20 00 00`` for its top button. Written as a constant, 0x20 is a magic number +lifted from a teardown. Declared as a descriptor, it is simply bit 5 of a variable +input -- button 6 of a button bitfield -- and the same declaration expresses any +other bitfield remote by changing which button is pressed, not which byte is sent. + +This module has no simulator or hardware dependency, so its self-tests run anywhere. +""" + +# --- HID short items (USB HID 1.11 sec 6.2.2.2: bTag << 4 | bType << 2 | bSize) --- +USAGE_PAGE = 0x05 +USAGE = 0x09 +USAGE_MIN = 0x19 +USAGE_MAX = 0x29 +LOGICAL_MIN = 0x15 +LOGICAL_MAX = 0x25 +REPORT_SIZE = 0x75 +REPORT_COUNT = 0x95 +REPORT_ID = 0x85 +INPUT = 0x81 +COLLECTION = 0xA1 +END_COLLECTION = 0xC0 + +# Input item flags (sec 6.2.2.5). Bit 1 selects Variable (a bitfield of independent +# controls) over Array (slots holding the usage code of whatever is active). +INPUT_ARRAY = 0x00 # Data, Array, Absolute +INPUT_VARIABLE = 0x02 # Data, Variable, Absolute +INPUT_CONSTANT = 0x01 # Constant -- padding, carries no control + +PAGE_GENERIC_DESKTOP = 0x01 +PAGE_KEYBOARD = 0x07 +PAGE_BUTTON = 0x09 +PAGE_CONSUMER = 0x0C + +USAGE_KEYBOARD = 0x06 +USAGE_CONSUMER_CONTROL = 0x01 + + +def _item(tag, value=None, size=1): + """One short item. size 0 emits the tag alone (e.g. End Collection).""" + if value is None: + return bytes([tag]) + if size == 1: + return bytes([tag | 0x01, value & 0xFF]) + return bytes([tag | 0x02, value & 0xFF, (value >> 8) & 0xFF]) + + +class Field: + """One Input item: a run of `count` controls, each `size` bits wide. + + variable=True -> each usage owns one bit position (a button bitmap) + variable=False -> the run is an array of slots holding active usage codes + (how boot-protocol keyboards report keys) + """ + + def __init__(self, page, usage_min, usage_max, count, size=1, variable=True, + logical_max=None): + self.page = page + self.usage_min = usage_min + self.usage_max = usage_max + self.count = count + self.size = size + self.variable = variable + self.logical_max = logical_max if logical_max is not None else ( + 1 if variable else usage_max) + + @property + def bits(self): + return self.count * self.size + + def descriptor(self): + return (_item(USAGE_PAGE, self.page) + + _item(USAGE_MIN, self.usage_min) + + _item(USAGE_MAX, self.usage_max) + + _item(LOGICAL_MIN, 0) + + _item(LOGICAL_MAX, self.logical_max) + + _item(REPORT_SIZE, self.size) + + _item(REPORT_COUNT, self.count) + + _item(INPUT, INPUT_VARIABLE if self.variable else INPUT_ARRAY)) + + +class Padding: + """Constant bits inserted to reach a byte boundary. Carries no control.""" + + def __init__(self, bits): + self.bits = bits + + def descriptor(self): + return (_item(REPORT_SIZE, self.bits) + + _item(REPORT_COUNT, 1) + + _item(INPUT, INPUT_CONSTANT)) + + +class Device: + """A HID device: one application collection over an ordered list of fields.""" + + def __init__(self, name, page, usage, fields, report_id=None): + self.name = name + self.page = page + self.usage = usage + self.fields = fields + self.report_id = report_id + if sum(f.bits for f in fields) % 8: + raise ValueError(f"{name}: reports must be whole bytes; got " + f"{sum(f.bits for f in fields)} bits") + + def report_map(self): + out = _item(USAGE_PAGE, self.page) + _item(USAGE, self.usage) + out += _item(COLLECTION, 0x01) # Application + if self.report_id is not None: + out += _item(REPORT_ID, self.report_id) + for f in self.fields: + out += f.descriptor() + return out + _item(END_COLLECTION, size=0) + + @property + def report_len(self): + """Payload bytes, excluding the Report ID prefix.""" + return sum(f.bits for f in self.fields) // 8 + + def report(self, active=()): + """Bytes for a report in which `active` (page, usage) controls are pressed. + + A Report ID, when declared, is transmitted as the first byte of every + report (hidintro.html), so it is prepended here. + """ + bits = bytearray(self.report_len) + offset = 0 + for f in self.fields: + if isinstance(f, Padding): + offset += f.bits + continue + if f.variable: + # Each usage owns one bit, in usage order from usage_min. + for page, usage in active: + if page != f.page or not (f.usage_min <= usage <= f.usage_max): + continue + bit = offset + (usage - f.usage_min) * f.size + bits[bit // 8] |= 1 << (bit % 8) + else: + # Array: successive slots hold the usage codes that are active. + slot = 0 + for page, usage in active: + if page != f.page or slot >= f.count: + continue + bits[(offset // 8) + slot] = usage + slot += 1 + offset += f.bits + payload = bytes(bits) + return (bytes([self.report_id]) + payload + if self.report_id is not None else payload) + + +# --- Device shapes ---------------------------------------------------------- +# Each is a declaration, not a capture. Nothing below hardcodes a report byte. + +def boot_keyboard(report_id=1): + """Boot-protocol keyboard: modifier bitfield, reserved byte, 6 key slots. + + The key slots are an ARRAY -- they hold usage codes, not bit positions -- which + is why an arrow key arrives as its usage (Right Arrow = 0x4F) rather than a bit. + """ + return Device("boot-keyboard", PAGE_GENERIC_DESKTOP, USAGE_KEYBOARD, [ + Field(PAGE_KEYBOARD, 0xE0, 0xE7, count=8, size=1, variable=True), + Padding(8), + Field(PAGE_KEYBOARD, 0x00, 0x65, count=6, size=8, variable=False), + ], report_id=report_id) + + +def button_bitfield(buttons=8, report_id=None): + """A button bitmap: `buttons` controls, one bit each, padded to a byte. + + This is the shape a Free 2 presents. Button N sets bit N-1, so button 6 gives + 0x20 and button 3 gives 0x04 -- derived, not copied. + """ + pad = (-buttons) % 8 + fields = [Field(PAGE_BUTTON, 1, buttons, count=buttons, size=1, variable=True)] + if pad: + fields.append(Padding(pad)) + return Device("button-bitfield", PAGE_GENERIC_DESKTOP, 0x00, fields, + report_id=report_id) + + +def consumer_array(report_id=2, slots=1): + """Consumer Control reporting usage codes in array slots (media remotes).""" + return Device("consumer-array", PAGE_CONSUMER, USAGE_CONSUMER_CONTROL, [ + Field(PAGE_CONSUMER, 0x00, 0xFF, count=slots, size=8, variable=False, + logical_max=0xFF), + ], report_id=report_id) diff --git a/test/sim/test_hid_descriptor.py b/test/sim/test_hid_descriptor.py new file mode 100644 index 00000000000..b0ce8ca02db --- /dev/null +++ b/test/sim/test_hid_descriptor.py @@ -0,0 +1,143 @@ +"""Descriptor-driven HID report construction, and what the host's hints get wrong. + +These run anywhere: no simulator, no hardware, no payload. They exist to keep the +bench honest about the difference between declaring a device and copying one. +""" +import pytest + +from hid_descriptor import (PAGE_BUTTON, PAGE_CONSUMER, PAGE_GENERIC_DESKTOP, + PAGE_KEYBOARD, Device, Field, boot_keyboard, + button_bitfield, consumer_array) + + +class TestReportGeometry: + def test_reports_are_whole_bytes(self): + """HID reports are byte-aligned; the helpers pad to reach a boundary.""" + assert len(button_bitfield(5).report()) == 1 # 5 buttons + 3 pad bits + assert len(button_bitfield(9).report()) == 2 # 9 buttons + 7 pad bits + + def test_an_unpadded_descriptor_is_rejected(self): + """A field run that does not land on a byte boundary is a malformed + descriptor, and saying so at construction beats emitting a short report.""" + with pytest.raises(ValueError, match="whole bytes"): + Device("bad", PAGE_GENERIC_DESKTOP, 0x00, + [Field(PAGE_BUTTON, 1, 5, count=5, size=1, variable=True)]) + + def test_report_id_is_the_first_byte(self): + """A declared Report ID is transmitted as the first byte of every report.""" + assert button_bitfield(8, report_id=None).report() == b"\x00" + assert button_bitfield(8, report_id=7).report() == b"\x07\x00" + + +class TestVariableVersusArray: + """The Input item's Variable-vs-Array flag is what decides a control's encoding. + + Getting this backwards is the single most common way to mis-model a remote, and + it is invisible if you only ever copy one device's bytes. + """ + + def test_variable_fields_give_each_control_its_own_bit(self): + bf = button_bitfield(8) + assert bf.report([(PAGE_BUTTON, 1)]) == b"\x01" + assert bf.report([(PAGE_BUTTON, 8)]) == b"\x80" + # Independent bits: two controls held at once set two bits. + assert bf.report([(PAGE_BUTTON, 1), (PAGE_BUTTON, 8)]) == b"\x81" + + def test_array_fields_carry_usage_codes_in_slots(self): + kb = boot_keyboard() + # 0x4F is Right Arrow's usage code. In an array it appears verbatim in the + # first free slot -- it is NOT bit 79 of a bitfield. + assert kb.report([(PAGE_KEYBOARD, 0x4F)])[3] == 0x4F + # A second key fills the next slot rather than OR-ing into the first. + two = kb.report([(PAGE_KEYBOARD, 0x4F), (PAGE_KEYBOARD, 0x50)]) + assert (two[3], two[4]) == (0x4F, 0x50) + + def test_the_same_control_number_encodes_differently_per_field_type(self): + """Control 6 is 0x20 in a bitfield and 0x06 in an array. Same "button 6".""" + assert button_bitfield(8).report([(PAGE_BUTTON, 6)]) == b"\x20" + assert consumer_array().report([(PAGE_CONSUMER, 6)])[1] == 0x06 + + +class TestKnownDevicesAreReproducedNotCopied: + """Both shapes below are declared, then checked against what real hardware + emits. No report byte is hardcoded anywhere in hid_descriptor.py.""" + + def test_boot_keyboard_matches_a_real_arrow_key_report(self): + assert boot_keyboard().report([(PAGE_KEYBOARD, 0x4F)]).hex() == ( + "0100004f0000000000") + + def test_free2_page_turner_falls_out_of_a_24_button_bitfield(self): + """A Hanlinyue Free 2 emits 20 00 00 / 04 00 00 / 00 00 00 (teardown on + crosspoint-reader#2418). Those are buttons 6 and 3 of a 3-byte button + bitmap -- nothing about the device is special.""" + bf = button_bitfield(24) + assert bf.report([(PAGE_BUTTON, 6)]).hex() == "200000" + assert bf.report([(PAGE_BUTTON, 3)]).hex() == "040000" + assert bf.report().hex() == "000000" + + +# --- What the host currently does with a report map -------------------------- + +def naive_usage_page_scan(report_map): + """Faithful transcription of BleKeyboardHost::parseReportMapHints(). + + Kept here so its behaviour is executable and testable rather than argued + about. It walks bytes, not HID items: + + for (size_t i = 0; i + 1 < len; ++i) + if (map[i] == 0x05) { if (map[i+1] == 0x07) kbd = true; + else if (map[i+1] == 0x0C) consumer = true; } + """ + kbd = consumer = False + for i in range(len(report_map) - 1): + if report_map[i] == 0x05: + if report_map[i + 1] == 0x07: + kbd = True + elif report_map[i + 1] == 0x0C: + consumer = True + return kbd, consumer + + +def item_walk_usage_pages(report_map): + """A correct short-item walk (USB HID 1.11 sec 6.2.2.2). + + bSize lives in the low two bits, with 3 meaning four data bytes, so data is + skipped rather than re-read as items. + """ + pages = set() + i = 0 + while i < len(report_map): + prefix = report_map[i] + size = prefix & 0x03 + size = 4 if size == 3 else size + tag_type = prefix & 0xFC + if tag_type == 0x04: # Usage Page + pages.add(int.from_bytes(report_map[i + 1:i + 1 + size], "little")) + i += 1 + size + return pages + + +class TestReportMapHintsAreNotAParser: + def test_it_agrees_with_a_real_walk_on_ordinary_descriptors(self): + """A positive control: on a plain keyboard the shortcut is right, which is + why the defect below survived.""" + kbd, _ = naive_usage_page_scan(boot_keyboard().report_map()) + assert kbd is True + assert PAGE_KEYBOARD in item_walk_usage_pages(boot_keyboard().report_map()) + + def test_it_false_positives_on_data_bytes_that_look_like_an_item(self): + """0x0A 0x05 0x07 is a legal 2-byte Usage item (Usage 0x0705). Its DATA + bytes are 05 07, which the byte scan reads as Usage Page (Keyboard).""" + descriptor = bytes([0x0A, 0x05, 0x07]) + kbd, _ = naive_usage_page_scan(descriptor) + assert kbd is True, "scan claims a keyboard page" + assert PAGE_KEYBOARD not in item_walk_usage_pages(descriptor), ( + "but the descriptor declares no keyboard page at all") + + def test_it_cannot_see_a_two_byte_usage_page(self): + """Vendor-defined pages are declared with 0x06 nn nn, which the scan (which + only matches 0x05) never sees -- so vendor remotes get no hint at all.""" + descriptor = bytes([0x06, 0x00, 0xFF]) # Usage Page (Vendor 0xFF00) + kbd, consumer = naive_usage_page_scan(descriptor) + assert (kbd, consumer) == (False, False) + assert item_walk_usage_pages(descriptor) == {0xFF00} From dd97596dec3ae22d9efe5bfea3a92c3a1098c37b Mon Sep 17 00:00:00 2001 From: Shahriar Ahnaf Date: Wed, 12 Aug 2026 16:26:41 -0400 Subject: [PATCH 4/6] test: make the peer a conformant HOGP device, and import siblings properly Two spec gaps in the peer, both invisible while only one permissive host talked to it. HOGP requires the HID Device to expose a single Device Information Service instance, and requires that instance to include the PnP ID characteristic -- it is how a host identifies vendor and product before reading a single report. The peer exposed HID and Battery but no DIS at all, so it was not a conformant HID device even though CrossPoint was happy to pair with it. Added, with the pid.codes open-source VID so it identifies as a test device rather than impersonating a vendor. The sibling import now uses __file__ rather than the working directory, which needed the ScriptedBlePeer fix in simantic-core: peer scripts previously had no __file__ and no directory on sys.path, so they could not be split across files at all. --- test/sim/ble_hid_peer_bitmap.py | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/test/sim/ble_hid_peer_bitmap.py b/test/sim/ble_hid_peer_bitmap.py index 61b50db6841..0b40a2349e1 100644 --- a/test/sim/ble_hid_peer_bitmap.py +++ b/test/sim/ble_hid_peer_bitmap.py @@ -46,6 +46,13 @@ UUID_PROTOCOL_MODE = 0x2A4E UUID_BATTERY_SERVICE = 0x180F UUID_BATTERY_LEVEL = 0x2A19 +# HOGP requires the HID Device to expose a single Device Information Service +# instance, and requires that instance to include the PnP ID characteristic. It is +# how a host identifies the device's vendor/product before it has read a single +# report, so a peer without it is not a conformant HID device even though a +# permissive host will still talk to it. +UUID_DEVICE_INFO_SERVICE = 0x180A +UUID_PNP_ID = 0x2A50 CHAR_PROP_READ = 0x02 CHAR_PROP_WRITE_NR = 0x04 @@ -71,6 +78,9 @@ H_CTRLPOINT_DECL = 0x001B H_CTRLPOINT_VALUE = 0x001C H_HID_LAST = H_CTRLPOINT_VALUE +H_DIS_SERVICE = 0x0020 # group 0x0020..0x0022 +H_PNP_DECL = 0x0021 +H_PNP_VALUE = 0x0022 # The device this peer presents is DECLARED, not copied. hid_descriptor builds both # the Report Map and the conforming report bytes from the same declaration, so the @@ -81,16 +91,11 @@ # its bottom (teardown on crosspoint-reader#2418). Those are not magic numbers: they # are buttons 6 and 3 of a 24-bit button bitmap. Change PRESS_BUTTON below and the # peer becomes a different remote, without editing a single report byte. -# The simulator's embedded Python host does not define __file__ for a peripheral -# script, so the module directory is located via the working directory instead. import os import sys -try: - from hid_descriptor import PAGE_BUTTON, button_bitfield -except ImportError: - sys.path.insert(0, os.getcwd()) - from hid_descriptor import PAGE_BUTTON, button_bitfield +sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) +from hid_descriptor import PAGE_BUTTON, button_bitfield # noqa: E402 DEVICE = button_bitfield(buttons=24) REPORT_MAP = DEVICE.report_map() @@ -109,6 +114,7 @@ # (service handle, last handle, 16-bit uuid) (H_BAT_SERVICE, H_BAT_VALUE, UUID_BATTERY_SERVICE), (H_HID_SERVICE, H_HID_LAST, UUID_HID_SERVICE), + (H_DIS_SERVICE, H_PNP_VALUE, UUID_DEVICE_INFO_SERVICE), ] CHARACTERISTICS = [ @@ -121,6 +127,7 @@ (H_REPORT_DECL, H_REPORT_VALUE, CHAR_PROP_READ | CHAR_PROP_NOTIFY, UUID_REPORT), (H_CTRLPOINT_DECL, H_CTRLPOINT_VALUE, CHAR_PROP_WRITE_NR, UUID_HID_CONTROL_POINT), + (H_PNP_DECL, H_PNP_VALUE, CHAR_PROP_READ, UUID_PNP_ID), ] DESCRIPTORS = { @@ -135,6 +142,11 @@ H_PROTOMODE_VALUE: bytes([0x01]), # report protocol H_REPORT_VALUE: bytes(DEVICE.report_len), H_REPORT_REF: bytes([0x01, 0x01]), # report ID 1, input report + # PnP ID: vendor ID source, vendor ID, product ID, product version (LE). + # Source 0x02 = USB Implementer's Forum; 0x1209 is pid.codes, the VID handed + # out for open-source hardware, so this identifies as a test device rather + # than impersonating a real vendor. + H_PNP_VALUE: bytes([0x02, 0x09, 0x12, 0x01, 0x00, 0x00, 0x01]), } # Once subscribed, click the top button -- press then explicit release, the pair a From 99db170a57174ed684d9eb742ffa20e7991259c7 Mon Sep 17 00:00:00 2001 From: Shahriar Ahnaf Date: Wed, 12 Aug 2026 16:47:13 -0400 Subject: [PATCH 5/6] test: give the HID peer an on_command surface so tests can drive it The peer clicked on a timer and nothing could intervene. With the scripted-peer command channel in simantic-core, a test drives it directly: peer.Command = "click=6" # press-and-release button 6 peer.Command = "press=3" # hold peer.Command = "autoclick=0" # stop the free-running repeat Which button a name maps to, and what a click even is, lives in this script rather than in the simulator, so the same peripheral running a different script is a different BLE slave. --- test/sim/ble_hid_peer_bitmap.py | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/test/sim/ble_hid_peer_bitmap.py b/test/sim/ble_hid_peer_bitmap.py index 0b40a2349e1..6ab24416dce 100644 --- a/test/sim/ble_hid_peer_bitmap.py +++ b/test/sim/ble_hid_peer_bitmap.py @@ -170,6 +170,9 @@ def __init__(self, ctx): self.ctx = ctx self.subscribed = False self.queue = [] + # Auto-repeat the click, as a hand would. Turn off with `Command = autoclick=0` + # to drive every press explicitly from a test instead. + self.repeat = True def on_connect(self): self.subscribed = False @@ -183,6 +186,33 @@ def on_disconnect(self, reason): self.queue = [] self.ctx.info(f"disconnected (reason 0x{reason:02x})") + def on_command(self, name, value): + """Runtime control surface, driven by writes to the peer's Command + property (`peer.Command = "click=6"` from pytest or the monitor). + + What a command means lives here rather than in the simulator, so this + peer is a page-turner because its script says so -- a different script + makes the same peripheral a different BLE slave. + """ + if name in ("click", "press", "release"): + button = int(value) if value else PRESS_BUTTON + pressed = DEVICE.report([(PAGE_BUTTON, button)]) + released = DEVICE.report(RELEASE) + if name == "press": + self.queue = [pressed] + elif name == "release": + self.queue = [released] + else: + self.queue = [pressed, released] + self.ctx.schedule_oneshot(KEYPRESS_DELAY_US) + return + if name == "autoclick": + # "autoclick=0" stops the repeat; anything else re-arms it. + self.repeat = value not in ("0", "off", "false") + return + raise ValueError( + "unknown command %r; known: click, press, release, autoclick" % name) + def on_timer(self): if not self.queue: return @@ -191,7 +221,7 @@ def on_timer(self): self.ctx.notify(H_REPORT_VALUE, report) if self.queue: self.ctx.schedule_oneshot(KEYPRESS_DELAY_US) - elif self.subscribed: + elif self.subscribed and self.repeat: self.queue = list(CLICK_TOP) self.ctx.schedule_oneshot(REPEAT_PERIOD_US) From 69b9a4b7a360a42b2d846078829cc0730b15c71d Mon Sep 17 00:00:00 2001 From: Shahriar Ahnaf Date: Wed, 12 Aug 2026 17:00:37 -0400 Subject: [PATCH 6/6] test: fix two peer-script defects found by driving it at runtime Both surfaced only once commands could actually be sent, which is the point of having a runtime surface at all. Commands replaced the pending queue instead of appending to it, so writes arriving faster than the timer drains them lost all but the last. Driving click=6, press=3 and release=3 in quick succession delivered one report -- 000000 -- because each command overwrote the one before it. A test pressing twice in a row would have silently lost the first press. on_timer logged "sending HID report" whether or not anything was subscribed, so reports that never reached the air appeared in the log as if they had. That is the same failure that made a dead link read as a working one earlier in this work; unsubscribed reports are now logged as dropped. --- test/sim/ble_hid_peer_bitmap.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/test/sim/ble_hid_peer_bitmap.py b/test/sim/ble_hid_peer_bitmap.py index 6ab24416dce..a685a67ff85 100644 --- a/test/sim/ble_hid_peer_bitmap.py +++ b/test/sim/ble_hid_peer_bitmap.py @@ -198,13 +198,17 @@ def on_command(self, name, value): button = int(value) if value else PRESS_BUTTON pressed = DEVICE.report([(PAGE_BUTTON, button)]) released = DEVICE.report(RELEASE) + # Append rather than replace: commands arriving faster than the + # timer drains them must all be delivered, or a test that presses + # twice in quick succession silently loses the first press. if name == "press": - self.queue = [pressed] + self.queue.append(pressed) elif name == "release": - self.queue = [released] + self.queue.append(released) else: - self.queue = [pressed, released] - self.ctx.schedule_oneshot(KEYPRESS_DELAY_US) + self.queue.extend((pressed, released)) + if len(self.queue) == 1: + self.ctx.schedule_oneshot(KEYPRESS_DELAY_US) return if name == "autoclick": # "autoclick=0" stops the repeat; anything else re-arms it. @@ -217,6 +221,12 @@ def on_timer(self): if not self.queue: return report = self.queue.pop(0) + if not self.subscribed: + # No subscriber: there is nothing to notify. Saying "sending" here + # would put reports in the log that never reached the air, which is + # exactly how a dead link reads as a working one. + self.ctx.info("dropping HID report %s (not subscribed)" % report.hex()) + return self.ctx.info("sending HID bitmap report %s" % report.hex()) self.ctx.notify(H_REPORT_VALUE, report) if self.queue: