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

Fix USB enumeration-related compile signedness warnings #21

Closed
wants to merge 1 commit into from
Closed

Fix USB enumeration-related compile signedness warnings #21

wants to merge 1 commit into from

Conversation

brainstorm
Copy link

@brainstorm brainstorm commented Dec 17, 2020

Fixes compilation warnings of the sort (mentioned on linklayer/cantact#3)... quick on-the-web type of patch/PR so tabs and spaces are garbled, but you get the point :-S

arm-none-eabi-gcc -DCORE_M0 -D HSI48_VALUE=48000000 -D HSE_VALUE=16000000 -D CANTACT_BUILD_NUMBER=0 -DSTM32F042x6 -IDrivers/CMSIS/Include -IDrivers/CMSIS/Device/ST/STM32F0xx/Include -IDrivers/STM32F0xx_HAL_Driver/Inc -IInc -IMiddlewares/ST/STM32_USB_Device_Library/Core/Inc -IMiddlewares/ST/STM32_USB_Device_Library/Class/CDC/Inc  -mcpu=cortex-m0 -mthumb -Wall -g -ffunction-sections -fdata-sections -Os -Os -c -o build/usbd_desc.o Src/usbd_desc.c
Src/usbd_desc.c: In function 'USBD_FS_ManufacturerStrDescriptor':
Src/usbd_desc.c:70:34: warning: pointer targets in passing argument 1 of 'USBD_GetString' differ in signedness [-Wpointer-sign]
   70 | #define USBD_MANUFACTURER_STRING "CANtact"
      |                                  ^~~~~~~~~
      |                                  |
      |                                  char *
Src/usbd_desc.c:227:19: note: in expansion of macro 'USBD_MANUFACTURER_STRING'
  227 |   USBD_GetString (USBD_MANUFACTURER_STRING, USBD_StrDesc, length);
      |                   ^~~~~~~~~~~~~~~~~~~~~~~~
In file included from Middlewares/ST/STM32_USB_Device_Library/Core/Inc/usbd_core.h:36,
                 from Src/usbd_desc.c:37:
Middlewares/ST/STM32_USB_Device_Library/Core/Inc/usbd_ctlreq.h:90:39: note: expected 'uint8_t *' {aka 'unsigned char *'} but argument is of type 'char *'
   90 | void USBD_GetString         (uint8_t *desc, uint8_t *unicode, uint16_t *len);
      |                              ~~~~~~~~~^~~~

Fixes compilation warnings of the sort:

```C
arm-none-eabi-gcc -DCORE_M0 -D HSI48_VALUE=48000000 -D HSE_VALUE=16000000 -D CANTACT_BUILD_NUMBER=0 -DSTM32F042x6 -IDrivers/CMSIS/Include -IDrivers/CMSIS/Device/ST/STM32F0xx/Include -IDrivers/STM32F0xx_HAL_Driver/Inc -IInc -IMiddlewares/ST/STM32_USB_Device_Library/Core/Inc -IMiddlewares/ST/STM32_USB_Device_Library/Class/CDC/Inc  -mcpu=cortex-m0 -mthumb -Wall -g -ffunction-sections -fdata-sections -Os -Os -c -o build/usbd_desc.o Src/usbd_desc.c
Src/usbd_desc.c: In function 'USBD_FS_ManufacturerStrDescriptor':
Src/usbd_desc.c:70:34: warning: pointer targets in passing argument 1 of 'USBD_GetString' differ in signedness [-Wpointer-sign]
   70 | #define USBD_MANUFACTURER_STRING "CANtact"
      |                                  ^~~~~~~~~
      |                                  |
      |                                  char *
Src/usbd_desc.c:227:19: note: in expansion of macro 'USBD_MANUFACTURER_STRING'
  227 |   USBD_GetString (USBD_MANUFACTURER_STRING, USBD_StrDesc, length);
      |                   ^~~~~~~~~~~~~~~~~~~~~~~~
In file included from Middlewares/ST/STM32_USB_Device_Library/Core/Inc/usbd_core.h:36,
                 from Src/usbd_desc.c:37:
Middlewares/ST/STM32_USB_Device_Library/Core/Inc/usbd_ctlreq.h:90:39: note: expected 'uint8_t *' {aka 'unsigned char *'} but argument is of type 'char *'
   90 | void USBD_GetString         (uint8_t *desc, uint8_t *unicode, uint16_t *len);
      |                              ~~~~~~~~~^~~~
```
@trollcop
Copy link

It would be nice if your changed did not introduce unnecessary whitespace modifications to the files.
It's difficult to tell what was changed or not.

@brainstorm brainstorm closed this Jan 19, 2021
@brainstorm
Copy link
Author

Agreed, it was a quick on-my-phone type of pullrequest, but this repo has not seen a merge since 2017, so closing.

It's unlikely it'll be acted upon, happy to be convinced otherwise by @ericevenchick or whoever maintains this repo... also, feel free to add my changes to your pullrequest in #20 or open another one yourself @trollcop ;)

@normaldotcom
Copy link

FYI I do have a fork of this repo with an updated HAL library with better performance on heavily loaded busses/etc. I haven't merged in hardware filtering yet, and I think I may have broken support for devices with external oscillators (if I remember correctly). I'm still doing active development on this fork.

https://github.com/normaldotcom/cantact-fw

@brainstorm
Copy link
Author

Oh, nice work! While you are here, I have this burning question, let's see if you can address it since I'm genuinely curious:

linklayer/cantact#3 (comment)

TL;DR: The author, @ericevenchick, claimed that SLCAN is "generally not good" and that I should use gs_usb and another firmware entirely (candleLight) instead... what's your take on that? Is there really a perf difference and do you reckon you've corrected it in your fork?

Cheers and kudos for your work @normaldotcom!

@normaldotcom
Copy link

I would agree... slcan is definitely not the best option if you're running on Linux. Using the gs_usb / candlelight firmware gives better performance at high bitrates/busloads and eliminates the overhead of slcand. And it's just nice to plug it in and "can0" shows up immediately. I always recommend gs_usb when running on Linux with a reasonably modern kernel, the fact that you don't need to run slcand is enough motivation for me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants