feat(hid-rp): add 3Dconnexion SpaceMouse HID report descriptor - #790
Conversation
Add SpaceMouseTranslationInputReport, SpaceMouseRotationInputReport, SpaceMouseButtonsInputReport, and SpaceMouseLedOutputReport, following the existing hid-rp-xbox.hpp/hid-rp-gamepad.hpp conventions. The report layout (Generic Desktop / Multi-Axis Controller usage, per-axis 16-bit signed logical/physical ranges, relative data flag, report ID split across translation/rotation/buttons) was verified against a genuine 3Dconnexion SpaceNavigator HID report descriptor rather than guessed; sources are cited in the header. Includes a host-buildable test (verified with `c++ -std=c++20 -Wall -Wextra -Werror`) and docs/example wiring. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
|
✅Static analysis result - no issues found! ✅ |
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Adds first-class HID report descriptor support for 3Dconnexion SpaceMouse devices to the hid-rp component, including a descriptor helper, formatters, docs/example wiring, and a host-buildable verification test.
Changes:
- Added SpaceMouse translation/rotation/buttons/LED report classes +
spacemouse_descriptor()helper. - Added
fmt::formatterspecializations for SpaceMouse report types. - Updated docs/example and added a host-buildable test that validates report bytes and descriptor contents.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| doc/en/hid/hid-rp.rst | Documents SpaceMouse support and links API reference include for the new header. |
| components/hid-rp/include/hid-rp-3dconnexion.hpp | Implements SpaceMouse HID report classes and a combined descriptor helper. |
| components/hid-rp/include/hid-rp-3dconnexion-formatters.hpp | Adds fmt formatters for the new SpaceMouse report types. |
| components/hid-rp/example/main/hid_rp_example.cpp | Demonstrates generating the SpaceMouse descriptor and formatting reports. |
| components/hid-rp/README.md | Updates component README to mention SpaceMouse support. |
| components/hid-rp/test/hid_rp_3dconnexion_host_test.cpp | Host-buildable test validating report layouts and descriptor byte patterns. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Add missing direct includes (<vector>, <cstdint>, <cstddef>) to hid-rp-3dconnexion.hpp instead of relying on transitive includes. - Clamp set_data() copy length to num_data_bytes in all four report classes (translation, rotation, buttons, LED) so an over-long input vector cannot write past the backing storage. - Make hid-rp-3dconnexion-formatters.hpp self-contained by adding <bitset>, <cstddef>, <cstdint>, and including hid-rp-3dconnexion.hpp. - Strengthen the host test's descriptor-size precondition from `> 0` to `>= 4` before indexing descriptor[0..3], fixing the cppcheck containerOutOfBounds warning. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
…Mouse reports - Thread the fmt output iterator through every format_to() call in the SpaceMouseButtonsInputReport formatter instead of discarding the advanced iterator, matching correct fmt::formatter usage. - Zero-fill the remaining payload bytes in set_data() (translation, rotation, buttons, LED reports) when the supplied data is shorter than the report's payload, so a short write always produces a well-defined report instead of leaving stale bytes from a previous update. - Wrap SpaceMouseButtonsInputReport in #pragma pack(push, 1) and add a constructor static_assert verifying sizeof(report) == 1 + num_data_bytes, making the report-id-plus-payload contiguity that get_report()/set_data() rely on explicit and self-checking rather than implicit. - Replace the anonymous bitfield backing SpaceMouseLedOutputReport's LED byte (implementation-defined packing) with an explicit uint8_t payload byte plus mask/shift accessors, and add the same pack+static_assert layout guarantee, since this byte is serialized directly to/from the wire. Host test (components/hid-rp/test/hid_rp_3dconnexion_host_test.cpp) still passes; descriptor size unchanged at 115 bytes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
…bytes Self-review follow-ups: - Add the same sizeof() static_assert to the translation/rotation report constructors that the buttons/LED reports already have, so all four wire layouts are compiler-checked (1 id byte + payload, no padding). - The host test now asserts the property the descriptor exists to replicate: translation/rotation are Input(Data,Var,Relative) `81 06` (exactly two), buttons stay absolute `81 02`, the axis usages X/Y/Z + Rx/Ry/Rz are present, the LED report is Usage Page(LEDs)/Generic Indicator/Output(Data,Var,Abs), and the 8-button variant actually carries Report Count(8)/Usage Maximum(8) (previously only its length was compared). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
|
Self-review follow-up (pushed): added the missing |
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Add a static_assert(std::endian::native == std::endian::little) to the translation/rotation reports documenting and enforcing the existing little-endian wire-format assumption (matches the object-representation-copy idiom already used by hid-rp-gamepad.hpp/hid-rp-xbox.hpp). Add BUTTON_COUNT >= 1 and <= 255 static_asserts to SpaceMouseButtonsInputReport and spacemouse_descriptor() (the upper bound is required because get_descriptor() encodes REPORT_COUNT as a 1-byte item). Make the LED output report (Report ID 4) actually optional via a new INCLUDE_LED template parameter on spacemouse_descriptor(), defaulting to true to preserve the current 115-byte descriptor. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
Keep this branch's wording for the WDI README dependency table and manifest description: the device-role PR said the host-role headers arrive in a follow-up, and this PR is that follow-up. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
Summary
components/hid-rp/include/hid-rp-3dconnexion.hppimplementing HID report classes for the 3Dconnexion SpaceMouse family (SpaceNavigator, SpaceMouse Wireless/Compact/Pro/Enterprise):SpaceMouseTranslationInputReport(Report ID 1),SpaceMouseRotationInputReport(Report ID 2),SpaceMouseButtonsInputReport(Report ID 3, button count is a template parameter), andSpaceMouseLedOutputReport(Report ID 4, optional), plus aspacemouse_descriptor()helper combining them into a complete application-collection descriptor. Follows the conventions ofhid-rp-xbox.hpp/hid-rp-gamepad.hpp(constexpr accessors,get_report()/set_data(),get_descriptor(), Doxygen\section/\snippet).hid-rp-3dconnexion-formatters.hppwithfmt::formatterspecializations, matching the existing per-device formatter headers.hid_rp_example.cpp) and docs (doc/en/hid/hid-rp.rst,components/hid-rp/README.md).components/hid-rp/test/hid_rp_3dconnexion_host_test.cpp) that exercises the axis/button/LED accessors, the byte-level report layout (little-endian round trip), and checks the generated descriptor bytes against the verified-real-hardware byte sequences.Verified report layout (not guessed)
The usages, 16-bit signed logical range
[-350, 350], physical range[-1400, 1400](unit exponent -4, SI Linear/centimeter), and the "relative" data flag were taken from a genuine 3Dconnexion SpaceNavigator HID report descriptor, decoded byte-for-byte, not guessed:Two intentional deviations from a byte-for-byte clone of the real descriptor, both documented in the header:
BUTTON_COUNT, matchingespp::GamepadInputReport's convention so it generalizes across the SpaceMouse family's varying button counts (2 to 15+).Collection (Physical)with no precedingUsageitem, and a redundantUsage Page (Generic Desktop)beforeUsage Page (Button)in the buttons collection; this implementation omits those specific quirks (which don't affect parsing) while preserving every usage, report ID, bit size, and logical/physical range/unit/flag exactly.Test plan
c++ -std=c++20 -Wall -Wextra -Werror -isystem components/hid-rp/include -isystem components/hid-rp/detail/hid-rp/hid-rp -isystem components/format/include -isystem components/format/detail/fmt/include components/hid-rp/test/hid_rp_3dconnexion_host_test.cpp -o /tmp/hid_rp_3dconnexion_host_test && /tmp/hid_rp_3dconnexion_host_test→ALL TESTS PASSED(descriptor size: 115 bytes for the default 2-button configuration)🤖 Generated with Claude Code
https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU