Fix SpaceNavigator device detection - #7544
Conversation
Signed-off-by: Pranav Shirgur <pranav.shirgur@gmail.com>
There was a problem hiding this comment.
Isaac Lab Review Bot
The PR makes the targeted compatibility change by adding the exact HID product string "SpaceNavigator" to the existing Se3SpaceMouse detection allowlist, accompanied by a package changelog fragment and contributor entry.
- Design and architecture: The change extends the existing product-string allowlist at the sole HID detection point without introducing a new abstraction or dependency. The new device remains distinct from the
3Dconnexion Universal Receiverspecial case and therefore follows the established generic SpaceMouse read path. - API: No public symbols, signatures, defaults, or return types change. Detection behavior for the four existing product strings is preserved, and exact matching keeps "SpaceNavigator" distinct from "SpaceNavigator for Notebooks".
- Implementation: The
_find_deviceto_run_devicepath was traced: the newly recognized model uses the existing 7-byte non-receiver report branch, consistent with the reported Linux hardware test. Report layout and axis behavior are supported by manual testing rather than an automated regression test; multi-device selection remains unchanged pre-existing behavior.
No blocking issues. No inline issue met the actionable-evidence threshold; the assessment above records the review feedback.
Automated review; human maintainers own approval decisions.
Greptile SummaryThis PR adds legacy 3Dconnexion SpaceNavigator detection to the existing SpaceMouse HID product-string allowlist.
Confidence Score: 5/5The PR appears safe to merge with no actionable correctness or security issues identified. The narrowly scoped allowlist addition enables the described legacy device while preserving the existing initialization and input-processing behavior. Important Files Changed
Reviews (1): Last reviewed commit: "Fix SpaceNavigator device detection" | Re-trigger Greptile |
There was a problem hiding this comment.
Thanks @5hirgur for the PR! LGTM, @kellyguo11 do we need to QA this sort of changes?
|
run-ci |
# Description LEAPP 0.7.0 introduced two regressions in the Isaac Lab export suite after it was published on September 4: - Kuka-Allegro export re-enters the graph with `robot_root_link_quat_w` after registering the aliased tensor as `robot_root_quat_w`. - SKRL Cartpole export fails while LEAPP reconstructs Warp `BuiltinCallDesc` tuple subclasses during `wp.rand_init` interception. The `isaaclab_rl` package-test job installed bare `leapp`, so the new release entered CI without a repository change. The same failures appeared in unrelated PRs, including #7585 and #7544. This change pins both the public `leapp` extra and the package-test installation to `leapp==0.6.1`. The lockfile now resolves the same version, keeping project environments and CI aligned until the LEAPP 0.7 regressions are fixed upstream. ## Type of change - Bug fix (non-breaking change which fixes an issue) ## Release backport - [x] <!-- backport-active-release --> Backport this pull request to the active release branch after it merges into `develop` ## Screenshots Not applicable. ## Validation - `uv lock --check` passed. - `uv run --extra leapp python -c "..."` resolved and verified LEAPP 0.6.1. - Focused pre-commit YAML and TOML checks passed. - `uv run isaaclab -f` passed every formatting, syntax, metadata, and spelling hook except the changelog-fragment hook. That hook reports four pre-existing `develop`-base fragment problems in `isaaclab_newton`, `isaaclab_tasks`, `isaaclab_experimental`, and `isaaclab_visualizers`; this PR changes no `source/` path. - `git diff --check upstream/develop` passed. ## Checklist - [x] I have read and understood the contribution guidelines. - [x] I have run the pre-commit checks; the unrelated base-branch changelog failure is documented above. - [x] Documentation changes are not required for this dependency pin. - [x] My changes generate no new warnings. - [x] No test was added; dependency resolution and repository metadata checks cover this pinning change. - [x] No changelog fragment is required because no release-managed package under `source/` changes. - [x] My name already exists in `CONTRIBUTORS.md`.
# Description LEAPP 0.7.0 introduced two regressions in the Isaac Lab export suite after it was published on September 4: - Kuka-Allegro export re-enters the graph with `robot_root_link_quat_w` after registering the aliased tensor as `robot_root_quat_w`. - SKRL Cartpole export fails while LEAPP reconstructs Warp `BuiltinCallDesc` tuple subclasses during `wp.rand_init` interception. The `isaaclab_rl` package-test job installed bare `leapp`, so the new release entered CI without a repository change. The same failures appeared in unrelated PRs, including #7585 and #7544. This change pins both the public `leapp` extra and the package-test installation to `leapp==0.6.1`. The lockfile now resolves the same version, keeping project environments and CI aligned until the LEAPP 0.7 regressions are fixed upstream. ## Type of change - Bug fix (non-breaking change which fixes an issue) ## Release backport - [x] <!-- backport-active-release --> Backport this pull request to the active release branch after it merges into `develop` ## Screenshots Not applicable. ## Validation - `uv lock --check` passed. - `uv run --extra leapp python -c "..."` resolved and verified LEAPP 0.6.1. - Focused pre-commit YAML and TOML checks passed. - `uv run isaaclab -f` passed every formatting, syntax, metadata, and spelling hook except the changelog-fragment hook. That hook reports four pre-existing `develop`-base fragment problems in `isaaclab_newton`, `isaaclab_tasks`, `isaaclab_experimental`, and `isaaclab_visualizers`; this PR changes no `source/` path. - `git diff --check upstream/develop` passed. ## Checklist - [x] I have read and understood the contribution guidelines. - [x] I have run the pre-commit checks; the unrelated base-branch changelog failure is documented above. - [x] Documentation changes are not required for this dependency pin. - [x] My changes generate no new warnings. - [x] No test was added; dependency resolution and repository metadata checks cover this pinning change. - [x] No changelog fragment is required because no release-managed package under `source/` changes. - [x] My name already exists in `CONTRIBUTORS.md`. (cherry picked from commit 995d96c)
# Description Backports #7544 to `release/3.0.0` after the automated backport workflow skipped the checked selection. This is an exact cherry-pick of merged commit `6fdadeac68be8f01813a4f260b66b9e85d1af9ae`. It adds legacy 3Dconnexion SpaceNavigator detection, the original changelog fragment, and the original contributor entry. No additional dependencies are required. ## Type of change - Bug fix (non-breaking change which fixes an issue) ## Release backport - This PR directly targets `release/3.0.0`; no further backport is requested. ## Screenshots Not applicable. This change affects HID device detection and does not alter the user interface. ## Testing - [x] Validated the candidate with `.github/scripts/backport.py validate-candidate --exact_patch` against #7544. - [x] Ran the full pre-commit suite; all hooks passed. - [ ] Hardware validation was not repeated for this exact cherry-pick. The original PR was manually tested on Linux with a SpaceNavigator. ## Checklist - [x] I have read and understood the contribution guidelines - [x] I have run the pre-commit checks - [x] Documentation changes are not required because this change does not affect the public API - [x] My changes generate no new warnings - [x] The original fix and validation are preserved exactly - [x] The original changelog fragment is included - [x] The original contributor entry is included Signed-off-by: Pranav Shirgur <pranav.shirgur@gmail.com> Co-authored-by: Pranav Shirgur <49152866+5hirgur@users.noreply.github.com>
PR isaac-sim#7544 added the plain SpaceNavigator to the SE(3) product-string allowlist while this branch was open. Carry that support over to the id-based matching and add the device's USB identifier, which the author verified against hardware (046d:c626), so the SpaceNavigator is also found when the HID backend cannot read its product string.
PR isaac-sim#7544 added the SpaceNavigator without tests. Cover both the product-string path it relied on and the USB-id path, which keeps the device detectable when the HID backend cannot read its descriptors.
Description
This PR adds support for detecting the legacy 3Dconnexion SpaceNavigator as an
Se3SpaceMouseinput device.The device reports
SpaceNavigatoras its HID product string. Because this value was absent from the supported-device check, Isaac Lab raised aNo device found by SpaceMouseerror even when the device was connected and accessible.This change adds
SpaceNavigatorto the existing SpaceMouse product-string allowlist. No additional dependencies are required.Type of change
Release backport
developScreenshots
Not applicable. This change affects HID device detection and does not alter the user interface.
Testing
Manually tested on Linux with a 3Dconnexion SpaceNavigator:
046d:c626SpaceNavigatorSe3SpaceMousefailed to find the connected device../isaaclab.sh --format.Checklist
Docker and GPU tests run on demand. Push the commits you want tested, then comment
run-cion the pull request.pre-commitchecks with./isaaclab.sh --formatsource/<pkg>/changelog.d/for every touched packageCONTRIBUTORS.mdor my name already exists there