feat: add async USB HID transport#5
Open
okhsunrog wants to merge 5 commits into
Open
Conversation
📝 WalkthroughWalkthroughThe 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. ChangesUSB transport and protocol integration
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
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
4 tasks
There was a problem hiding this comment.
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
📒 Files selected for processing (13)
Cargo.tomlREADME.mddebug/hardware_ble/suites.rsdebug/hardware_usb.rssrc/ble.rssrc/connection.rssrc/lib.rssrc/protocol/diagnosis.rssrc/protocol/mod.rssrc/protocol/product.rssrc/protocol/scp.rssrc/transports/mod.rssrc/transports/usb.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
usb-supportbackend with asynchronousnusbinterrupt I/O.IqosTransportfor applications that support both BLE and USB.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:
0x10/0x90frames 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):Both transports reported the same product and firmware values. Diagnosis returned identical raw counters (
1296usage counter and1074currently unidentified counter) over both transports; their exact semantics remain unverified. Battery voltage matched at4.614 V. The kernelusbhiddriver reattached after the USB handle was dropped.Stateful USB writes were intentionally not exercised and remain documented as experimental.
Verification
cargo fmt --all --checkcargo test --all-targets --all-features(162 tests)cargo clippy --all-targets --all-features -- -D warningscargo clippy --all-targets --no-default-features --features usb-support -- -D warningscargo clippy --all-targets --no-default-features --features btleplug-support -- -D warningscargo clippy --all-targets --no-default-features -- -D warningscargo package --allow-dirtySummary by CodeRabbit
New Features
hardware_usbtool andiqos --usb diagnosis.IQOS_TEST_TRACE_DIAGNOSIS).Bug Fixes
Documentation