drivers: udc: eliminate C++ errors by casting void* to the target type

[benedekkupper]

When this header is included in a C++ source file, compilation fails due to invalid syntax. It can be converted to a warning with -fpermissive, but the long term solution is to just perform the missing casts as done here.

[github-actions]

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

[benedekkupper]

@jfischer-no is there anything to be done to move this PR forward?

[jfischer-no]

Here in project, we are using C language. Implicit conversion to a pointer to void is fine and vice versa.
What this PR is trying to fix?

[benedekkupper]

It may be that the bugfix in the branch name is a misnomer, since the Zephyr codebase itself builds without issues. However, Zephyr claims to support applications written in both C and C++. This PR is a step to reach this claim in effect. It has absolutely no adverse effects when compiled into C, but this change is vital to allow compilation into C++.

[kartben]

@jfischer-no please revisit

[henrikbrixandersen]

Here in project, we are using C language. Implicit conversion to a pointer to void is fine and vice versa.

But we do support out-of-tree applications written in C++, where implicit pointer conversion leads to a warning.

[kartben]

@jfischer-no please revisit. There are many drivers in-tree that do this already so I find it really hard to justify why this would be an issue. Thanks!

[henrikbrixandersen]

@jfischer-no please revisit. There are many drivers in-tree that do this already so I find it really hard to justify why this would be an issue. Thanks!

We even have a dedicated test for including device driver headers from C++: https://github.com/zephyrproject-rtos/zephyr/blob/main/tests/lib/cpp/cxx/src/main.cpp

[jfischer-no]

This is UDC driver API, for the drivers written in C, for the USB device stack written in C, you have to compile it with C compiler not C++. There is no need for this type conversion in C!

[carlescufi]

@benedekkupper can you expand on your use case to try and figure out what the issue is here? Are you trying to write a UDC driver in C++, or are you including the udc.h header in a C++ file? or is it being auto-included making your build fail?

The reason I ask is that I don't see this file included in any public header file that you'd want to include from the application, nor it seems to be included by any .c file outside the core of the USB stack.

[benedekkupper]

I am using the UDC driver (and zephyr itself) as one of the supported platforms of an alternative USB device stack (written in C++).

You might be asking, why not using zephyr's own USB device stack instead?

1. Our project (UHK-80) started 2 years ago, when zephyr's USB device stack was nowhere yet, we needed something working sooner.
2. C++ allows for a compile-time HID report descriptor parser, so with this solution the USB endpoint max packet sizes are calculated from the report descriptor directly, providing a single source of truth. (It also provides the number of BLE HOGP attributes needed in compile-time, making GATT characteristic and UUID pools unnecessary - but that's nrf sdk territory.)

And now a question from my side: why put so much effort into such a well-defined and well-designed separation between the USB device stack and the UDC driver, if you don't let them be used on their own?

[carlescufi]

I am using the UDC driver (and zephyr itself) as one of the supported platforms of an alternative USB device stack (written in C++).

You might be asking, why not using zephyr's own USB device stack instead?

1. Our project (UHK-80) started 2 years ago, when zephyr's USB device stack was nowhere yet, we needed something working sooner.
2. C++ allows for a compile-time HID report descriptor parser, so with this solution the USB endpoint max packet sizes are calculated from the report descriptor directly, providing a single source of truth. (It also provides the number of BLE HOGP attributes needed in compile-time, making GATT characteristic and UUID pools unnecessary - but that's nrf sdk territory.)

And now a question from my side: why put so much effort into such a well-defined and well-designed separation between the USB device stack and the UDC driver, if you don't let them be used on their own?

Thanks @benedekkupper! This explains it very clearly. I am very much in favor of this change following your explanation. This is a +1 from me.

[jfischer-no]

I am using the UDC driver (and zephyr itself) as one of the supported platforms of an alternative USB device stack (written in C++).

You might be asking, why not using zephyr's own USB device stack instead?

1. Our project (UHK-80) started 2 years ago, when zephyr's USB device stack was nowhere yet, we needed something working sooner.

2. C++ allows for a compile-time HID report descriptor parser, so with this solution the USB endpoint max packet sizes are calculated from the report descriptor directly, providing a single source of truth. (It also provides the number of BLE HOGP attributes needed in compile-time, making GATT characteristic and UUID pools unnecessary - but that's nrf sdk territory.)

(1) there was a USB device stack there more than two years ago, (2) there is no advantage to doing this for USB transfers.

And now a question from my side: why put so much effort into such a well-defined and well-designed separation between the USB device stack and the UDC driver, if you don't let them be used on their own?

Certainly not for third-party layers, but to enforce consistent behavior and maintainability of the drivers and stack. Unfortunately, there is no clear separation between user and internal headers/interfaces in Zephyr, such as include/zephyr/uapi, which would make it clearer which interfaces are for users.

You provide no information as to why you cannot fix this in your stack where you interface with the UDC API.

[benedekkupper]

(1) there was a USB device stack there more than two years ago

Which didn't have the necessary feature-set that we needed, but I shouldn't need to explain this any more than you did when creating the USB device_next stack.

(2) there is no advantage to doing this for USB transfers

You are confused, the HID report sizes are calculated and used to populate the USB max packet size fields of the endpoint descriptors. Much less error-prone than having an independent definition of these descriptors, and having to remember to keep the two things in sync.

You provide no information as to why you cannot fix this in your stack where you interface with the UDC API.

The issue that this patch is trying to fix is syntactic, I have no control over the C++ syntax rules. I can't not include this C header, because the header provides the UDC API types and functions.

[yperess]

I run into these issues all the time. If it's in a header we need to make it compatible with C++ which means cast it.

[jfischer-no]

(1) there was a USB device stack there more than two years ago

Which didn't have the necessary feature-set that we needed, but I shouldn't need to explain this any more than you did when creating the USB device_next stack.

(2) there is no advantage to doing this for USB transfers

You are confused, the HID report sizes are calculated and used to populate the USB max packet size fields of the endpoint descriptors. Much less error-prone than having an independent definition of these descriptors, and having to remember to keep the two things in sync.

I am not, especially on nRF USB the maximum allowable interrupt data payload size is 64 bytes or less, so there is no real benefit of using smaller endpoint MPS and no bandwidth issues.

You provide no information as to why you cannot fix this in your stack where you interface with the UDC API.

The issue that this patch is trying to fix is syntactic, I have no control over the C++ syntax rules. I can't not include this C header, because the header provides the UDC API types and functions.

You can add your own header as interface to the UDC API and do there what ever you want.

Also you commit message states:
"It would be nice if C warning was raised for these cases,
so they can be caught faster."
Implicit void pointer conversions are just fine in C and are widely used in Zephyr tree, because we write the code in C not c++. Please rework you commit message to reflect the issue you are facing, describing that the implicit pointer conversion in c++ is not possible.

[benedekkupper]

Please rework you commit message to reflect the issue you are facing, describing that the implicit pointer conversion in c++ is not possible.

Done (I also rebased the commit), please review.

[jfischer-no]

Okay, but in general C++ limitations should not weaken our interfaces.

[kartben]

@benedekkupper thanks so much for your patience! Glad this is now merged :)