feat: add generic digitizer relay support#198
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/bluetooth_2_usb/loopback/capture.py (1)
724-747:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDeduplicate multi-collection digitizers before checking candidate count.
The new HID function intentionally combines both touch and tablet top-level collections in a single USB interface. Since hidapi enumerates each collection as a separate entry, both will match as
"digitizer"role and get appended todigitizer_nodes, causing the check at line 765-768 to fail with "Multiple digitizer HID devices matched" even though they're the same physical interface.Deduplicate by
raw_pathornodebefore the length validation to handle this expected multi-collection scenario.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/bluetooth_2_usb/loopback/capture.py` around lines 724 - 747, The digitizer_nodes list currently collects separate hidapi entries for different top-level collections of the same physical interface; before performing the candidate-count checks and before constructing GadgetNodeCandidates, deduplicate digitizer_nodes by a stable identifier (preferably info.raw_path, falling back to info.node) so multiple collections from the same device collapse to one entry; replace the current digitizer_nodes list with the deduped sequence (preserving order or sorting by node as done later) so the later validation that raises "Multiple digitizer HID devices matched" and the GadgetNodeCandidates(...) creation operate on unique devices.
🧹 Nitpick comments (1)
tests/test_gadgets.py (1)
142-153: ⚡ Quick winAdd failed-enable reset assertions for
touchandtablet.
_enable_with_fakes()now initializes digitizer writers, but the rebuild-failure test still only verifies keyboard/mouse/consumer refs are cleared. Addtouch/tabletassertions there to prevent stale digitizer writers after failed enable.Suggested test addition
self.assertIsNone(hid_gadgets.keyboard) self.assertIsNone(hid_gadgets.mouse) self.assertIsNone(hid_gadgets.consumer) + self.assertIsNone(hid_gadgets.touch) + self.assertIsNone(hid_gadgets.tablet)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_gadgets.py` around lines 142 - 153, The rebuild-failure test doesn't assert that digitizer writers are cleared after a failed enable; update the test that calls _enable_with_fakes to also verify that both touch and tablet writer references are reset (cleared) on failure by adding assertions for the touch and tablet objects (the same way keyboard/mouse/consumer refs are checked), ensuring any stale _FakeDigitizer instances are not left behind; locate the verification block in the rebuild-failure test that currently checks keyboard/mouse/consumer and add analogous assertions for touch and tablet.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 160: The sentence "Touchpad behavior is not claimed; it needs required
HID feature-report handling" uses a double modal ("needs required"); update the
README sentence to use a single verb (e.g., "requires HID feature-report
handling" or "needs HID feature-report handling") so the phrasing is standard
and unambiguous—locate the sentence in README.md and replace the phrase "needs
required HID feature-report handling" accordingly.
In `@src/bluetooth_2_usb/gadgets/manager.py`:
- Around line 167-168: Touch and Tablet digitizers each create their own
asyncio.Lock but both write to the same HID digitizer device, causing
interleaved writes; update the gadget creation in Manager so both TouchDigitizer
and TabletDigitizer share a single asyncio.Lock instance (or refactor into a
combined Digitizer wrapper) by creating one lock (e.g., shared_digitizer_lock)
and passing it into the TouchDigitizer and TabletDigitizer constructors (or
using a combined class that serializes writes) so writes to the shared hidg node
are serialized and report ordering is preserved.
In `@src/bluetooth_2_usb/hid/absolute.py`:
- Around line 338-343: PadAccumulator currently leaves the internal _wheel value
latched across flushes and release_all calls, causing stale relative wheel
deltas to be resent; update both the release_all method and the flush logic to
clear self._wheel (set to 0) when emitting/resetting reports, and ensure the
returned PadReport uses wheel=0 so the accumulator no longer preserves stale
wheel state; reference the PadAccumulator class, its _wheel attribute, and the
release_all and flush methods when making the changes.
In `@src/bluetooth_2_usb/hid/dispatch.py`:
- Around line 3-5: The code currently maps BrokenPipeError to
_handle_broken_pipe() but lets OSError(ENODEV) fall through; update the
exception handling in dispatch.py so OSError with errno.ENODEV is treated the
same as BrokenPipeError (call _handle_broken_pipe() / suspend writes). Import
errno at top, and in the try/except blocks that handle BrokenPipeError (the
blocks around _handle_broken_pipe in dispatch.py, including the one covering the
dispatch loop at the ~253-266 region), add an except OSError as e: if e.errno ==
errno.ENODEV: return _handle_broken_pipe() (or otherwise invoke the same
suspension logic) and re-raise or handle other OSErrors unchanged.
In `@src/bluetooth_2_usb/loopback/result.py`:
- Around line 16-24: The to_dict method currently omits the "digitizer_node" key
when self.digitizer_node is None, causing inconsistent node shapes; change to
always include "digitizer_node" in the returned nodes mapping (just assign
nodes["digitizer_node"] = self.digitizer_node alongside keyboard_node,
mouse_node, consumer_node) and remove the conditional so the serialized dict
always contains "digitizer_node": None when absent.
In `@src/bluetooth_2_usb/loopback/scenarios.py`:
- Around line 333-342: SCENARIO_DIGITIZER is being exposed while Windows capture
(run_capture()/Raw Input backend) doesn't support digitizer reports yet; either
prevent registering/publishing SCENARIO_DIGITIZER on Windows or add a fast-fail
prerequisite in run_capture() when the requested scenario == SCENARIO_DIGITIZER
and platform is Windows. Locate SCENARIO_DIGITIZER (ScenarioDefinition) and the
run_capture() code path that dispatches scenarios to the Raw Input backend, and
implement a platform guard (e.g., if sys.platform == "win32" then skip
registration or raise a clear error) so invoking "loopback capture --scenario
digitizer" on Windows returns a descriptive prerequisite error instead of
exposing a broken scenario.
---
Outside diff comments:
In `@src/bluetooth_2_usb/loopback/capture.py`:
- Around line 724-747: The digitizer_nodes list currently collects separate
hidapi entries for different top-level collections of the same physical
interface; before performing the candidate-count checks and before constructing
GadgetNodeCandidates, deduplicate digitizer_nodes by a stable identifier
(preferably info.raw_path, falling back to info.node) so multiple collections
from the same device collapse to one entry; replace the current digitizer_nodes
list with the deduped sequence (preserving order or sorting by node as done
later) so the later validation that raises "Multiple digitizer HID devices
matched" and the GadgetNodeCandidates(...) creation operate on unique devices.
---
Nitpick comments:
In `@tests/test_gadgets.py`:
- Around line 142-153: The rebuild-failure test doesn't assert that digitizer
writers are cleared after a failed enable; update the test that calls
_enable_with_fakes to also verify that both touch and tablet writer references
are reset (cleared) on failure by adding assertions for the touch and tablet
objects (the same way keyboard/mouse/consumer refs are checked), ensuring any
stale _FakeDigitizer instances are not left behind; locate the verification
block in the rebuild-failure test that currently checks keyboard/mouse/consumer
and add analogous assertions for touch and tablet.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 269ecf77-08ed-4bb6-980d-6862ad989e3b
📒 Files selected for processing (31)
README.mddocs/device-capture.mddocs/runtime-architecture.mddocs/testing-strategy.mdsrc/bluetooth_2_usb/evdev/__init__.pysrc/bluetooth_2_usb/evdev/types.pysrc/bluetooth_2_usb/gadgets/layout.pysrc/bluetooth_2_usb/gadgets/manager.pysrc/bluetooth_2_usb/hid/absolute.pysrc/bluetooth_2_usb/hid/constants.pysrc/bluetooth_2_usb/hid/consumer.pysrc/bluetooth_2_usb/hid/descriptors.pysrc/bluetooth_2_usb/hid/dispatch.pysrc/bluetooth_2_usb/hid/keyboard.pysrc/bluetooth_2_usb/hid/mouse.pysrc/bluetooth_2_usb/hid/mouse_delta.pysrc/bluetooth_2_usb/hid/tablet.pysrc/bluetooth_2_usb/hid/touch.pysrc/bluetooth_2_usb/inputs/inventory.pysrc/bluetooth_2_usb/inputs/profile.pysrc/bluetooth_2_usb/loopback/capture.pysrc/bluetooth_2_usb/loopback/inject.pysrc/bluetooth_2_usb/loopback/result.pysrc/bluetooth_2_usb/loopback/scenarios.pysrc/bluetooth_2_usb/relay/input.pytests/test_evdev.pytests/test_gadgets.pytests/test_hid.pytests/test_inputs.pytests/test_loopback.pytests/test_relay.py
💤 Files with no reviewable changes (1)
- src/bluetooth_2_usb/hid/mouse_delta.py
| SCENARIO_DIGITIZER: ScenarioDefinition( | ||
| name=SCENARIO_DIGITIZER, | ||
| keyboard_steps=(), | ||
| mouse_rel_steps=DIGITIZER_MOUSE_REL_STEPS, | ||
| mouse_button_steps=DIGITIZER_MOUSE_BUTTON_STEPS, | ||
| consumer_steps=(), | ||
| digitizer_report_ids=(1, 1, 2, 2, 3, 3), | ||
| default_post_delay_ms=DIGITIZER_POST_DELAY_MS, | ||
| default_capture_timeout_sec=DIGITIZER_CAPTURE_TIMEOUT_SEC, | ||
| ), |
There was a problem hiding this comment.
Avoid publishing the digitizer scenario on Windows before the backend can capture it.
SCENARIO_DIGITIZER is now public, but this slice only adds digitizer matching to the hidapi capture path. run_capture() still sends every win32 scenario through the Raw Input backend, so loopback capture --scenario digitizer is exposed without a working Windows capture implementation. Gate this scenario by platform or fail fast with a prerequisite error until the Windows backend understands digitizer reports.
🧰 Tools
🪛 GitHub Actions: CI / validate
[warning] dwc2 module not present in your kernel. did you insmod it?
[warning] libcomposite module not present in your kernel. did you insmod it?
[warning] Bluetooth rfkill rfkill0 is hard-blocked; leaving it unchanged.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/bluetooth_2_usb/loopback/scenarios.py` around lines 333 - 342,
SCENARIO_DIGITIZER is being exposed while Windows capture (run_capture()/Raw
Input backend) doesn't support digitizer reports yet; either prevent
registering/publishing SCENARIO_DIGITIZER on Windows or add a fast-fail
prerequisite in run_capture() when the requested scenario == SCENARIO_DIGITIZER
and platform is Windows. Locate SCENARIO_DIGITIZER (ScenarioDefinition) and the
run_capture() code path that dispatches scenarios to the Raw Input backend, and
implement a platform guard (e.g., if sys.platform == "win32" then skip
registration or raise a clear error) so invoking "loopback capture --scenario
digitizer" on Windows returns a descriptive prerequisite error instead of
exposing a broken scenario.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/bluetooth_2_usb/hid/tablet.py`:
- Around line 71-88: release_all() currently calls send_pen() and send_pad()
separately but each of those releases _report_lock, allowing interleaving; to
fix, acquire the shared async lock (_report_lock) once inside release_all (e.g.,
async with self._report_lock or explicit acquire()/release()) and call
send_pen(...) and send_pad(...) while holding that lock, then release it only
after both reports are sent so no other digitizer task can interleave between
the two reports.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1186ed94-e326-4c7e-9a3a-f8b44d5fee84
📒 Files selected for processing (13)
README.mdsrc/bluetooth_2_usb/gadgets/manager.pysrc/bluetooth_2_usb/hid/absolute.pysrc/bluetooth_2_usb/hid/dispatch.pysrc/bluetooth_2_usb/hid/tablet.pysrc/bluetooth_2_usb/hid/touch.pysrc/bluetooth_2_usb/loopback/capture.pysrc/bluetooth_2_usb/loopback/capture_windows.pysrc/bluetooth_2_usb/loopback/result.pytests/test_gadgets.pytests/test_hid.pytests/test_loopback.pytests/test_relay.py
🚧 Files skipped from review as they are similar to previous changes (8)
- src/bluetooth_2_usb/loopback/result.py
- src/bluetooth_2_usb/gadgets/manager.py
- src/bluetooth_2_usb/hid/dispatch.py
- src/bluetooth_2_usb/hid/absolute.py
- tests/test_gadgets.py
- src/bluetooth_2_usb/hid/touch.py
- src/bluetooth_2_usb/loopback/capture.py
- tests/test_hid.py
| async def release_all(self) -> None: | ||
| await self.send_pen( | ||
| PenReport( | ||
| in_range=False, | ||
| tip=False, | ||
| eraser=False, | ||
| barrel=False, | ||
| barrel2=False, | ||
| x=0, | ||
| y=0, | ||
| pressure=0, | ||
| distance=0, | ||
| tilt_x=0, | ||
| tilt_y=0, | ||
| serial=0, | ||
| ) | ||
| ) | ||
| await self.send_pad(PadReport(buttons=0, wheel=0)) |
There was a problem hiding this comment.
Keep release_all() atomic on the shared report lock.
send_pen() and send_pad() each release _report_lock before the next call, so another digitizer task can slip a report between the two releases. During disconnect cleanup that can leave stale pad state pressed until the queue drains; hold the shared lock across both release reports.
🔒 Proposed fix
+ async def _send_pen_unlocked(self, pen_report: PenReport) -> None:
+ report = bytearray(TABLET_PEN_IN_REPORT_LENGTH)
+ report[0] = (
+ (0x01 if pen_report.in_range else 0)
+ | (0x02 if pen_report.tip else 0)
+ | (0x04 if pen_report.eraser else 0)
+ | (0x08 if pen_report.barrel else 0)
+ | (0x10 if pen_report.barrel2 else 0)
+ )
+ report[1:3] = pen_report.x.to_bytes(2, "little")
+ report[3:5] = pen_report.y.to_bytes(2, "little")
+ report[5:7] = pen_report.pressure.to_bytes(2, "little")
+ report[7:9] = pen_report.distance.to_bytes(2, "little")
+ report[9] = pen_report.tilt_x.to_bytes(1, "little", signed=True)[0]
+ report[10] = pen_report.tilt_y.to_bytes(1, "little", signed=True)[0]
+ report[11:15] = pen_report.serial.to_bytes(4, "little")
+ logger.debug("Sending tablet pen report: %s", report.hex(" "))
+ await self._send_report(report, TABLET_PEN_REPORT_ID)
+
+ async def _send_pad_unlocked(self, pad_report: PadReport) -> None:
+ report = bytearray(TABLET_PAD_IN_REPORT_LENGTH)
+ report[0:2] = pad_report.buttons.to_bytes(2, "little")
+ report[2] = pad_report.wheel.to_bytes(1, "little", signed=True)[0]
+ logger.debug("Sending tablet pad report: %s", report.hex(" "))
+ await self._send_report(report, TABLET_PAD_REPORT_ID)
+
async def send_pen(self, pen_report: PenReport) -> None:
async with self._report_lock:
- report = bytearray(TABLET_PEN_IN_REPORT_LENGTH)
- ...
- await self._send_report(report, TABLET_PEN_REPORT_ID)
+ await self._send_pen_unlocked(pen_report)
async def send_pad(self, pad_report: PadReport) -> None:
async with self._report_lock:
- report = bytearray(TABLET_PAD_IN_REPORT_LENGTH)
- ...
- await self._send_report(report, TABLET_PAD_REPORT_ID)
+ await self._send_pad_unlocked(pad_report)
async def release_all(self) -> None:
- await self.send_pen(...)
- await self.send_pad(...)
+ async with self._report_lock:
+ await self._send_pen_unlocked(...)
+ await self._send_pad_unlocked(...)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/bluetooth_2_usb/hid/tablet.py` around lines 71 - 88, release_all()
currently calls send_pen() and send_pad() separately but each of those releases
_report_lock, allowing interleaving; to fix, acquire the shared async lock
(_report_lock) once inside release_all (e.g., async with self._report_lock or
explicit acquire()/release()) and call send_pen(...) and send_pad(...) while
holding that lock, then release it only after both reports are sent so no other
digitizer task can interleave between the two reports.
Summary
Adds generic HID digitizer relay support for touchpads/tablet-touch, tablet pens, and tablet pads while preserving the existing keyboard, mouse, and consumer-control relay behavior.
Key points:
hid.usb0throughhid.usb3gadget limit.EV_ABS/EV_MSC/EV_SYNevents andBTN_TOUCHcontact lifetime fallback.digitizerloopback scenario that validates touch, pen, pad, and mouse reports together, including overlapping mouse/digitizer button codes.Validation
Local:
venv/bin/black --check src testsvenv/bin/ruff check src testsvenv/bin/python -m compileall src testsvenv/bin/python -m unittest discover -s tests -v(467 tests)venv/bin/python -m build --wheelvenv/bin/bluetooth_2_usb --helpvenv/bin/bluetooth_2_usb --versionvenv/bin/bluetooth_2_usb --validate-env || test $? -eq 3venv/bin/yamllint .github/workflows/ci.ymlPi/live:
pi4b.sudo bluetooth_2_usb smoketest --verbosepassed with the expected warning that no relayable physical input devices were attached.pi4b, including digitizer interface1-2.1.2:1.3.loopback node-discoverypassed.loopback combopassed: 250 keyboard, 8 mouse rel, 16 mouse buttons, 26 consumer.loopback digitizerpassed: 2 mouse rel, 4 mouse buttons, 6 digitizer reports.sudo bluetooth_2_usb debug --duration 10completed and wrote/var/log/bluetooth_2_usb/debug_20260516_214019.md.bluetoothctl showandbtmgmt infosucceeded.active, UDC remainedconfigured, and host still saw 4pi4bHID interfaces.Summary by CodeRabbit
New Features
Documentation