search

adafruit / Adafruit_TinyUSB_Arduino

Arduino library for TinyUSB
MIT License
489 stars 130 forks source link

HID regression in 2.0.2 due to weak definitions #296

Open delan opened 1 year ago

delan commented 1 year ago

Operating System

Windows 10

Arduino IDE version

PlatformIO 6.1.6

Board

pico

ArduinoCore version

earlephilhower/arduino-pico@3.1.1

TinyUSB Library version

2.0.2+

Sketch as ATTACHED TXT

delan/usb3sun@04ba656717d7a04db0a5b68266c1fe9469e5f6f5

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@04ba656717d7a04db0a5b68266c1fe9469e5f6f5 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

JonnyHaystack commented 10 months ago

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 commented 8 months ago

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 commented 5 months ago

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.