hidapi icon indicating copy to clipboard operation
hidapi copied to clipboard

hidapi 0.14.0 for Windows with C++-Builder 11

Open Stoccarda opened this issue 2 years ago • 13 comments

I have been using hid.c in a version from before September 2019 for 32- and 64-bit Windows programs. Compiler is CLANG in C++ Builder 11.3 (and earlier) IDE. hid.c was compiled and embedded into the main program, no dll. The main program had in most cases a GUI.

It worked for me, except for rising an exception when closing the program, and hence closing the HID device.

When compiling the current 0.14.0 several errors / warning occurred:


in hidapi_descriptor_reconstruct.h:

#if defined(MINGW32) || defined(CYGWIN) ... #else union { hid_pp_cap caps[]; hid_pp_link_collection_node LinkCollectionArray[]; }; #endif

[bcc32c error] hidapi_descriptor_reconstruct.h(220): type name requires a specifier or qualifier [bcc32c error] hidapi_descriptor_reconstruct.h(220): expected ';' at end of declaration list

When extending the first line to #if defined(MINGW32) || defined(CYGWIN) || defined(clang) it works. It also works with BORLANDC .


Compiler warning "[bcc64 Warnung] hidapi_hidsdi.h(43): redefinition of typedef 'PHIDP_PREPARSED_DATA' is a C11 feature"

typedef struct _HIDP_PREPARSED_DATA * PHIDP_PREPARSED_DATA;

was previously defined in hidapi_hidpi.h(39) Won't be the typedef in hidapi_hidpi.h(39) sufficient?


Second warning from hid.c in line 267: int printf_written = swprintf(msg, msg_len + 1, L"%.*ls: (0x%08X) %.*ls", (int)op_len, op, error_code, (int)system_err_len, system_err_buf);

[bcc64 Warnung] hid.c(267): incompatible integer to pointer conversion passing 'unsigned long' to parameter of type 'const wchar_t *' (aka 'const unsigned short *')


There is a linker error with the 32-bit compiler version: [ilink32 Fehler] Fatal: Invalid VIRDEF Fixup-Index in Modul 'hid.c'

It works when creating a 64-bit version.


When running a program

res = hid_get_input_report_length(handle);
ShowMessage((String)"input_report_length: " + res);

shows a length of zero.

res = hid_get_output_report_length(handle);
ShowMessage((String)"output_report_length: " + res);

returns the correct value.

The following code was used for it:

struct hid_device_ { HANDLE device_handle; BOOL blocking; USHORT output_report_length; size_t input_report_length; void *last_error_str; DWORD last_error_num; BOOL read_pending; char *read_buf; OVERLAPPED ol; };

int HID_API_EXPORT_CALL HID_API_CALL hid_get_input_report_length(hid_device *dev) { return dev->input_report_length; }

int HID_API_EXPORT_CALL HID_API_CALL hid_get_output_report_length(hid_device *dev) { return dev->output_report_length; }

Sending data to a HID device seems to work.


The exception when closing the program still exists.


Any hints welcome!

Stoccarda avatar Oct 19 '23 15:10 Stoccarda

I've never tried to compile HIDAPI with clang on Windows. PR with fixes are welcome. As for other issues - I have no specific siggestions, need to investigate carefully.

Youw avatar Oct 19 '23 15:10 Youw

When extending the first line to #if defined(MINGW32) || defined(CYGWIN) || defined(clang) it works. It also works with BORLANDC .

Which code path is used in case of BorlandC ? Is BorlandC using clang?

I think we should add clang-cl to our CI, than we should see such issues. Since is clang-cl.exe is command line compatible to the MSVC cl.exe, it should easy to set up.

JoergAtGithub avatar Oct 19 '23 15:10 JoergAtGithub

#if defined(MINGW32) || defined(CYGWIN) || defined(clang)

At first a correction: when copying the text, the underscores got lost due to wrong formatting. More correct: #if defined(__MINGW32__) || defined(__CYGWIN__) || defined(__clang__) The same for __BORLANDC__

From C++ Builder 10 Seattle on you had the choice between the classical Borland compiler and a CLANG based compiler. The 64-bit compiler (since XE4) was always Clang based. Current clang / LLVM version is 3.3 for both, 5.0 is also mentioned in the help. C++17 is supported.

