Avoid taking an exclusive lock on Litra devices on macOS #93
Conversation
Currently, when a Litra handle device is opened with `open()`, on macOS we take an exclusive lock on the device, which stops it being managed by other applications. This isn't a problem if the device handle is quickly closed again (e.g. if the application exits), but it can be annoying if you have a long-running application that holds onto the device handle for a long time (e.g. `litra auto-toggle`). As an example, if you run `litra auto-toggle` and then try to turn the device on or off with `litra on` or `litra off` in another shell, it would fail with an error: > HID error occurred: hidapi error: hid_open_path: failed to open IOHIDDevice from mach entry: (0xE00002C5) (iokit/common) exclusive access and device already open This switches to taking a non-exclusive lock on macOS, affecting use of this code as a Rust library and our long-running `auto-toggle` command.
WalkthroughThe changes in this pull request involve updates to the Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
👍 Looks good to me! Reviewed everything up to a945f6c in 9 seconds
More details
- Looked at
25lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. src/lib.rs:183
- Draft comment:
The documentation comment for theopenmethod should be updated to reflect that the device is opened in non-exclusive mode on macOS. This is important for users to understand the behavior change. - Reason this comment was not posted:
Confidence changes required:50%
The PR changes the lock mode to non-exclusive on macOS, which is correctly implemented using conditional compilation. However, the documentation comment for theopenmethod should be updated to reflect this change.
Workflow ID: wflow_DXUwniNG0ARIfBij
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/lib.rs (1)
183-183: Approved: Clear documentation update for macOS behaviorThe added note about non-exclusive mode on macOS is accurate and consistent with the implementation change. This provides valuable information to users about platform-specific behavior.
Consider rephrasing slightly for even more clarity:
- /// device status. On macOS, this will open the device in non-exclusive mode. + /// device status. Note: On macOS, the device will be opened in non-exclusive mode.This minor change emphasizes the platform-specific nature of the behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- src/lib.rs (2 hunks)
🔇 Additional comments (2)
src/lib.rs (2)
52-55: Excellent implementation of non-exclusive mode for macOSThis change directly addresses the PR objective by setting the HidApi to open devices in non-exclusive mode on macOS. The use of the conditional compilation directive ensures that this behavior is specific to macOS, maintaining compatibility with other platforms.
Line range hint
1-483: Summary: Successful implementation of non-exclusive locks on macOSThe changes in this file effectively address the PR objective of avoiding exclusive locks on Litra devices on macOS. The implementation is clean, well-integrated, and properly documented. These modifications enhance the usability of the code as a Rust library and improve the functionality of the
auto-togglecommand without affecting other platforms or the overall structure of the library.Key points:
- Non-exclusive mode is set for macOS in the
Litra::new()method.- The
Device::open()method documentation is updated to reflect the macOS-specific behavior.- The changes are minimal and focused, maintaining the integrity of the existing codebase.
Currently, when a Litra handle device is opened with
open(), on macOS we take an exclusive lock on the device, which stops it being managed by other applications.This isn't a problem if the device handle is quickly closed again (e.g. if the application exits), but it can be annoying if you have a long-running application that holds onto the device handle for a long time (e.g.
litra auto-toggle).As an example, if you run
litra auto-toggleand then try to turn the device on or off withlitra onorlitra offin another shell, it would fail with an error:This switches to taking a non-exclusive lock on macOS, affecting use of this code as a Rust library and our long-running
auto-togglecommand.Important
On macOS,
Litra::new()now avoids exclusive locks, allowing concurrent device access.Litra::new()now setsset_open_exclusive(false)to avoid exclusive locks on devices.open()docstring inDeviceto indicate non-exclusive mode on macOS.hidapi.set_open_exclusive(false);inLitra::new()for macOS.This description was created by
for a945f6c. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Documentation
openmethod, clarifying non-exclusive mode behavior on macOS.