feat(hotas): Honeycomb Alpha yoke and Bravo throttle protocols#69
Open
EffortlessSteven wants to merge 3 commits into
Open
feat(hotas): Honeycomb Alpha yoke and Bravo throttle protocols#69EffortlessSteven wants to merge 3 commits into
EffortlessSteven wants to merge 3 commits into
Conversation
Implement USB HID protocol parsing for Honeycomb Aeronautics Alpha Flight Controls yoke and Bravo Throttle Quadrant. Includes axis parsing, button matrix decoding, LED control for AP panel and annunciators, gear lever and flap switch detection. New features: - Bravo flap switch position tracker (4 detents: UP/1/2/FULL) - Bravo trim wheel delta tracker (rising-edge detection) - Alpha rocker switch decoding (2 rockers, ±1 directional) - LED report deserialization (inverse of serialize_led_report) - Alpha profile updated with rocker switch button mappings - README updated with Charlie pedals and all protocol features - Compat manifest updated to reflect dedicated crate support - Snapshot fix for Alpha yoke normalisation precision Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
✨ 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 |
Review Summary by QodoAdd Honeycomb protocol support for flaps, trim, rockers, and LED deserialization
WalkthroughsDescription• Add LED report deserialization for Bravo diagnostics and testing • Implement Bravo flap switch position tracker with 4-detent support • Implement Bravo trim wheel delta tracker with edge detection • Add Alpha rocker switch decoding with directional values • Enhance Alpha profile with rocker switch button mappings • Update documentation for all three Honeycomb device protocols Diagramflowchart LR
BravoLED["Bravo LED Report"]
FlapTracker["Flap Switch Tracker"]
TrimTracker["Trim Wheel Tracker"]
AlphaRocker["Alpha Rocker Switches"]
BravoLED -- "serialize/deserialize" --> LEDState["LED State"]
FlapTracker -- "4 detents UP/1/2/FULL" --> FlapPos["Flap Position"]
TrimTracker -- "±1 delta per click" --> TrimDelta["Trim Delta"]
AlphaRocker -- "±1 directional" --> RockerState["Rocker State"]
LEDState --> Profiles["Device Profiles"]
FlapPos --> Profiles
TrimDelta --> Profiles
RockerState --> Profiles
File Changes1. crates/flight-hotas-honeycomb/src/bravo_leds.rs
|
Code Review by Qodo
1.
|
There was a problem hiding this comment.
Pull request overview
Adds deeper protocol-level support to flight-hotas-honeycomb for Honeycomb Alpha/Bravo devices (and documents Charlie), including stateful decoding helpers and LED report round-tripping to improve diagnostics and profile richness.
Changes:
- Added Bravo flap detent tracking, Bravo trim wheel delta tracking, and Alpha rocker switch decoding helpers (with unit tests).
- Added
deserialize_led_report()as the inverse ofserialize_led_report()(with round-trip tests) and re-exported new APIs from the crate root. - Updated Alpha snapshot precision and enriched Alpha default button mappings + docs/compat notes.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/flight-hotas-honeycomb/src/protocol.rs | New flap/trim trackers + Alpha rocker decoding and accompanying tests. |
| crates/flight-hotas-honeycomb/src/bravo_leds.rs | Added LED report deserialization and round-trip tests. |
| crates/flight-hotas-honeycomb/src/lib.rs | Re-exported newly added protocol and LED APIs. |
| crates/flight-hotas-honeycomb/src/profiles.rs | Added Alpha rocker button mappings with sim event hints. |
| crates/flight-hotas-honeycomb/tests/snapshots/snapshot_tests__alpha_yoke_full_right_roll.snap | Updated expected roll normalization value/precision. |
| crates/flight-hotas-honeycomb/README.md | Expanded device/features documentation (Alpha/Bravo/Charlie), including new protocol features. |
| compat/devices/honeycomb/bravo-throttle.yaml | Updated support notes to reflect full protocol support in-crate. |
Comment on lines
+471
to
+476
| pub fn with_detents(num_detents: u8) -> Self { | ||
| Self { | ||
| position: 0, | ||
| prev_down: false, | ||
| prev_up: false, | ||
| max_position: num_detents.saturating_sub(1).max(1), |
There was a problem hiding this comment.
FlapSwitchTracker::with_detents implies an arbitrary detent count, but the tracker returns FlapPosition (only 4 states) and max_position can exceed 3. For num_detents > 4, position can grow beyond 3 but will still be reported as FlapPosition::Full, which is misleading. Consider clamping max_position to 3 (4 detents) and documenting the supported range, or change the API to return a numeric position when using custom detent counts. Also, num_detents = 1 currently yields max_position = 1 (2 detents), which is surprising—special-case 0/1 to keep max_position = 0 if a single detent is desired.
Suggested change
| pub fn with_detents(num_detents: u8) -> Self { | |
| Self { | |
| position: 0, | |
| prev_down: false, | |
| prev_up: false, | |
| max_position: num_detents.saturating_sub(1).max(1), | |
| /// | |
| /// The logical flap positions are backed by [`FlapPosition`], which | |
| /// currently supports four discrete states (indices 0–3). To avoid | |
| /// reporting misleading states, the effective detent count is clamped | |
| /// to this range: | |
| /// | |
| /// - `num_detents == 0` or `1` → single detent (index 0 only). | |
| /// - `2 <= num_detents <= 4` → `num_detents` logical detents. | |
| /// - `num_detents > 4` → clamped to 4 logical detents. | |
| pub fn with_detents(num_detents: u8) -> Self { | |
| let max_position = match num_detents { | |
| // Single-detent use: UP only. | |
| 0 | 1 => 0, | |
| // For 2+ detents, clamp to the 4 states that FlapPosition can represent. | |
| n => (n.saturating_sub(1)).min(3), | |
| }; | |
| Self { | |
| position: 0, | |
| prev_down: false, | |
| prev_up: false, | |
| max_position, |
Comment on lines
+216
to
+219
| /// | ||
| /// # Panics | ||
| /// | ||
| /// Panics if `data.len() < 5`. |
There was a problem hiding this comment.
The deserialize_led_report docstring claims the function panics if data.len() < 5, but the parameter type is &[u8; 5] so callers cannot pass a slice shorter than 5 bytes. The # Panics section should be removed or updated to match actual behavior.
Suggested change
| /// | |
| /// # Panics | |
| /// | |
| /// Panics if `data.len() < 5`. |
Comment on lines
+131
to
+135
| ButtonMapping { | ||
| name: "Rocker 1 Up", | ||
| button_num: 7, | ||
| sim_event_hint: "VIEW_ZOOM_IN", | ||
| }, |
There was a problem hiding this comment.
ALPHA_BUTTONS now lists rocker buttons (7–10) after the magneto buttons (25–27), so the table is no longer in ascending button_num order. Reordering these new entries (e.g., placing 7–10 near the other low-numbered controls) would make the mapping easier to scan and maintain.
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
+470
to
+477
| /// Create a flap tracker with a custom number of detents. | ||
| pub fn with_detents(num_detents: u8) -> Self { | ||
| Self { | ||
| position: 0, | ||
| prev_down: false, | ||
| prev_up: false, | ||
| max_position: num_detents.saturating_sub(1).max(1), | ||
| } |
There was a problem hiding this comment.
1. Flap detents can desync 🐞 Bug ✓ Correctness
FlapSwitchTracker::with_detents() can set max_position above 3, but FlapPosition::from_index() clamps all indices >=3 to Full. This allows the internal counter to drift beyond the representable enum state (always reporting Full), and then require multiple “flaps up” edges before the reported position changes away from Full.
Agent Prompt
### Issue description
`FlapSwitchTracker::with_detents()` can set `max_position` beyond what `FlapPosition` can represent (enum only supports indices 0..=3). This lets the internal counter drift beyond 3, while the public API continues returning `FlapPosition::Full`, and then requires extra UP edges to recover.
### Issue Context
- `FlapPosition::from_index()` maps any index >= 3 to `Full`.
- `FlapSwitchTracker::update()` increments internal `position` up to `max_position`.
- `with_detents()` currently sets `max_position` from `num_detents` without clamping to 3.
### Fix Focus Areas
- crates/flight-hotas-honeycomb/src/protocol.rs[470-477]
- crates/flight-hotas-honeycomb/src/protocol.rs[480-496]
- crates/flight-hotas-honeycomb/src/protocol.rs[418-426]
### Suggested direction
- Clamp: `max_position = num_detents.saturating_sub(1).min(3)`
- Consider handling `num_detents == 0` explicitly (e.g., treat as 1 or default to 4) and documenting valid inputs.
- Optionally add tests for `with_detents(0)`, `with_detents(1)`, and `with_detents(>4)` to lock behavior.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Clamp FlapSwitchTracker::with_detents max_position to 3; handle 0|1 detents - Remove invalid '# Panics' docstring from deserialize_led_report - Reorder ALPHA_BUTTONS rocker entries (7-10) to ascending button_num order Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
Member
Author
|
Not auto-merging in this PR review pass — this branch is now ~2 months stale and the merge would delete ~2,900 lines of The new flap/trim trackers and rocker decoding look like genuinely useful additions. Could you rebase onto current Generated by Claude Code |
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
Enhances the light-hotas-honeycomb crate with deep protocol support for Honeycomb Aeronautics flight peripherals.
New features
Fixes