Skip to content

Commit e81cc97

Browse files
committed
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`).
1 parent 7b95100 commit e81cc97

File tree

2 files changed

+23
-9
lines changed

2 files changed

+23
-9
lines changed

Diff for: cores/arduino/USB/PluggableUSBDevice.cpp

+4
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,10 @@ uint32_t arduino::internal::PluggableUSBModule::write_finish(usb_ep_t endpoint)
6060
return PluggableUSBD().write_finish(endpoint);
6161
}
6262

63+
void arduino::internal::PluggableUSBModule::callback_reset()
64+
{
65+
}
66+
6367
arduino::PluggableUSBDevice::PluggableUSBDevice(uint16_t vendor_id, uint16_t product_id)
6468
: USBDevice(get_usb_phy(), vendor_id, product_id, 1 << 8)
6569
{

Diff for: cores/arduino/USB/PluggableUSBDevice.h

+19-9
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ class PluggableUSBModule {
3838
PluggableUSBModule(uint8_t numIfs) :
3939
numInterfaces(numIfs)
4040
{ }
41+
virtual ~PluggableUSBModule()
42+
{ }
4143
void lock();
4244
void unlock();
4345
void assert_locked();
@@ -49,17 +51,25 @@ class PluggableUSBModule {
4951
uint32_t write_finish(usb_ep_t endpoint);
5052

5153
protected:
52-
virtual const uint8_t *configuration_desc(uint8_t index);
53-
virtual void callback_reset() {};
54-
virtual void callback_state_change(USBDevice::DeviceState new_state);
55-
virtual uint32_t callback_request(const USBDevice::setup_packet_t *setup, USBDevice::RequestResult *result, uint8_t** data);
56-
virtual bool callback_request_xfer_done(const USBDevice::setup_packet_t *setup, bool aborted);
57-
virtual bool callback_set_configuration(uint8_t configuration);
58-
virtual void callback_set_interface(uint16_t interface, uint8_t alternate);
59-
virtual void init(EndpointResolver& resolver);
60-
virtual const uint8_t *string_iinterface_desc();
54+
virtual const uint8_t *configuration_desc(uint8_t index) = 0;
55+
virtual void callback_state_change(USBDevice::DeviceState new_state) = 0;
56+
virtual uint32_t callback_request(const USBDevice::setup_packet_t *setup, USBDevice::RequestResult *result, uint8_t** data) = 0;
57+
virtual bool callback_request_xfer_done(const USBDevice::setup_packet_t *setup, bool aborted) = 0;
58+
virtual bool callback_set_configuration(uint8_t configuration) = 0;
59+
virtual void callback_set_interface(uint16_t interface, uint8_t alternate) = 0;
60+
virtual void init(EndpointResolver& resolver) = 0;
61+
virtual const uint8_t *string_iinterface_desc() = 0;
6162
virtual uint8_t getProductVersion() = 0;
6263

64+
/**
65+
* Non-pure virtual method with a non-inline definition
66+
*
67+
* @note This satisfies all criteria necessary for the compiler to emit a
68+
* vtable, allowing users to safely subclass any of the derived "pluggable"
69+
* USB device classes (USBCDC, USBHID, USBMIDI, etc).
70+
*/
71+
virtual void callback_reset();
72+
6373
uint8_t pluggedInterface;
6474

6575
const uint8_t numInterfaces;

0 commit comments

Comments
 (0)