hidapi 0.14.0 for Windows with C++-Builder 11
Stoccarda opened this issue · 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!
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.
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.
#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.
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
#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.
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
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.
Closed by automation.
@Stoccarda please confirm with latest master (and reopen is the issue(s) still there). Thanks.
My PR #634 fixed only the first two problems.
@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:
There was also a similar discussion a few years ago:
https://stackoverflow.com/questions/35072373/swprintf-declaration-mismatch
This answer: https://stackoverflow.com/a/35072422/3697939 summarizes my oppinion on this.
@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.
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.