Skip to content

feat: add async USB HID transport#5

Open
okhsunrog wants to merge 5 commits into
hauntedfail:mainfrom
okhsunrog:agent/usb-transport
Open

feat: add async USB HID transport#5
okhsunrog wants to merge 5 commits into
hauntedfail:mainfrom
okhsunrog:agent/usb-transport

Conversation

@okhsunrog

@okhsunrog okhsunrog commented Jul 14, 2026

Copy link
Copy Markdown

Summary

  • Implement the reserved usb-support backend with asynchronous nusb interrupt I/O.
  • Carry the existing SCP application frames over 64-byte IQOS HID reports, including atomic and segmented framing.
  • Add a runtime-selectable IqosTransport for applications that support both BLE and USB.
  • Validate ordinary CRC-8 frames and the length-delimited CRC-16/OPENSAFETY-B frames used by diagnosis telemetry.
  • Harden BLE request handling by arming notifications before writes, matching the response command, validating checksums, and applying a timeout.
  • Strip the trailing SCP CRC from holder product numbers and extend the hardware probes and documentation.

Why

The crate exposed a transport-agnostic API and a reserved USB feature, but the USB backend was only a stub. IQOS uses the same SCP messages over BLE and USB with an additional HID envelope.

Two protocol details were necessary for reliable operation:

  • The device can NAK an OUT transfer unless an IN transfer is already armed, so USB reads and writes must progress concurrently.
  • SCP opcode 0x10/0x90 frames carry a declared payload length and a little-endian CRC-16/OPENSAFETY-B checksum (poly=0x755b), while ordinary frames use CRC-8 (poly=0x07). Treating diagnosis frames as CRC-8 rejected otherwise valid commands.

The existing diagnosis command bytes are unchanged.

Hardware validation

Validated read-only over both BLE and USB on an IQOS ILUMA i (2759:0003):

  • device metadata and serial number
  • stick and holder product numbers
  • stick and holder firmware
  • battery voltage
  • brightness and vibration settings
  • FlexPuff, FlexBattery, and AutoStart
  • diagnosis telemetry

Both transports reported the same product and firmware values. Diagnosis returned identical raw counters (1296 usage counter and 1074 currently unidentified counter) over both transports; their exact semantics remain unverified. Battery voltage matched at 4.614 V. The kernel usbhid driver reattached after the USB handle was dropped.

Stateful USB writes were intentionally not exercised and remain documented as experimental.

Verification

  • cargo fmt --all --check
  • cargo test --all-targets --all-features (162 tests)
  • cargo clippy --all-targets --all-features -- -D warnings
  • cargo clippy --all-targets --no-default-features --features usb-support -- -D warnings
  • cargo clippy --all-targets --no-default-features --features btleplug-support -- -D warnings
  • cargo clippy --all-targets --no-default-features -- -D warnings
  • cargo package --allow-dirty
  • real-device BLE and USB read-only hardware harnesses

Summary by CodeRabbit

  • New Features

    • Added experimental USB HID connectivity for supported IQOS devices.
    • Introduced unified transport support to switch between BLE and USB.
    • Added a local USB diagnosis probe via the hardware_usb tool and iqos --usb diagnosis.
    • Added optional raw diagnosis-frame tracing for local debugging (IQOS_TEST_TRACE_DIAGNOSIS).
  • Bug Fixes

    • Improved BLE SCP response matching with stricter validation and timeouts.
    • Fixed product-number parsing and CRC handling for stick/holder responses.
  • Documentation

    • Updated transport and USB setup/troubleshooting guidance, including diagnosis details.

@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR adds experimental USB HID transport support, shared SCP CRC and frame validation, runtime BLE/USB transport selection, USB hardware probing, enhanced BLE diagnostics, updated product-number parsing, and related documentation.

Changes

USB transport and protocol integration

Layer / File(s) Summary
Shared SCP validation and BLE request handling
src/protocol/*, src/protocol/product.rs, src/ble.rs
Adds CRC-8/CRC-16 validation and response matching, applies trailing CRC handling to product numbers, adds telemetry coverage, and makes BLE requests timeout-aware and checksum-validated.
USB HID connection and report exchange
src/transports/usb.rs
Implements USB discovery, HID claiming, serial parsing, atomic and segmented report encoding/decoding, response validation, transport operations, and comprehensive tests.
Runtime transport API and feature wiring
Cargo.toml, src/connection.rs, src/lib.rs, src/transports/mod.rs, src/transport.rs
Adds the USB feature and binary target, exposes IqosTransport and IqosUsb, and delegates shared transport operations across BLE and USB.
Hardware probes and usage documentation
debug/*, README.md
Adds a USB diagnostic probe, BLE AutoStart and diagnosis tracing, and documentation for USB support, commands, and Linux device access.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant IqosTransport
  participant IqosUsb
  participant USBDevice
  Caller->>IqosTransport: request(command)
  IqosTransport->>IqosUsb: delegate request
  IqosUsb->>USBDevice: write HID reports
  USBDevice-->>IqosUsb: return HID response reports
  IqosUsb-->>IqosTransport: return validated SCP response
  IqosTransport-->>Caller: return response payload
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding an async USB HID transport.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 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 `@debug/hardware_ble/suites.rs`:
- Around line 86-92: Update the IQOS_TEST_TRACE_DIAGNOSIS loop in snapshot so
failures from session.request do not propagate through the ? operator or abort
the function. Handle each diagnosis command error locally, report or skip the
failed raw trace consistently with the other diagnostic reads, and continue
processing subsequent commands and the main diagnosis totals.

In `@src/ble.rs`:
- Around line 138-169: Move the self.send(command).await operation into the
existing timeout(SCP_RESPONSE_TIMEOUT, ...) future so the same deadline bounds
both the BLE write and response-wait loop. Preserve the current notification
subscription order and error propagation, ensuring write stalls return the
existing timeout transport error instead of blocking indefinitely.

In `@src/transports/usb.rs`:
- Around line 162-179: Update transact to use Tokio’s fail-fast try_join! for
read_matching_response and write_reports instead of join!, and propagate the
combined result directly so an immediate write failure is returned without
waiting for the response timeout. Preserve the existing biased polling order and
report encoding behavior.
- Around line 318-363: Update SegmentCollector::push to ignore non-initial
continuation reports when no segment sequence is active, returning Ok(None)
without modifying collector state. Continue rejecting continuation reports with
an active but mismatched next_id, and preserve normal initial-segment assembly
and completion behavior.
- Around line 234-249: Normalize the `Transport::send` contract across backends:
document that it may consume and await a matching response, or update the BLE
implementation to perform the same response-draining behavior as
`IqosUsb::send`. Align the `Transport` trait and backend implementations so
callers through `IqosTransport` receive consistent latency and timeout
semantics.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9d5d2f40-9465-4d21-b6af-8bee3e38181d

📥 Commits

Reviewing files that changed from the base of the PR and between 0651c53 and d846471.

📒 Files selected for processing (13)
  • Cargo.toml
  • README.md
  • debug/hardware_ble/suites.rs
  • debug/hardware_usb.rs
  • src/ble.rs
  • src/connection.rs
  • src/lib.rs
  • src/protocol/diagnosis.rs
  • src/protocol/mod.rs
  • src/protocol/product.rs
  • src/protocol/scp.rs
  • src/transports/mod.rs
  • src/transports/usb.rs

Comment thread debug/hardware_ble/suites.rs
Comment thread src/ble.rs
Comment thread src/transports/usb.rs
Comment thread src/transports/usb.rs
Comment thread src/transports/usb.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant