libusb: Allow building using non-POSIX runtime

[slouken]

This isn't a full Windows or macOS port, but abstracts the libusb threading code so that it can be easily ported to other platforms.

Fixes #581

[mcuee]

I think CMake files need to be updated as well.

[slouken]

I don't think so, it work as-is...?

[mcuee]

I don't think so, it work as-is...?

If you look at the CMake file, I think you need to add options to build both native Windows version and Windows libusb version, same for native macOS version and macOS libusb versoon. I think you can reuse HIDAPI_WITH_LIBUSB CMake option, or new HIDAPI_USING_SDL_RUNTIME option.

hidapi/CMakeLists.txt

Lines 41 to 44 in d3013f0

	if(CMAKE_SYSTEM_NAME MATCHES "Linux")
	option(HIDAPI_WITH_HIDRAW "Build HIDRAW-based implementation of HIDAPI" ON)
	option(HIDAPI_WITH_LIBUSB "Build LIBUSB-based implementation of HIDAPI" ON)
	endif()

    if(CMAKE_SYSTEM_NAME MATCHES "Linux")
        option(HIDAPI_WITH_HIDRAW "Build HIDRAW-based implementation of HIDAPI" ON)
        option(HIDAPI_WITH_LIBUSB "Build LIBUSB-based implementation of HIDAPI" ON)
    endif()

[slouken]

The macOS build can be made as a separate PR and isn't dependent on anything here.
The Windows build requires a Windows threads implementation. I used SDL, but that's not applicable here, so that would be a separate change that would need to implement the abstraction in this PR.

[mcuee]

No problem, you can create a new PR for macOS. Thanks.

[mcuee]

@slouken

BTW, just FYI, there is a bug with the libusb backend which is difficult to fix. It may not affect many devices though.

• HID reports of more than 64 bytes and libusb backend #274

[mcuee]

@slouken

I'd like to test this PR but unfortuantely I do not see how to build this PR without the changes to CMake or the legacy auto-tools script. Please help to explain how you build this PR. Thanks.

As of now, Windows build will only use the native Windows HID backend and not libusb.

[JoergAtGithub]

This change looks unrelated

[Youw]

My guess if that SDL has something like:
sdl_libusb_hid.c:

typedef struct
{
//...
} hid_device_thread_state;
// more thread_state defines
...
#define HIDAPI_USING_SDL_RUNTIME
#include "hidapi/libusb/hid.c"

am I correct on the approach this is used?

---

UPD: only now I checked #581, and my guess is right.

[Youw]

While libsdl-org/SDL@57b33ae makes perfect sense in the context of SDL project, this change is not at its best in the context of HIDAPI project.

First of all: the "SDL" term shouldn't be used among HIDAPI code.
Secondly: if we're introducing a threading abstraction - we better explicitly do so.

What I suggest:

• lets introduce explicit naming: compared to the original code, replace pthread_ part with hidapi_thread_, so we would get:
  • hidapi_thread_state_init
  • hidapi_thread_mutex_lock
  • etc.
• introduce a new source file: libusb/hidapi_thread_pthread.h with the includes/declarations/definitions for pthread implementation;
• to get keep backward compatibility and extensibility, inside libusb/hid.c :

  #ifndef HIDAPI_THREAD_MODEL_INCLUDE
  #define HIDAPI_THREAD_MODEL_INCLUDE "hidapi_thread_pthread.h"
  #endif
  #include HIDAPI_THREAD_MODEL_INCLUDE

  and use the functions below as suggested.

It would also greatly simplify making a native HIDAPI port of libusb backend on Windows.

UPD: I imaging it would require some (presumably small but) extra effort on SDL side to be ported to the upstream version of HIDAPI after the change with my suggestions accepted, but I have a strong feeling that it is worth doing anyway longterm.

[Youw]

I think CMake files need to be updated as well.

This change doesn't actually make it possible to build libusb backend of HIDAPI on Windows, but rather it is just a nesessary refactoring to allow replacing pthread with any other threading library.

I'm generally agree with the aproach, just have a few conserns regarding implementation (my prev comment).

[slouken]

I've updated the pull request and scoped it just to create the threading abstraction.

[Youw]

Perfect, just a few small comments.

[slouken]

Okay, I think this addresses everything. Please let me know if you have any other suggestions!

[Youw]

oh, I haven't though of this one, but great suggestion, thanks

[Youw]

Great work!

Now it should be relatively straight-forward to implement the native Windows port of libusb backend.
I'll keep this change in review for a few more days, so the comunity has a chance to catch up/review.

[mcuee]

Now it should be relatively straight-forward to implement the native Windows port of libusb backend.

That will be great!

[JoergAtGithub]

I wonder if it would make sense to seperate the two time functions into an own entity like HIDAPI_TIME_REF_INCLUDE.
This would allow to use a application specific time reference for future features like time stamping of incoming reports.
Other time references could be a the soundcard clock in an audio application or the network clock in a streaming application.

[Youw]

As of this moment (and for many years before that) the only place where this time related thing is used - is for thread_cond_timedwait.
As of this moment - this looks compact and we avoid additional abstraction/implementation.
If we need an abstraction in the future - we can easily introduce it later.