Stoccarda avatar Oct 20 '23 07:10 Stoccarda

I think we need to invert the condition here, as the Microsoft compiler seems to be the exception and not the other way around. Let's use #ifndef _MSC_VER instead of #if defined(__MINGW32__) || defined(__CYGWIN__) || defined(__clang__) || defined(__BORLANDC__)

JoergAtGithub avatar Oct 21 '23 07:10 JoergAtGithub

@JoergAtGithub #ifndef _MSC_VER instead of #if defined(__MINGW32__) || defined(__CYGWIN__) works well and is surely a better solution than adding further defines.

In hidapi_hidsdi.h: typedef struct _HIDP_PREPARSED_DATA * PHIDP_PREPARSED_DATA; Can it be removed since the typedef is already in hidapi_hidpi.h which is included a few lines before?

In hid.c , line 267: int printf_written = swprintf(msg, msg_len + 1, L"%.*ls: (0x%08X) %.*ls", (int)op_len, op, error_code, (int)system_err_len, system_err_buf);

the compiler warns: [bcc64 Warnung] hid.c(267): incompatible integer to pointer conversion passing 'unsigned long' to parameter of type 'const wchar_t *' (aka 'const unsigned short *')

If swprintf has a size (msg_len + 1) as the second parameter, isn't it the C++ function? the C function is just 'int swprintf(wchar_t *buffer, const wchar_t *format[, argument, ...]);' without size Since there is #ifdef __cplusplus extern "C" { #endif in the beginning of the file, don't the functions need to be C and not C++ style? When compiling without the size, there is no warning.

Stoccarda avatar Oct 26 '23 15:10 Stoccarda

the C function is just 'int swprintf(wchar_t *buffer, const wchar_t *format[, argument, ...]);' without size

Do you have a reference where you've got it from? I have found at least 3 different sources stating otherwise: https://linux.die.net/man/3/swprintf https://cplusplus.com/reference/cwchar/swprintf/ https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/sprintf-sprintf-l-swprintf-swprintf-l-swprintf-l

Youw avatar Oct 26 '23 18:10 Youw

In hidapi_hidsdi.h: typedef struct _HIDP_PREPARSED_DATA * PHIDP_PREPARSED_DATA; Can it be removed since the typedef is already in hidapi_hidpi.h which is included a few lines before?

I searched for this, and it seems that the original hidsdi.h from Microsoft doesn't contain this definition, only hidpi.h does. Therefore it makes sense to remove it from hidapi_hidsdi.h.

JoergAtGithub avatar Oct 26 '23 19:10 JoergAtGithub

Closed by automation. @Stoccarda please confirm with latest master (and reopen is the issue(s) still there). Thanks.

Youw avatar Oct 26 '23 23:10 Youw

My PR #634 fixed only the first two problems.

JoergAtGithub avatar Oct 27 '23 05:10 JoergAtGithub

@Youw I found the different definition there: https://docwiki.embarcadero.com/RADStudio/Alexandria/en/Sprintf%2C_swprintf

I once installed CodeLite and found both declaration:

swprintf-CodeLite

There was also a similar discussion a few years ago: https://stackoverflow.com/questions/35072373/swprintf-declaration-mismatch

Stoccarda avatar Oct 27 '23 10:10 Stoccarda

This answer: https://stackoverflow.com/a/35072422/3697939 summarizes my oppinion on this.

Youw avatar Oct 27 '23 15:10 Youw

@Youw I can corfirm that the error/warning related to the 2 include files is solved with the new ones

The confusion with swprintf and the 32-bit linker error [ilink32 Fehler] Fatal: Invalid VIRDEF Fixup-Index in Modul 'hid.c' still exists and the hid_get_input_report_length still returns 0 while input_report_length is the correct 65 in my program.

Stoccarda avatar Oct 27 '23 16:10 Stoccarda

The swprintf issue seems to be a compiler bug. It works for me with the temporary workaround

#include <stdio.h> #include <stdlib.h> #include <string.h>

#if !defined(__cplusplus) && defined(BORLANDC) #undef swprintf #define swprintf snwprintf #endif

The other problems still exist.

Stoccarda avatar Oct 31 '23 15:10 Stoccarda