Support hidraw(4) on FreeBSD #597
Conversation
There was a problem hiding this comment.
Thank you for the PR. Please excuse all the noise coming from GitHub Actions. I have left a few questions, and would like to add another one, if I may: do I understand correctly that the PR is relying on the ioctl() in fido_hid_open() to place hidraw(4) in uhid(4) mode, as described in https://www.freebsd.org/cgi/man.cgi?query=hidraw&apropos=0&sektion=0&manpath=FreeBSD+13.1-RELEASE+and+Ports&arch=default&format=html#end?
-p.
|
Thank you for your good remarks.
That's correct. I'll add a couple of comments to the code, so the next one reading it will have those questions answered. |
|
Feel free to squash my commit on top of @grembo's (if they agree) before merging. |
There was a problem hiding this comment.
A few minor remarks on code and consistency.
Maybe you can give me a better understanding of your philosophy when it comes to comments - I tried to add them where things weren't clear to you in the review, but it seems like almost all of them were - excuse the word - butchered removed in the process. If that's the underlying philosophy, that's ok, I'm just curious if it wouldn't make sense to have something in there that explains what is going on, as the hidraw/uhid situation isn't necessarily obvious.
|
@LDVG @kongeo @martelletto None of my remarks above are blockers (even though I think leaving comments in that help implementors to understand the hidraw driver on FreeBSD and how to activate it for testing might help). If you prefer as few comments as possible, maybe they could be moved to the commit message. |
|
@martelletto @hselasky The latest commit should incorporate all changes we discussed, please let me know if this needs more refinement. Thanks for your input so far, much appreciated! |
|
Thank you. I will try to take a look over the weekend. |
|
LGTM. Sorry once again for all the GitHub Actions noise. |
Add support for the hidraw(4) driver introduced in FreeBSD 13. As this driver is lacking support for USB_GET_DEVICEINFO at the moment, fall back to an alternative to retrieve device information. Once hidraw(4) gains support for USB_GET_DEVICEINFO, its results will be used automatically.
martelletto
force-pushed
the
freebsd-hidraw
branch
from
adae45e
to
b854c75
Compare
|
rebased and squashed; no code change. GitHub Actions should be OK now. |
|
Merged; thank you! |
|
@martelletto Thank you for reviewing (and improving) the code. |
Add support for the hidraw(4) driver introduced in FreeBSD 13.
As this driver is lacking support for USB_GET_DEVICEINFO at the
moment, fall back to an alternative to retrieve device information.
Once hidraw(4) gains support for USB_GET_DEVICEINFO, its results
will be used automatically.