From e81cc975ac23ad2dcfdbbe364fd5fc178e7f3017 Mon Sep 17 00:00:00 2001 From: ardnew Date: Wed, 22 Nov 2023 22:46:42 -0600 Subject: [PATCH] USB: add pure specifiers and emit vtable Issue #130 correctly identifies a newly-added method as pure virtual and fixes the declaration. However, for some reason it didn't address all of the other virtual methods in that same class (`PluggableUSBModule`) that do not define a default implementation. The only virtual method that has a default implementation is provided inline in the class interface: ```c++ virtual void callback_reset() {}; ``` These issues combined prevent the compiler from being able to emit a vtable for the `PluggableUSBModule` class, thus preventing users from correctly subclassing it or any one of its derived classes such as `USBCDC`, `USBHID`, `USBMIDI`, etc. Refer to the following answer on StackOverflow for a detailed explanation of the issue: https://stackoverflow.com/a/57504289/1054397 This PR adds the pure specifier (`= 0`) to all of the virtual methods in this class that do not have a default implementation. It also moves the default empty definition of `virtual void callback_reset()` to the class definition in `USB/PluggableUSBDevice.cpp` so that this class complies completely with the criteria for emitting a vtable. > #### Note > > The error that I was encountering prior to these changes was pretty > cryptic (from PlatformIO): > > ```txt > .pio/build/hid/src/target.cpp.o: In function `foo()': > USBHID/src/PluggableUSBHID.h:53: undefined reference to > `vtable for arduino::internal::PluggableUSBModule' > .pio/build/hid/src/target.cpp.o: In function `foo()': > foo.hpp:100: undefined reference to > `vtable for arduino::internal::PluggableUSBModule' > collect2: error: ld returned 1 exit status > *** [.pio/build/hid/firmware.elf] Error 1 > ``` > > Even stranger, the error would only be generated with a debug build; > i.e., the only difference in command-line arguments was the additional > CFLAGS of `-Og -g2 -ggdb2`. Without the debug flags, my project was > building without error. > > With the changes in this PR, my project now builds with and without > these additional debug flags. Further verification was performed by > testing the example sketches `Keyboard`, `KeyboardRaw`, and `Mouse` > from library `USBHID` as well as using the core `Serial` object for > ordinary USB serial I/O (`USBCDC`). --- cores/arduino/USB/PluggableUSBDevice.cpp | 4 ++++ cores/arduino/USB/PluggableUSBDevice.h | 28 ++++++++++++++++-------- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/cores/arduino/USB/PluggableUSBDevice.cpp b/cores/arduino/USB/PluggableUSBDevice.cpp index 288f940c6..c1449610d 100644 --- a/cores/arduino/USB/PluggableUSBDevice.cpp +++ b/cores/arduino/USB/PluggableUSBDevice.cpp @@ -60,6 +60,10 @@ uint32_t arduino::internal::PluggableUSBModule::write_finish(usb_ep_t endpoint) return PluggableUSBD().write_finish(endpoint); } +void arduino::internal::PluggableUSBModule::callback_reset() +{ +} + arduino::PluggableUSBDevice::PluggableUSBDevice(uint16_t vendor_id, uint16_t product_id) : USBDevice(get_usb_phy(), vendor_id, product_id, 1 << 8) { diff --git a/cores/arduino/USB/PluggableUSBDevice.h b/cores/arduino/USB/PluggableUSBDevice.h index a92bb9693..d766bd80b 100644 --- a/cores/arduino/USB/PluggableUSBDevice.h +++ b/cores/arduino/USB/PluggableUSBDevice.h @@ -38,6 +38,8 @@ class PluggableUSBModule { PluggableUSBModule(uint8_t numIfs) : numInterfaces(numIfs) { } + virtual ~PluggableUSBModule() + { } void lock(); void unlock(); void assert_locked(); @@ -49,17 +51,25 @@ class PluggableUSBModule { uint32_t write_finish(usb_ep_t endpoint); protected: - virtual const uint8_t *configuration_desc(uint8_t index); - virtual void callback_reset() {}; - virtual void callback_state_change(USBDevice::DeviceState new_state); - virtual uint32_t callback_request(const USBDevice::setup_packet_t *setup, USBDevice::RequestResult *result, uint8_t** data); - virtual bool callback_request_xfer_done(const USBDevice::setup_packet_t *setup, bool aborted); - virtual bool callback_set_configuration(uint8_t configuration); - virtual void callback_set_interface(uint16_t interface, uint8_t alternate); - virtual void init(EndpointResolver& resolver); - virtual const uint8_t *string_iinterface_desc(); + virtual const uint8_t *configuration_desc(uint8_t index) = 0; + virtual void callback_state_change(USBDevice::DeviceState new_state) = 0; + virtual uint32_t callback_request(const USBDevice::setup_packet_t *setup, USBDevice::RequestResult *result, uint8_t** data) = 0; + virtual bool callback_request_xfer_done(const USBDevice::setup_packet_t *setup, bool aborted) = 0; + virtual bool callback_set_configuration(uint8_t configuration) = 0; + virtual void callback_set_interface(uint16_t interface, uint8_t alternate) = 0; + virtual void init(EndpointResolver& resolver) = 0; + virtual const uint8_t *string_iinterface_desc() = 0; virtual uint8_t getProductVersion() = 0; + /** + * Non-pure virtual method with a non-inline definition + * + * @note This satisfies all criteria necessary for the compiler to emit a + * vtable, allowing users to safely subclass any of the derived "pluggable" + * USB device classes (USBCDC, USBHID, USBMIDI, etc). + */ + virtual void callback_reset(); + uint8_t pluggedInterface; const uint8_t numInterfaces;