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

HID regression in 2.0.2 due to weak definitions #296

Open
delan opened this issue Apr 30, 2023 · 3 comments
Open

HID regression in 2.0.2 due to weak definitions #296

delan opened this issue Apr 30, 2023 · 3 comments
Labels
Bug Something isn't working

Comments

@delan
Copy link

delan commented Apr 30, 2023

Operating System

Windows 10

Arduino IDE version

PlatformIO 6.1.6

Board

pico

ArduinoCore version

earlephilhower/[email protected]

TinyUSB Library version

2.0.2+

Sketch as ATTACHED TXT

delan/usb3sun@04ba656

Compiled Log as ATTACHED TXT

n/a

What happened ?

When upgrading the library from 2.0.1 to 2.0.2, tuh_mount_cb is called, but tuh_hid_mount_cb and tuh_hid_report_received_cb are no longer called.

How to reproduce ?

  1. Get two pico boards, and flash picoprobe-cmsis-v1.0.2 to one of them
  2. Connect GND to picoprobe GND, SWCLK to picoprobe GP2, SWDIO to picoprobe GP3, GP1 to picoprobe GP4, GP0 to picoprobe GP5, VBUS to picoprobe VBUS, GP4 to USB-A port D+, GP5 to USB-A port D-
  3. Check out delan/usb3sun@04ba656 and git apply repro.patch
  4. Follow the instructions in doc/firmware.md except the step about tinyusb2.patch (the patch is not needed when CFG_TUSB_DEBUG=3, and the bug report is better if the library is unmodified)
  5. Connect picoprobe to computer, and connect to the picoprobe’s cdc serial port
  6. Build and upload the firmware (pio run -t upload)
  7. Connect a keyboard or mouse to the USB-A port

Debug Log

Screenshots

No response

@delan delan added the Bug Something isn't working label Apr 30, 2023
@JonnyHaystack
Copy link

JonnyHaystack commented Jan 4, 2024

Having the same issue here with native USB host on RP2040. tuh_mount_cb() works as in the example, but the HID callbacks don't work because it seems to always link the weak definitions from Adafruit_USBH_Host.cpp instead of being overridden by my own strong definitions.

For the record I have confirmed that this is what is happening by using breakpoints on Picoprobe debugger. I have also confirmed that commenting out the weak definitions fixes the problem.

I don't know what it is about these particular weak definitions that is confusing the linker, but I do wonder is there even any reason to have them duplicated in Adafruit_USBH_Host.cpp when they already exist in hid_host.h?

@delan delan changed the title HID regression in 2.0.2 when using pico pio usb HID regression in 2.0.2 due to weak definitions Mar 4, 2024
@delan
Copy link
Author

delan commented Mar 4, 2024

Thanks @JonnyHaystack! 3.1.0 is still affected, and I can confirm removing those definitions fixes the problem for me.

@hathach, it looks like the addition of TU_ATTR_WEAK in hid_host.h in hathach/tinyusb#1968 was the breaking change, so if we can’t remove those empty definitions in Adafruit_USBH_Host.cpp for some reason, then we need to remove TU_ATTR_WEAK from the tuh_hid_mount_cb declaration (and tuh_hid_umount_cb and tuh_hid_report_received_cb).

(This probably explains why tuh_hid_umount_cb has never worked for me…)

@pprice
Copy link

pprice commented Jun 9, 2024

Can confirm this with 3.1.5 + PlatformIO + rp2020 earlephilhower core; removal of TU_ATTR_WEAK from tuh_hid_mount_cb and tuh_hid_umount_cb definitions in hid_host.h , i can see hid callbacks being raised. Workarounds appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants