feat(hotas-vkb): add protocol parser, configuration, and axis mapping#64
Open
EffortlessSteven wants to merge 2 commits into
Open
feat(hotas-vkb): add protocol parser, configuration, and axis mapping#64EffortlessSteven wants to merge 2 commits into
EffortlessSteven wants to merge 2 commits into
Conversation
- Implement VKB USB HID protocol parsing with report descriptor decoding - Add device family detection for Gladiator, Gunfighter, and SEM lines - Add configuration reading for button/hat/axis layout from device reports - Implement axis mapping with calibration, deadzone, and curve support - Add 30+ unit tests covering protocol, configuration, and axis mapping 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 selected for processing (4)
✨ 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 VKB protocol parser, axis mapping, and configuration support
WalkthroughsDescription• Add VKB protocol parser with unified VkbProtocol struct for joystick-class devices • Implement axis mapping module with device-family-specific resolution and normalization • Add configuration parsing for firmware-level device settings (sensitivity, deadzone, curves) • Provide 42 new unit tests covering protocol, axis mapping, and configuration parsing Diagramflowchart LR
A["Raw HID Report"] -->|VkbProtocol| B["StickState"]
A -->|axis_mapping| C["Normalized Axes"]
A -->|configuration| D["VkbConfig"]
B --> E["Unified API"]
C --> E
D --> E
File Changes1. crates/flight-hotas-vkb/src/axis_mapping.rs
|
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
+339
to
+390
| impl VkbProtocol { | ||
| /// Create a protocol handler for a VKB device identified by VID/PID. | ||
| /// | ||
| /// Returns `None` if the VID is not VKB or the PID is not a known joystick. | ||
| pub fn new(vid: u16, pid: u16) -> Option<Self> { | ||
| if vid != VKB_VENDOR_ID { | ||
| return None; | ||
| } | ||
| VkbDeviceFamily::from_pid(pid).map(|family| Self { | ||
| family, | ||
| has_report_id: false, | ||
| }) | ||
| } | ||
|
|
||
| /// Enable stripping a 1-byte HID report ID prefix before parsing. | ||
| pub fn with_report_id(mut self, enabled: bool) -> Self { | ||
| self.has_report_id = enabled; | ||
| self | ||
| } | ||
|
|
||
| /// Return the detected device family. | ||
| pub fn family(&self) -> VkbDeviceFamily { | ||
| self.family | ||
| } | ||
|
|
||
| /// Parse a raw HID report into a unified [`StickState`]. | ||
| /// | ||
| /// Works for Gladiator NXT EVO, Gunfighter, and related joystick families. | ||
| /// For SEM THQ or STECS, use the dedicated parsers instead. | ||
| pub fn parse_report(&self, report: &[u8]) -> Result<StickState, VkbProtocolParseError> { | ||
| let payload = if self.has_report_id { | ||
| report.get(1..).unwrap_or(&[]) | ||
| } else { | ||
| report | ||
| }; | ||
|
|
||
| const MIN_LEN: usize = 12; | ||
| if payload.len() < MIN_LEN { | ||
| return Err(VkbProtocolParseError::ReportTooShort { | ||
| expected: MIN_LEN, | ||
| actual: payload.len(), | ||
| }); | ||
| } | ||
|
|
||
| let axes = StickAxes { | ||
| roll: normalize_signed(le_u16(payload, 0)), | ||
| pitch: normalize_signed(le_u16(payload, 2)), | ||
| yaw: normalize_signed(le_u16(payload, 4)), | ||
| mini_x: normalize_signed(le_u16(payload, 6)), | ||
| mini_y: normalize_signed(le_u16(payload, 8)), | ||
| throttle: normalize_u16(le_u16(payload, 10)), | ||
| }; |
There was a problem hiding this comment.
1. Protocol accepts non-stick families 🐞 Bug ✓ Correctness
VkbProtocol::new() can construct a protocol for families like SemThq/GladiatorMk2 (because from_pid returns them), but parse_report() always interprets the payload as the 6-axis stick layout. This can silently return incorrect axes/buttons/hats for non-stick layouts, despite the docs implying only joystick-class devices are accepted.
Agent Prompt
## Issue description
`VkbProtocol::new()` currently accepts any PID recognized by `VkbDeviceFamily::from_pid`, including non-stick families like SEM THQ, but `parse_report()` always parses the 6-axis stick layout. This can silently mis-parse reports.
## Issue Context
The codebase already defines a distinct SEM THQ layout (4 axes, no hat) via `VKB_SEM_THQ_LAYOUT`/`report_layout_for_family`.
## Fix Focus Areas
- Add an explicit family gate in `VkbProtocol::new()` (or `parse_report()`) so only stick families are supported, or implement family-aware parsing and return an `UnsupportedFamily` error.
- Update docs/tests to reflect the enforced behavior.
### Focus references
- crates/flight-hotas-vkb/src/protocol.rs[339-419]
- crates/flight-hotas-vkb/src/protocol.rs[39-71]
- crates/flight-hotas-vkb/src/protocol.rs[106-133] সচ
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Use report_layout_for_family() to dynamically select axis count, button word offsets, and hat presence based on device family. Fixes silent mis-parsing for non-stick families like SemThq (4 axes, no hat). 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 ~3,000 lines of The new 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
Adds VKB device protocol support to the
flight-hotas-vkbcrate with three new capabilities:New Modules
configuration.rs— VKB firmware configuration parsingVkbConfig,ConfigProfile,CurveTypetypesread_config_from_report()for HID report parsingaxis_mapping.rs— Device-family-specific axis resolutionVkbAxisenum covering all physical axesGladiatorAxisMapandGunfighterAxisMapwith resolution dataresolve_axis()andresolve_axis_by_name()helpersModified Modules
protocol.rs— Unified protocol handlerVkbProtocolstruct with family detection and report parsingStickState/StickAxesunified state typesVkbProtocolParseErrorfor structured error handlinglib.rs— Module registration and public re-exportsTesting
cargo clippy -p flight-hotas-vkb -- -D warningscleancargo fmtclean