Skip to content
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: empty path for devices in long USB chain #236

Closed
todbot opened this issue Jan 22, 2021 · 14 comments
Closed

MacOS: empty path for devices in long USB chain #236

todbot opened this issue Jan 22, 2021 · 14 comments
Assignees
Labels
bug Something isn't working macOS Related to macOS backend

Comments

@todbot
Copy link
Contributor

todbot commented Jan 22, 2021

Problem: Plug two 8-port USB hubs in a chain, add a HID device to second hub: hidapi returns empty string for path.

This was an old signal11 issue signal11/hidapi#301 and I think we still have it. Over in node-hid land (node-hid/node-hid#417), someone was experiencing this problem and had a good test showing both hidapitester and node-hid being able to get a path when the device is plugged in at one leaf of the USB tree but not at the other.

I think the issue happens at Line 464 of hidapi/mac/hid.c and I think it's because io_string_t is 512 characters and MacOS USB paths are already > 200 chars if you have just a single external USB hub.

I'm looking into finding definitions of io_string_t and IORegistryEntryGetPath() to see what our options are.

(The reason why I specify 8-port hubs above is those are implemented as two 4-port hubs, meaning the external USB tree look can look like: hub->hub->hub->hub->device, depending on where you plug what into what)

@todbot
Copy link
Contributor Author

todbot commented Jan 22, 2021

It looks like perhaps a solution is to use IORegistryEntryGetRegistryEntryID() instead of IORegistryEntryGetPath(): Yubico/python-fido2@61c2a5d

@Youw
Copy link
Member

Youw commented Jan 22, 2021

Related to #127 and flirc/hidapi@8d251c3 (the fix)?

@todbot
Copy link
Contributor Author

todbot commented Jan 23, 2021

I think so. I'll be attempting it this weekend.

@mcuee mcuee added the macOS Related to macOS backend label Jan 26, 2021
@mcuee
Copy link
Member

mcuee commented Jun 14, 2021

@Youw Maybe this can be merged as well. flirc/hidapi@8d251c3

@Youw
Copy link
Member

Youw commented Jun 14, 2021

I'd suggest a few changes to flirc/hidapi@8d251c3 before accepting it.

@mcuee
Copy link
Member

mcuee commented Aug 7, 2021

#249 (comment)

I was hit by this issue as well, since I have two external powered USB hubs (one 7-port and one 11-port) which are actually complicate hubs (nested hub). So even the first hub (D-Link USB 3 powered 7-port hub) already cause issues.

@mcuee
Copy link
Member

mcuee commented Aug 8, 2021

@Youw @todbot It seems to me that hidapi has changed a lot that commit todbot@8839e54 no longer applies. Please help if you got time. Thanks.

@Youw
Copy link
Member

Youw commented Aug 12, 2021

I'll work on it this week.

@Youw Youw self-assigned this Aug 12, 2021
@noah-nuebling
Copy link

noah-nuebling commented Aug 14, 2021

Maybe we could use IORegistryEntryGetPath() and simply pass it a char buffer that‘s greater than 512 bytes.

(Since io_string_t is just a char[512])

Notice that the Swift version of this function just uses a char buffer of any size and the docs mention nothing about a 512 character limit.

@Youw
Copy link
Member

Youw commented Aug 14, 2021

I'm confident that won't work. The implementation of IORegistryEntryGetPath expects char[512] as an input, which practically means that that parameter is passed by pointer/reference, but even if we pass a pointer/reference to a larger buffer - the implementation will only use what it originally expects.

@noah-nuebling
Copy link

I think it might be worth a shot.

The documentation reads like it just expect a char buffer of any length not just 512. E.g. „IORegistryEntryGetPath() will fail […] if the buffer is not large enough to contain the path.“

Youw added a commit that referenced this issue Aug 14, 2021
With some device connection configurations,
the device paths become over 512 bytes (io_string_t max length)
which makes them unusable with current implementation.

Rather than using ServiceRegistry string path, use its ID,
which is uint64_t and easily serializable/deserializable into a string.

Implementation idea by felix.schwarz@iospirit.com
flirc/hidapi@8d251c3

Fixes: #127, #236.
@Youw
Copy link
Member

Youw commented Aug 14, 2021

And the documentation is missleading. The buffer size is not passed into IORegistryEntryGetPath. It is hard-coded in its declaration.

Youw added a commit that referenced this issue Aug 14, 2021
With some device connection configurations,
the device paths become over 512 bytes (io_string_t max length)
which makes them unusable with current implementation.

Rather than using ServiceRegistry string path, use its ID,
which is uint64_t and easily serializable/deserializable into a string.

Implementation idea by felix.schwarz@iospirit.com
flirc/hidapi@8d251c3

Fixes: #127, #236.
@noah-nuebling
Copy link

noah-nuebling commented Aug 14, 2021

This probably isn't relevant anymore since the ID-based method is already implemented, but just out of interest, I dug through the Apple open source files and took a look at the implementation of IORegistryEntryGetPath(), and you're right - the limit of 512 characters is hardcoded into it.

However I found a newer pair of functions called IORegistryEntryCopyPath() and IORegistryEntryCopyFromPath() which seem to do the same thing as IORegistryEntryGetPath() and IORegistryEntryFromPath() but relying on CFStringRef instead of io_string_t. I'm quite certain that these functions don't have the 512 character limit.

However they are only available starting with macOS 10.11.

Youw added a commit that referenced this issue Aug 23, 2021
With some device connection configurations,
the device paths become over 512 bytes (io_string_t max length)
which makes them unusable with current implementation.

Rather than using ServiceRegistry string path, use its ID,
which is uint64_t and easily serializable/deserializable into a string.

Implementation idea by felix.schwarz@iospirit.com
flirc/hidapi@8d251c3

Fixes: #127, #236.
@mcuee
Copy link
Member

mcuee commented Aug 25, 2021

This should be fixed by #322. Tested with my Mac Mini M1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working macOS Related to macOS backend
Projects
None yet
Development

No branches or pull requests

4 participants