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

hidapi 0.14.0 for Windows with C++-Builder 11 #630

Open
Stoccarda opened this issue Oct 19, 2023 · 13 comments · Fixed by #634
Open

hidapi 0.14.0 for Windows with C++-Builder 11 #630

Stoccarda opened this issue Oct 19, 2023 · 13 comments · Fixed by #634
Labels
build system/CI Anything related to building the project or running on CI Windows Related to Windows backend

Comments

@Stoccarda
Copy link

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!

@Youw
Copy link
Member

Youw commented Oct 19, 2023

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.

@JoergAtGithub
Copy link
Contributor

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.

@Stoccarda
Copy link
Author

#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.

@JoergAtGithub
Copy link
Contributor

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__)

@mcuee mcuee added build system/CI Anything related to building the project or running on CI Windows Related to Windows backend labels Oct 23, 2023
@Stoccarda
Copy link
Author

Stoccarda commented Oct 26, 2023

@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.

@Youw
Copy link
Member

Youw commented Oct 26, 2023

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

@JoergAtGithub
Copy link
Contributor

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.

@Youw Youw closed this as completed in #634 Oct 26, 2023
Youw pushed a commit that referenced this issue Oct 26, 2023
* Invert preprocessor condition, as the Microsoft compiler seems to be the special case - not the other way around

* Removed redefinition of typedef 'PHIDP_PREPARSED_DATA' in hidapi_hidsdi.h to match the original hidsdi.h

As reported in #630
@Youw
Copy link
Member

Youw commented Oct 26, 2023

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

@JoergAtGithub
Copy link
Contributor

My PR #634 fixed only the first two problems.

@Youw Youw reopened this Oct 27, 2023
@Stoccarda
Copy link
Author

@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

@Youw
Copy link
Member

Youw commented Oct 27, 2023

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

@Stoccarda
Copy link
Author

@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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build system/CI Anything related to building the project or running on CI Windows Related to Windows backend
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants