feat: M1 software ACs — descriptor, codec, and mockable bosunctl - #4
Conversation
Load the committed TOML descriptor, decode kept key bits into edge events, and exercise info|watch|record|rgb|leds|lcd test over MockTransport with golden fixtures. Hardware tests stay ignored; AC-R1 and physical LEFT/DOWN/TOP mapping remain parked. Co-authored-by: RaapTechllc <RaapTechllc@users.noreply.github.com>
Assert ignored-bit flips produce an empty event stream, restore device-list parse helpers, cover new clap commands, and keep watch/record off the feature-report-6 path. Drop process-only docs/agents files that pointed at untracked scratch tickets. Co-authored-by: RaapTechllc <RaapTechllc@users.noreply.github.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 19121da376
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| } | ||
| } | ||
| Err(HidError::Disconnected) => match opener() { |
There was a problem hiding this comment.
When a live device is unplugged, HidTransport::read maps every hidapi failure to HidError::Backend in crates/bosun-hid/src/hid.rs, so the Disconnected arm is only reachable with MockTransport; the following catch-all returns an error and terminates device watch instead of reopening after a real hot-plug. Treat live read failures as reconnect signals, as the transport's own documentation requires.
Useful? React with 👍 / 👎.
| fn open_live(descriptor: &DeviceDescriptor, path: Option<&str>) -> bosun_hid::Result<HidTransport> { | ||
| let api = HidTransport::api()?; | ||
| match path { | ||
| Some(path) => HidTransport::open_path(&api, path), |
There was a problem hiding this comment.
When --path names another collection of the same composite device, such as its keyboard-page interface, this branch opens it without checking the descriptor's VID, PID, or usage page. Consequently watch may receive no vendor reports and output commands may target the wrong HID collection; resolve the enumerated path and require it to satisfy match_criteria before opening it.
AGENTS.md reference: AGENTS.md:L26-L26
Useful? React with 👍 / 👎.
| let mut file = std::fs::File::create(&args.output) | ||
| .with_context(|| format!("could not create {}", args.output.display()))?; |
There was a problem hiding this comment.
When --output already exists and the device is absent or cannot be opened, File::create truncates the recording before record_reports invokes the opener and returns its error. A failed capture can therefore destroy the previous hardware fixture; open the transport first or write to a temporary file and replace the destination only after capture begins or succeeds.
Useful? React with 👍 / 👎.
| if keys.len() + raw.caps.ignored_key_bits.len() != KEY_BIT_COUNT { | ||
| return Err(DescriptorError::Malformed(format!( | ||
| "caps.keys ({}) plus ignored_key_bits ({}) must cover {KEY_BIT_COUNT} bits", | ||
| keys.len(), | ||
| raw.caps.ignored_key_bits.len() | ||
| ))); |
There was a problem hiding this comment.
Any descriptor whose key report is not exactly 40 bits is rejected here before its device-specific codec can inspect it, so the generic TOML layer cannot represent the planned second device or another HID layout without inventing fake ignored bits. Keep the 40-bit coverage invariant in the G13 codec and allow the descriptor loader to accept other key counts.
AGENTS.md reference: AGENTS.md:L12-L12
Useful? React with 👍 / 👎.
2 participants
M1 software acceptance criteria that can run without a finger on the G13. Live AC-R1 and physical LEFT/DOWN/TOP mapping stay parked. Engine, adapters, and Tauri are untouched.
What landed
devices/logitech-g13.tomlnow includesignored_key_bits)0..=21and24..=35→ named edge events; ignored bits emit no key events; stick bytes are 0–255 axesbosunctl device info|watch|record|rgb|leds|lcd testoverMockTransportreconnectline, resume ≤ 2 scrates/bosun-hid/tests/fixtures/g13/(synthetic protocol goldens, not a measured stick envelope)BOSUN_HW=1Asserted, not rewritten
Transport,HidTransport,MockTransport, device match/select,g13_probe, andbosunctl device listfilters.Review follow-up
Two-axis review (standards + spec) after the first commit. Follow-up commit asserts an empty event stream for ignored bits, restores
device listparse helpers, covers new clap commands, and keeps watch/record off feature report 6. Output builders stay a small measured codec (D11), not a second device layer.Gates (local)
cargo fmt --all -- --checkcargo clippy --workspace --all-targets --all-features -- -D warningscargo test --workspace --all-featurescargo deny check#[ignore]and printskipped: set BOSUN_HW=1...unless that gate is setOut of scope
docs/hardware-notes.mdEVIOCGRABDo not merge until review.