Skip to content

Comments

Update get_connected_devices API and examples to more clearly handle multiple devices of different types#47

Open
peter-mitrano-ar wants to merge 5 commits intomasterfrom
fix-testing-and-examples
Open

Update get_connected_devices API and examples to more clearly handle multiple devices of different types#47
peter-mitrano-ar wants to merge 5 commits intomasterfrom
fix-testing-and-examples

Conversation

@peter-mitrano-ar
Copy link
Collaborator

@peter-mitrano-ar peter-mitrano-ar commented Feb 9, 2026

Previously, the function only returned device type names, e.g. SpaceMouseCompact or SpaceMousePro. But these names do not uniquely identify the device. If you have two of the same device, you cannot know which one is which.

Now, it returns both names and file paths (e.g. [('SpaceMouseCompact', '/dev/hidraw4')])

For example, you can now get print out of the device names (types) and paths:

(pyspacemouse) peter@T15:~/code/PySpaceMouse$ pyspacemouse --list-connected
(pyspacemouse) peter@T15:~/code/PySpaceMouse$ pyspacemouse --list-connected
Connected SpaceMouse devices:
  - SpaceMouseCompact (/dev/hidraw2)
  - SpaceMouseWirelessNew (/dev/hidraw3)

@sourcery-ai
Copy link

sourcery-ai bot commented Feb 9, 2026

Reviewer's Guide

This PR changes get_connected_devices to return (device_path, device_name) tuples instead of plain names and updates the API consumers (examples and CLI) to use paths for device selection and display, improving support for multiple identical devices.

Sequence diagram for updated device listing via CLI

sequenceDiagram
    actor User
    participant PySpaceMouseCLI as pyspacemouse_cli
    participant API as api
    participant HID as hid

    User->>PySpaceMouseCLI: list_spacemouse_cli()
    PySpaceMouseCLI->>API: get_connected_devices()
    API->>API: device_specs = get_device_specs()
    API->>HID: hid.find()
    HID-->>API: hid_devices
    loop match_supported_devices
        API->>API: compare vendor_id and product_id with device_specs
        API->>API: devices_by_path[hid_device.path] = device_name
    end
    API-->>PySpaceMouseCLI: list(devices_by_path.items())
    alt devices_found
        loop for_each_device
            PySpaceMouseCLI->>PySpaceMouseCLI: print device_name and device_path
        end
    else no_devices_found
        PySpaceMouseCLI->>PySpaceMouseCLI: print no devices message
    end
    PySpaceMouseCLI-->>User: console output with names and paths
Loading

Flow diagram for updated get_connected_devices return structure

flowchart TD
    A["Start get_connected_devices"] --> B["Call get_device_specs"]
    B --> C["Initialize devices_by_path as empty dict"]
    C --> D["Call hid.find to get hid_devices"]
    D --> E{Any hid_device remaining?}
    E -->|Yes| F["Take next hid_device"]
    F --> G["Compare vendor_id and product_id with each spec in device_specs"]
    G --> H{Match found?}
    H -->|Yes| I["Set devices_by_path[hid_device.path] = device_name"]
    H -->|No| E
    I --> E
    E -->|No| J["Convert devices_by_path.items to list of (device_path, device_name)"]
    J --> K["Return list of (device_path, device_name)"]
    K --> L["End get_connected_devices"]
Loading

File-Level Changes

Change Details Files
Update get_connected_devices to return unique device path/name pairs and adjust default selection in open().
  • Change get_connected_devices return type from List[str] of names to List[Tuple[str, str]] of (device_path, device_name).
  • Deduplicate devices by HID path using a dictionary keyed by hid_device.path before returning items.
  • Update open() to treat connected[0] as a (path, name) tuple and use the name element when defaulting the device argument.
pyspacemouse/api.py
Adapt multi-device example to the new path-based API and clarify console output.
  • Replace get_connected_devices usage to unpack into separate lists of paths and names.
  • Use open_by_path with explicit device paths rather than device_index-based open().
  • Tweak printed labels from shorthand 'L/R' to 'Left/Right' for clarity.
examples/03_multi_device.py
Adjust discovery example to work with (path, name) tuples while preserving name-based checks for supported devices.
  • Derive connected_names list from get_connected_devices results and iterate over it when printing connected devices.
  • Update supported device listing to check connection status using membership in connected_names.
examples/05_discovery.py
Update custom configuration examples to handle (path, name) tuples and constrain them to a single connected device.
  • Add guard clauses to require exactly one connected device for the examples to run.
  • Unpack the single connected device into (device_path, device_name) for subsequent use.
  • Switch base_spec lookup in example_modify_existing to use specs[device_path] instead of specs[device_name].
  • In example_invert_rotations, keep base_spec lookup by device_name but use the unpacked tuple and fix naming passed to modify_device_info.
examples/09_custom_config.py
Enhance CLI listing to display both device names and their paths using the new get_connected_devices contract.
  • Iterate over (path, device) tuples from get_connected_devices.
  • Print each device as 'name (path)' to make multiple identical devices distinguishable in CLI output.
pyspacemouse/pyspacemouse_cli.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue, and left some high level feedback:

  • In examples/09_custom_config.py, base_spec = specs[device_path] looks incorrect since get_device_specs() appears to be keyed by device name (as used elsewhere in the file); this should likely index by device_name instead to avoid a runtime KeyError.
  • Changing get_connected_devices() from returning a list of names to a list of (path, name) tuples is a breaking API change; consider providing a backward-compatible helper or alias (e.g., a wrapper that returns just names) to avoid surprising existing users.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `examples/09_custom_config.py`, `base_spec = specs[device_path]` looks incorrect since `get_device_specs()` appears to be keyed by device name (as used elsewhere in the file); this should likely index by `device_name` instead to avoid a runtime KeyError.
- Changing `get_connected_devices()` from returning a list of names to a list of `(path, name)` tuples is a breaking API change; consider providing a backward-compatible helper or alias (e.g., a wrapper that returns just names) to avoid surprising existing users.

## Individual Comments

### Comment 1
<location> `examples/09_custom_config.py:39` </location>
<code_context>

     # Get base spec and create modified version
-    base_spec = specs[device_name]
+    base_spec = specs[device_path]
     print(f"Original mappings: y scale = {base_spec.mappings['y'].scale}")

</code_context>

<issue_to_address>
**issue (bug_risk):** Using `device_path` as the key into `specs` is likely incorrect and will raise at runtime.

In `example_modify_existing`, `specs` still comes from `pyspacemouse.get_device_specs()`, which is keyed by device name, not device path. In `example_invert_rotations` you already use `specs[device_name]`. Using `device_path` here will likely raise a `KeyError`; this should be `base_spec = specs[device_name]` to match the expected `specs` structure.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant