MacOS Hotplug: connection-callback implementation for MacOS #653
No reviewers
Labels
No labels
3rdparty
Android
API
Author does not respond
bsd
bug
build system/CI
ci-virtual-device
CMake
Contributions Welcome
Core
dependencies
device-specific
documentation
don't_merge
duplicate
enhancement
Example
Haiku
hidraw
hotplug
info required
invalid
libusb
macOS
Misc
Not an issue
question
Release
Test
Test GUI
Windows
wontfix
No milestone
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
gh-libusb/hidapi!653
Loading…
Reference in a new issue
No description provided.
Delete branch "connection-callback-mac"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
This is a
MOCK-UP of afinalized but untested hotplug implementation for Mac OS. It has the entire structure already set up (callback list management, device cache, mutex - everything copied straight from Linux version), but may still have some issues with RunLoop and event processing, which I will not be able to resolve myself because of a total lack of any meaningful documentation.github action builds failed for macOS (automake and cmake)
I managed to set up a build environment two days ago and it showed me parts that I had not considered, currently trying to understand their API.
Welp, this was just a mock-up, after all :)
convertin into draft for now
Found an example implementation that should aid in finishing this code
https://code.qt.io/cgit/qt-creator/qt-creator.git/tree/src/plugins/ios/iosdevice.cpp?h=master
libusb macOS supports hotplug. Maybe that can be a reference as well.
https://github.com/libusb/libusb/blob/master/libusb/os/darwin_usb.c
It took me a while to understand that the approach I took was in a completely wrong direction (thanks, bad documentation!), and the example I linked - as well as the example in libusb - are not suitable for hidapi. The reason is that hidapi relies on IOHIDDeviceRef's to get info on usage pages - and we can't get said info from io_object_t that we get from the callbacks in all of those examples.
The right approach would be to:
I will, of course, try to make a fresh mock-up within the next few days. I do have a functional build environment now, so I will be able to check if it builds - but I will not be able to check if it reacts to the events as expected.
UPD: it would have been nice if we at least had any kind of documentation regarding the state of structures provided by macos in the disconnect callback :(
It's like navigating through fog
It took a while, but the implementation is finished and ready for a review.
This is my first time ever writing code for MacOS, so it might contain non-optimal solutions or straight up incorrect understanding of the API - the official documentation turned out to be garbage and the code had to be written half-intuitively.
It might require some adjustment regarding the RunLoop for the hotplug event processing (I couldn't find any reliable information to ensure it would work as expected with the current state of the implementation).
Checked the failed build, there were warnings treated as errors. Fixed them.
Ran some simple tests in a VM, all the initialization seems to work, but the events are not fired. Currently investigating how to fix runloops, should be done with that tomorrow. Converting to draft until I'm done with it.
@Youw
Windows-cmake github action failed but it is not related to this PR. Strange.
Great.
I see the same thing under my Mac Mini M1 2020 (latest macOS 14.3.1) -- no events fired. So it is good your VM behaves the same.
I just rebased this branch onto the fresh
masterand squashed all the commits so as not to clog the history. This should fix the issue with Windows CI failing and also make it easier to merge when it's all finished. It might be a good idea to rebase theconnection-callbackbranch itself.I added some code for the background event thread, going to test that before doing anything else with the PR.
@mcuee Ran tests on the new fixed implementation.
It successfully receives the connection events and processes them (seemingly) as expected.
The internal callback for disconnection events is also called, but so far it is unable to match the disconnected device with the cached ones, it is unable to generate a path for the disconnected device, need to find another way to match them.
IORegistryEntryGetRegistryEntryID returns 268435459, which isn't even a valid return code for a kern_return_t.
I will try to somehow store io_service_t values with the hid_device_info in the cache, they seem to work.
So far, it's all good news.
It worked. I'm gonna need confirmation that this is an allowed method to store io_service_t (I made the
create_device_info_with_usagealways allocate an "extended" structure), but it all works perfectly. If it's not the right solution to store additional data, I'm gonna need advice on what is the preferred method.hidtest output AFTER enumeration
@mcuee may I ask to re-test on a real Mac with the latest changes?
I believe the code to be ready for that now
@k1-801
It seems to work fine under my Mac Mini M1 2020 (macOS 14.3.1).
Testing with Microsoft Bluetooth Mouse is also good.
Remove battery to simulate unplug and then put back battery to simulate plug back in.
Amazing! Thank you!
Now it just needs to pass the codestyle review, I guess
I am more on the testing side. @Youw is the current active developer and he will be reviewing the PRs.
@Youw I apologize for the PR being a bit messy. The target branch needs a rebase onto the fresh master. I had to do it for this branch to avoid other issues, so the diff includes all the commits that didn't yet make their way into
connection-callback. Once that happens, I will rebase this branch ontoconnection-callbackand the diff will clear up.That I can do (if no conflicts)
I'm afraid there are a few, but they aren't that bad
it looked trivial, I believe I didn't break anything but need to check
All done on my side as well 👍
No issue with latest commit.
thanks
maybe a separate PR with the fix into master?
if will definitely get there faster as a separate change, than the feature branch
Same deal as with https://github.com/libusb/hidapi/pull/645 - getting it merged into a feature branch now.
No need - it was done the same way in the Windows Mutex PR (was asked to fix indentation and also had troubles with no
fflush()), so this change no longer even pops up in the diffoh, I only meant replacing spaces with tabs inside of this function
everything else, obviously, is feature/branch-specific