-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
macOS support #17
Comments
It is difficult to write applications for macOS when you don't have a mac. I won't do it myself. But If you or someone else wants to try, I can answer questions. Most of the work should be writing the implementation of the |
Fair enough. Unfortunately I am not a programmer myself and my role in the open source projects I am involved (libusb, libusb-win32, libusbk, pyusb, etc) is mainly testing and support side. Hopefully someone can jump in in the future. |
Hi there, I’m interested in adding full support for Logitech mice to https://github.com/noah-nuebling/mac-mouse-fix Of course I’d like to use a library for that if possible so it’s not as much work. @cvuchener Do you recommend I spend the time to try and port this library to macOS for this purpose? I read that you recommend people use ratbag instead of hidpp to build apps (source: #1). But it seems ratbag depends on lots of Linux-specific APIs and would be really hard to port (source). Whereas your library already works on Windows and Linux - which makes me think it’s much easier to port to macOS as well. The most crucial functionalities which I hope to use through this library are
But other functionality such as
would be really interesting, too Thanks a lot for your help, and kudos for making this! |
If you are familiar with macOS HID API and system programming, it should not be too hard. For About writing new tools, everything that only configure the device should be portable. But if you have to generate input events, it will likely be macOS specific. I have a linux-specific driver example (using uinput): hidpp20-raw-touchpad-driver. The way thread are managed are a bit annoying, I should improve that.
Depending on the exact feature used by your mouse, this may be already supported.
I don't have any code for gestures, but I have a few notes on "0x6501-Gesture2". This will need more research.
I think it's done by the kernel driver on Linux. I did not look at it, but it is documented as part of the Unifying Receiver registers (https://docs.google.com/document/d/0BxbRzx7vEV7eNDBheWY0UHM5dEU/edit?resourcekey=0-SPDGsNiO52FX6E-mJIXYXQ)
I don't know anything about that, this will need research too. |
Hey @cvuchener thanks for the quick reply.
I don't have any experience with system programming. So that's surely going to trip me up. I'm familiar with the macOS HID API, though. If I keep close to the Windows implementation I hope I won't make too many mistakes.
Hot-plugging definitely sounds very useful for my use case. I'll try to implement it if I work on this.
I luckily already have macOS code for intercepting and creating input events. So this shouldn't be an issue for my case. However, if you think an update to the command-line tools for macOS is warranted, I should be able to help as I do have experience writing macOS code around input events. Thanks for your assessment on the different features I'd like. I also had a look a the links in your README, and they look very promising! I found what seem to be interfaces for all the features I'm looking for in the Logitech GDrive you've linked (I think so at least, I don't really understand any of this).
Do you think that these interfaces would serve my needs? And do you think your API would make using them easier? |
Hey @cvuchener, I wrote up some stuff in this fork. I adjusted some of the It compiles fine under macOS and functionality-wise it's mostly done. But it doesn't work yet. (And it's probably bad code - again I have very little experience with Cpp or systems programming) I had some questions and concerns while writing the code. You can read them in these commit messages: Right now I'm stuck. I'll try to work on this more some other time, but I thought I'd describe the problem in case you know more: I tried to use the
|
Commit messages are not a place for discussion. It would be more practical to ask your questions here or in a pull request.
It's platform-specific, use whatever you prefer. You will have to write those strings on the command line.
Copy constructor and move constructor. The copy constructor makes a deep copy: the new object should open the same device but do not share any resource with the
Feature reports are not used, you only need to read input reports and write output reports.
Return 0 whenever a read is timed out or interrupted.
They are simple callbacks. The
I hope the debuggee freezes, not the debugger. You should be able to interrupt the debuggee and see where it is stuck. I expects it is in |
Just so you don't get stuck, know that you don't need to implement |
For macOS specific codes, maybe HIDAPI can be of some reference. |
The way to implement You need to make sure that the run loop still detects the source event even if it was signaled before |
The first kind of source (version 0) are not able to wake up the run loop thread, and The second kind (version 1) uses mach ports. If I understand correctly mach ports can work similarly to POSIX pipes. Being a kernel mechanism, it will be able to wake up the run loop thread, in the same way the HID reports do. This is closer to what I'm doing on Linux or Windows. You don't have to care which thread will receive the message and it should not require any synchronization. They may be a bit more complex to set up though. |
Hey guys thanks for all the feedback and info! I read up some more on runLoops, and looked at the hidapi source. Both of which have been very insightful. Regarding
|
Yes I was wondering why hidapi uses a custom runLoopSource to make the runLoop exit. From my understanding, just calling |
I would prefer to not create any thread at this level. And I don't think you need to. Please consider the mach ports way.
A lot is wrong with this:
In general, if you have to use multiple threads avoid as much as possible sharing mutable data and prefer message passing instead. |
Hey, thanks for your feedback.
I'm open to using the approach that you prefer. I'm definitely aware how easily multiple threads can introduce issues that are hard to fix, and I'm fully on board with avoiding multiple threads if possible. However, I don't understand how the implementation would be possible without threads, just using mach ports. To recap and hopefully clarify what I wrote in this reply, the main reason I started using threads, is because of this logic: Preconditions:
Inference:
If you think my argument is flawed, or that there's also a way to prevent this deadlock using mach ports, could you elaborate? Thank you. I hope this is understandable, and if you have more questions let me know. Edit: I thought about it again, and I think I see what you mean now. I now understand how an approach that doesn't spawn an extra thread is possible and I definitely think it's preferable to the current approach.
I guess I didn’t come up with this before because I thought a runLoop was some magical background thing that doesn’t block when it’s invoked and automatically does its work whenever the thread is finished with everything else. But it actually seems to be just a plain
Could you elaborate on this as well? Edit: I added a mutex that, along with the new thread.join() call, should avoid any race conditions I could make out.
I was under the assumption that the thread would be stopped automatically because the following sequence of events will happen
But I haven't been able to test this, yet, and I agree that it's pretty opaque. Also, I see that this can lead to crashes when the thread is destroyed while it's still running. How would you suggest we stop / join the thread? Edit: I added a .join() call and explicit CFRunLoopStop() calls Regarding your other feedback - I mostly agree, and I'll improve the details once everything is functional. Oh and do you have any idea how to fix the errors that I documented in the screenshots above? That would make further testing and debugging much easier. At this point the |
Hey guys, I won't have time to work on this in a while because I'm about to move to another country, but if you have questions about the code I wrote I'll try to find the time to answer. |
while (0 != device.readReport(report))
process(report); At some point you want to stop reading reports because the device has been removed or you want to quit the application. From another thread, you call This is the only thread safety constraint that
Your new thread and its loop will still have to wait for two different events: the HID report and whatever mechanism cancel it. You could have used the same mechanism in the first thread.
Yes.
Actually, I think I was wrong about As I said above you don't need to protect
I did not understand that, the thread is stopped then if you are right. The call to
There has been changes with the report descriptor: instead of storing a raw report descriptor, a parsed report descriptor is stored. It is worrying that you did not get build error from that change. If macOS gives you the raw report descriptor, you need to parse it with hidpp/src/libhidpp/hid/RawDevice_linux.cpp Lines 86 to 92 in 341bf60
Threads are likely the cause of the inconsistent behaviors. Start with something simple, make sure it works, then build upon it. (untested code as example) #include <hid/RawDevice.h>
int main (int argc, char *argv[])
{
HID::RawDevice device (argv[1]);
std::vector<uint8_t> report (256);
int res = device.readReport (report, 10000);
std::cerr << "readReport exited with result " << res << std::endl;
return 0;
} |
Thanks for your clear and informative reply. This should greatly help development. Regarding the crash on the line The latest version of the macOS port (under which I encountered this is error) was already using Since the error occurred when trying to log something in a part of the codebase which I didn't write, I thought the crash might be unrelated to my code. But given what you told me I now assume it's due to my But even if my The same goes for the error where the Other than this I think we're on the same page now. |
I just thought of one more thing:
Couldn't that be problematic if you have a scenario like this: for _ in 1...5:
report = device.readReport(report)
process(report)
device.interruptRead()
# ...
# (Much later somewhere else in the program)
# ...
report = device.readReport(report) # Problem: Will be interrupted immediately due to the previous device.interruptRead() call
process(report) A more elegant solution to this might be to use a function On macOS at least, this would also be more efficient than calling |
This is the expected behavior, not a problem.
This is what is implemented in the dispatcher classes. You will have callbacks for HID++ events.
It would only be easy if you were writing code for macOS only. The code outside of RawDevice cannot know there a CFRunLoop running and take advantage of it. And since the run loop belong to the thread and not the RawDevice, you will have to set up and clean up the loop in each readReport call. If this becomes too big of a performance issue, I may have an idea to fix it later, I'll have to think about it. |
Thanks for the quick reply and information If that's expected behaviour then I see nothing wrong with the current approach. On macOS, using a It's also good to know that there's a callback for HID++ events implemented, that should come in handy. |
I assume |
Something really weird is going on. I removed the threading now, and added safety/logging code around retrieving the reportDescriptor (copied it from the Linux version). Even though the report descriptor looks fine and there's only one thread operating, I still get the weird The exact same error occurs using |
All the crashes I'm experiencing seem to have to have to do with the Do you think there might be something about it that doesn't port well to macOS? |
After replacing So the |
Are you running with logging enabled? If not add Try adding this line when running without logging enabled. diff --git a/src/libhidpp/misc/Log.cpp b/src/libhidpp/misc/Log.cpp
index 7974254..30a63e1 100644
--- a/src/libhidpp/misc/Log.cpp
+++ b/src/libhidpp/misc/Log.cpp
@@ -64,6 +64,7 @@ Log::Category Log::Debug ("debug");
std::mutex Log::_mutex;
Log::Log ():
+ std::ostream (nullptr),
_buf ("null")
{
} Otherwise try running with valgrind (if it is available for your platform) or any address sanitizer. |
Adding that line fixed it! Do you understand what went wrong? |
I've never heard of such device. If it really exists you can set the ID to an obvious dummy value (0? 0xFFFF?) and maybe log a warning. If it is a system error, throw an exception. All properties must be valid, but they could be empty. IDs are simply integers, any value is valid, they may be used to find the device in a database (HID++ 1.0 requires some quirks, not everything can be guessed). Names are used for display. Report descriptors are used for checking which reports are present (an empty descriptor would be valid but would not be accepted as a HID++ device). |
This is what I get under macOS using HIDAPI. Somehow the Logitech bluetooth mouse M557 got an "unknown" entry for "Manufacturer". But the following device is causing the segfault mentioned in the above backtrace.
|
But there is anothe Apple device which can cause the segfaults as well. HIDAPI hidtest is having the following output for this device. This is a bit strange.
|
Yes macOS prompt me for the "Input Monitoring" permissions for hidpp-list-devices, but no prompt for hidpp-list-features. Let me try to add the pemissions for hidpp-list-features to see how it goes. |
@noah-nuebling Looks like the following is similar to yours (Logitech M557 bluetooth mouse).
|
It is also the same for the Logitech MK710 Combo (Unifying receiver with keyboard and M705 mouse)
|
Not so sure if the Linux output will help or not.
|
Wow thanks for the in-depth feedback. I’ll have a closer look at some point. Just one quick thing I spotted, in one of the logs is you posted it says:
TCC manages the privacy permissions, so some of the errors might have something to do with that. So to me it seems like some of the times that the VendorID is NULL it’s because the device actually doesn’t have a vendorID and sometimes it’s because it failed to open the device due to TCC. Pretty weird indeed. …But actually I think opening the device shouldn’t be necessary to read data from it if I remember correctly. So it’s just weird |
@noah-nuebling Is this 1999:23806 vid:pid? It translates to 0x07cf:0x5cfe but there is no such device in my system. |
I have no idea to be honest. We should probably update the code to check whether IOHIDDeviceOpen() was successful and log the circumstances of the failure to investigate this further |
Yes, that would be good. If possible, please also help to implement the "no vendor id" suggestion by Clément. |
I added some of the logging features and default values we talked about on the plane today. I didn't have a mouse or internet so it might not work properly, and I also can't help if something doesn't work because I'm really busy right now. But if you want to take a look it's on my fork. |
@noah-nuebling Great, now hidpp-list-devices works fine under my Mac Mini M1.
Debug log for your info. click to expand for the full debug log
|
No change for hidpp-list-features but I understand it will take you more time to debug this.
|
Hey guys, hope you had a nice Holidays! I just reworked I think the issue was that to To fix this I put the runLoop on its own thread and let it listen for reports all the time. I know Clement was against this, which I understand. It did make things a lot more complicated and error prone. But it works and it's the only thing I can think of that works. Issues I'm aware of:
|
Only the first report, like the device needs to wake up? Or every report is that slow?
It's enough for testing the lowest part of the protocol (the part that is system specific). The bad news is that if the device is really HID++ 1.0, there may not be much code you can reuse. But since the device was released in 2016, I'm surprised it is using the older protocol. Can you try adding |
I'm not sure it seems to be pretty inconsistent. I think it's just
I was interacting with the receiver only.
And Does this mean my mouse has 32% battery? (See hidpp 2.0 spec) |
I found the issue with the timeouts! |
hidpp/src/libhidpp/hid/RawDevice.h Lines 59 to 64 in 3f4fa72
-1 means no timeout |
Oh cool! Everything seems to be working great now. But I'm trying to build The problem is that I can't find an easy way to install Not sure what to do about this. Maybe build I also tried the |
Using |
This Razer driver for macOS sets and retrieves reports without a separate thread using IOUSBDeviceInterface: |
The DeviceRequest() function which they are using can be found here https://opensource.apple.com/source/IOUSBFamily/IOUSBFamily-630.4.5/IOUSBFamily/Headers/IOUSBLib.h.auto.html |
The current code seems to work but this would probably simplify it a lot. Let me know if you would prefer I redo it using these functions. |
DeviceRequestTO() can be used to specify a timeout and USBDeviceAbortPipeZero() might be used to interrupt read requests. (Both found in the same header as DeviceRequest()) This seems perfect. |
IOHIDDeviceCreate() might be used as a guide for obtaining those plug-in interfaces. See: |
Here’s how the razer driver gets the usb interface: |
Okay I dug into this some more and I don't think it will work. I think the way we currently do things with the threads and stuff is the best way to do things. Even though it's not good. See this commit message for more info. |
The macOS fork from @noah-nuebling seems to work pretty well.
|
It would be nice to have macOS support. macOS has native HID API which should do the work.
The text was updated successfully, but these errors were encountered: