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

Initial NetBSD/UHID native backend implementation #612

Merged
merged 3 commits into from Sep 4, 2023
Merged

Initial NetBSD/UHID native backend implementation #612

merged 3 commits into from Sep 4, 2023

Conversation

ghost
Copy link

@ghost ghost commented Aug 17, 2023

This is the culmination of the research I did for uhid on NetBSD. It should be as complete as a native backend can be given the limitations of NetBSD's kernel and uhid as a whole. See related issue, #608.

@mcuee mcuee added enhancement New feature or request bsd FreeBSD, NetBSD, OpenBSD, etc labels Aug 17, 2023
CMakeLists.txt Show resolved Hide resolved
@ghost
Copy link
Author

ghost commented Aug 18, 2023

Minor code cleanup. I noticed some improvements I could make when reusing some of the code for another project of mine.

@mcuee
Copy link
Member

mcuee commented Aug 21, 2023

Nice, the first simple test seems to work fine. I need to modify hidapitester to test this more.

I am using NetBSD 9.3 VM running under Proxmox and I attach a generic HID device to the VM.

localhost$ cmake -B build -D HIDAPI_BUILD_HIDTEST=1
-- The C compiler identification is GNU 7.5.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- hidapi: v0.14.0
-- Looking for pthread.h
-- Looking for pthread.h - found
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Failed
-- Looking for pthread_create in pthreads
-- Looking for pthread_create in pthreads - not found
-- Looking for pthread_create in pthread
-- Looking for pthread_create in pthread - found
-- Found Threads: TRUE
-- Found PkgConfig: /usr/pkg/bin/pkg-config (found version "1.9.4")
-- Checking for module 'libusb-1.0>=1.0.9'
--   Found libusb-1.0, version 1.0.24
-- Check for Iconv
-- Performing Test Iconv_IS_BUILT_IN
-- Performing Test Iconv_IS_BUILT_IN - Success
-- Found Iconv: built in to C library
-- Performing Test HIDAPI_ICONV_CONST
-- Performing Test HIDAPI_ICONV_CONST - Success
-- Building hidtest
-- Configuring done
-- Generating done
-- Build files have been written to: /home/mcuee/build/hidapi_netbsd/build
localhost$ cmake --build build
[ 16%] Building C object src/netbsd/CMakeFiles/hidapi_netbsd.dir/hid.c.o
[ 33%] Linking C shared library libhidapi-netbsd.so
[ 33%] Built target hidapi_netbsd
[ 50%] Building C object src/libusb/CMakeFiles/hidapi_libusb.dir/hid.c.o
[ 66%] Linking C shared library libhidapi-libusb.so
[ 66%] Built target hidapi_libusb
[ 83%] Building C object hidtest/CMakeFiles/hidtest.dir/test.c.o
[100%] Linking C executable hidtest
[100%] Built target hidtest
localhost$ sudo ./build/hidtest/hidtest
Password:
hidapi test/example tool. Compiled with hidapi version 0.14.0, runtime version 0.14.0.
Compile-time version matches runtime version of hidapi.

Device Found
  type: 0925 7001
  path: uhidev0
  serial_number:
  Manufacturer: Microchip Technology Inc.
  Product:      Generic HID
  Release:      1
  Interface:    -1
  Usage (page): 0x1 (0xffa0)
  Bus type: 1 (USB)

  Report Descriptor: (47 bytes)
0x06, 0xa0, 0xff, 0x09, 0x01, 0xa1, 0x01, 0x09, 0x03, 0x15,
0x00, 0x26, 0xff, 0x00, 0x75, 0x08, 0x95, 0x3f, 0x81, 0x02,
0x09, 0x04, 0x15, 0x00, 0x26, 0xff, 0x00, 0x75, 0x08, 0x95,
0x3f, 0x91, 0x02, 0x09, 0x05, 0x15, 0x00, 0x26, 0xff, 0x00,
0x75, 0x08, 0x95, 0x3f, 0xb1, 0x02, 0xc0,
unable to open device

localhost$ uname -a
NetBSD localhost 9.3 NetBSD 9.3 (GENERIC) #0: Thu Aug  4 15:30:37 UTC 2022  [email protected]:/usr/src/sys/arch/amd64/compile/GENERIC amd64

localhost$ sudo usbdevs
addr 1: UHCI root hub, NetBSD
addr 0: xHCI root hub, NetBSD
addr 0: xHCI root hub, NetBSD
 addr 1: Generic HID, Microchip Technology Inc.

@ghost
Copy link
Author

ghost commented Aug 22, 2023

Nice, the first simple test seems to work fine. I need to modify hidapitester to test this more.

I am using NetBSD 9.3 VM running under Proxmox and I attach a generic HID device to the VM.

Feel free to refine it further but I have reached the limit of what I can see to do. I am considering writing a FreeBSD backend after NetBSD has been merged. It is probably the last native BSD backend that has enough kernel API functionality to work with the current library abstraction. I may end up creating two versions for FreeBSD due to it having the option of using a hidraw compatibility layer as well as the older uhid one.

@mcuee
Copy link
Member

mcuee commented Aug 22, 2023

I am considering writing a FreeBSD backend after NetBSD has been merged. It is probably the last native BSD backend that has enough kernel API functionality to work with the current library abstraction. I may end up creating two versions for FreeBSD due to it having the option of using a hidraw compatibility layer as well as the older uhid one.

That would be wonderful. I tend to think the 1hidraw` backend is the way to go for FreeBSD.

@ghost
Copy link
Author

ghost commented Aug 23, 2023

That would be wonderful. I tend to think the 1hidraw` backend is the way to go for FreeBSD.

Preliminary investigation suggests this has its own problems due to how new this backend is. It isn't enabled by default on fresh installations. Of the two, uhid and hidraw, only one can be attached to any particular device.

uhid is the default if hw.usb.usbhid.enable is set to 0. hidraw can only be used if hw.usb.usbhid.enable is set to 1 and the hidraw module is loaded explicitly. If hw.usb.usbhid.enable is set to 1 but hidraw is not available, there won't be anything setup to provide userspace HID access.

Any solution would probably have to be prepared to use whichever API the FreeBSD environment is configured to use as I don't think it would be a good practice to forcefully enable hidraw against the wishes of the end user.

@Youw
Copy link
Member

Youw commented Aug 23, 2023

I don't think it would be a good practice to forcefully enable hidraw against the wishes of the end user.

I agree. I'd say that even if we will get the hidraw backend implementation for FreeBSD - that is absolutely fine that it will be functional only if user explicitly enables the hidraw subsystem/driver in his environmnet on his own.

@mcuee
Copy link
Member

mcuee commented Aug 24, 2023

That would be wonderful. I tend to think the 1hidraw` backend is the way to go for FreeBSD.

Preliminary investigation suggests this has its own problems due to how new this backend is. It isn't enabled by default on fresh installations. Of the two, uhid and hidraw, only one can be attached to any particular device.

uhid is the default if hw.usb.usbhid.enable is set to 0. hidraw can only be used if hw.usb.usbhid.enable is set to 1 and the hidraw module is loaded explicitly. If hw.usb.usbhid.enable is set to 1 but hidraw is not available, there won't be anything setup to provide userspace HID access.

Any solution would probably have to be prepared to use whichever API the FreeBSD environment is configured to use as I don't think it would be a good practice to forcefully enable hidraw against the wishes of the end user.

Thanks for the analysis.

However, for the FreeBSD OS, there is already a libusb backend which will be the default as it already works fine (other than common libusb issues: https://github.com/libusb/hidapi/issues?q=is%3Aopen+is%3Aissue+label%3Alibusb). So libusb backend will be the preferred backend in the foreseeable future.

I believe ugen driver is also attached to generic USB HID device along with uhid (or hidraw). That is why it works.

I see this as similar to Linux side last time -- libusb was the preferred and hidraw was secondary as there was too many limitations previously. Now hidraw is the preferred.

But of course you can develop uhid backend first as it is more common for now. However, I tend to believe hidraw may be the future for FreeBSD as well (and maybe NetBSD/OpenBSD).

@mcuee
Copy link
Member

mcuee commented Aug 26, 2023

@Youw

This PR does not support the existing auto-tools build system. Do we want to keep the auto-tools support?

@ghost
Copy link
Author

ghost commented Aug 26, 2023

This PR does not support the existing auto-tools build system. Do we want to keep the auto-tools support?

I left it out because I found comments saying that it was deprecated so I assumed it was going to be removed in the near future.

@Youw
Copy link
Member

Youw commented Aug 26, 2023

Do we want to keep the auto-tools support?

No.

I left it out because I found comments saying that it was deprecated so I assumed it was going to be removed in the near future.

This is exactly why.


BTW: I've reviewed all CMake changes - very consistent with what we already have. Very nice, thanks.

Copy link
Member

@Youw Youw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall very consistent implementation.
I didn't check all of the implementation/logic (specially what consernes the uhid API/usage) but overall looks very good.
Just a few comments/improvements to consider.

netbsd/hid.c Show resolved Hide resolved
netbsd/hid.c Outdated Show resolved Hide resolved
netbsd/hid.c Outdated Show resolved Hide resolved
netbsd/hid.c Outdated Show resolved Hide resolved
netbsd/hid.c Outdated Show resolved Hide resolved
netbsd/hid.c Outdated Show resolved Hide resolved
netbsd/hid.c Outdated Show resolved Hide resolved
netbsd/hid.c Show resolved Hide resolved
netbsd/hid.c Show resolved Hide resolved
netbsd/hid.c Show resolved Hide resolved
This also modifies an internal routine for walking the device tree
to accept a function pointer for the string comparison so it can
be reused in more places than its originally intended one.
@ghost
Copy link
Author

ghost commented Aug 27, 2023

Re-based on latest master branch.

@ghost
Copy link
Author

ghost commented Aug 27, 2023

Ok. That should handle the requested changes. Anything else?

@Youw
Copy link
Member

Youw commented Aug 27, 2023

just one last comment

@ghost
Copy link
Author

ghost commented Aug 27, 2023

There. That comment should cover it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bsd FreeBSD, NetBSD, OpenBSD, etc enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants