From 14bc4c44613b1f99378e9414b587e5c046099120 Mon Sep 17 00:00:00 2001 From: Taylor Yu Date: Wed, 11 Jan 2023 22:27:20 -0600 Subject: [PATCH 1/2] use singletons for HID objects Use singletons for the HID objects, so Kaleidoscope can force an ordering for the interfaces, because some BIOSes/UEFIs require that the boot keyboard be the first HID interface. Signed-off-by: Taylor Yu --- src/BootKeyboard/BootKeyboard.cpp | 5 ++++- src/BootKeyboard/BootKeyboard.h | 2 +- src/MultiReport/AbsoluteMouse.cpp | 5 ++++- src/MultiReport/AbsoluteMouse.h | 2 +- src/MultiReport/ConsumerControl.cpp | 5 ++++- src/MultiReport/ConsumerControl.h | 2 +- src/MultiReport/Gamepad.cpp | 5 ++++- src/MultiReport/Gamepad.h | 2 +- src/MultiReport/Keyboard.cpp | 5 ++++- src/MultiReport/Keyboard.h | 2 +- src/MultiReport/Mouse.cpp | 5 ++++- src/MultiReport/Mouse.h | 2 +- src/MultiReport/SystemControl.cpp | 5 ++++- src/MultiReport/SystemControl.h | 2 +- src/SingleReport/SingleAbsoluteMouse.cpp | 5 ++++- src/SingleReport/SingleAbsoluteMouse.h | 2 +- 16 files changed, 40 insertions(+), 16 deletions(-) diff --git a/src/BootKeyboard/BootKeyboard.cpp b/src/BootKeyboard/BootKeyboard.cpp index 08740b84..b0c0dc29 100644 --- a/src/BootKeyboard/BootKeyboard.cpp +++ b/src/BootKeyboard/BootKeyboard.cpp @@ -415,4 +415,7 @@ void BootKeyboard_::checkReset() { } __attribute__((weak)) -BootKeyboard_ BootKeyboard; +BootKeyboard_& BootKeyboard() { + static BootKeyboard_ obj; + return obj; +}; diff --git a/src/BootKeyboard/BootKeyboard.h b/src/BootKeyboard/BootKeyboard.h index 15ddb38a..177871b1 100644 --- a/src/BootKeyboard/BootKeyboard.h +++ b/src/BootKeyboard/BootKeyboard.h @@ -81,4 +81,4 @@ class BootKeyboard_ : public PluggableUSBModule { uint8_t leds; }; -extern BootKeyboard_ BootKeyboard; +extern BootKeyboard_& BootKeyboard(); diff --git a/src/MultiReport/AbsoluteMouse.cpp b/src/MultiReport/AbsoluteMouse.cpp index 437ef915..0dbad52b 100644 --- a/src/MultiReport/AbsoluteMouse.cpp +++ b/src/MultiReport/AbsoluteMouse.cpp @@ -51,4 +51,7 @@ void AbsoluteMouse_::sendReport(void* data, int length) { HID().SendReport(HID_REPORTID_MOUSE_ABSOLUTE, data, length); } -AbsoluteMouse_ AbsoluteMouse; +AbsoluteMouse_& AbsoluteMouse() { + static AbsoluteMouse_ obj; + return obj; +}; diff --git a/src/MultiReport/AbsoluteMouse.h b/src/MultiReport/AbsoluteMouse.h index 8dabd085..06700fd0 100644 --- a/src/MultiReport/AbsoluteMouse.h +++ b/src/MultiReport/AbsoluteMouse.h @@ -40,4 +40,4 @@ class AbsoluteMouse_ : public AbsoluteMouseAPI { virtual void sendReport(void* data, int length); }; -extern AbsoluteMouse_ AbsoluteMouse; +extern AbsoluteMouse_& AbsoluteMouse(); diff --git a/src/MultiReport/ConsumerControl.cpp b/src/MultiReport/ConsumerControl.cpp index ffada2b7..bd368e59 100644 --- a/src/MultiReport/ConsumerControl.cpp +++ b/src/MultiReport/ConsumerControl.cpp @@ -104,4 +104,7 @@ void ConsumerControl_::sendReport() { memcpy(&last_report_, &report_, sizeof(report_)); } -ConsumerControl_ ConsumerControl; +ConsumerControl_& ConsumerControl() { + static ConsumerControl_ obj; + return obj; +}; diff --git a/src/MultiReport/ConsumerControl.h b/src/MultiReport/ConsumerControl.h index ed3ac03f..85eafe43 100644 --- a/src/MultiReport/ConsumerControl.h +++ b/src/MultiReport/ConsumerControl.h @@ -62,4 +62,4 @@ class ConsumerControl_ { private: void sendReportUnchecked(); }; -extern ConsumerControl_ ConsumerControl; +extern ConsumerControl_& ConsumerControl(); diff --git a/src/MultiReport/Gamepad.cpp b/src/MultiReport/Gamepad.cpp index d533ff72..462085dc 100644 --- a/src/MultiReport/Gamepad.cpp +++ b/src/MultiReport/Gamepad.cpp @@ -144,4 +144,7 @@ void Gamepad_::sendReport(void* data, int length) { HID().SendReport(HID_REPORTID_GAMEPAD, data, length); } -Gamepad_ Gamepad; +Gamepad_& Gamepad() { + static Gamepad_ obj; + return obj; +}; diff --git a/src/MultiReport/Gamepad.h b/src/MultiReport/Gamepad.h index c2892d1d..f18215ab 100644 --- a/src/MultiReport/Gamepad.h +++ b/src/MultiReport/Gamepad.h @@ -122,4 +122,4 @@ class Gamepad_ { protected: HID_GamepadReport_Data_t report_; }; -extern Gamepad_ Gamepad; +extern Gamepad_& Gamepad(); diff --git a/src/MultiReport/Keyboard.cpp b/src/MultiReport/Keyboard.cpp index 8cd8de4f..f2ff3b66 100644 --- a/src/MultiReport/Keyboard.cpp +++ b/src/MultiReport/Keyboard.cpp @@ -273,4 +273,7 @@ void Keyboard_::releaseAll() { memset(&report_.allkeys, 0x00, sizeof(report_.allkeys)); } -Keyboard_ Keyboard; +Keyboard_& Keyboard() { + static Keyboard_ obj; + return obj; +}; diff --git a/src/MultiReport/Keyboard.h b/src/MultiReport/Keyboard.h index ef0881b8..0da9b8a6 100644 --- a/src/MultiReport/Keyboard.h +++ b/src/MultiReport/Keyboard.h @@ -74,4 +74,4 @@ class Keyboard_ { int sendReportUnchecked(); }; -extern Keyboard_ Keyboard; +extern Keyboard_& Keyboard(); diff --git a/src/MultiReport/Mouse.cpp b/src/MultiReport/Mouse.cpp index ca4bdf71..51a28717 100644 --- a/src/MultiReport/Mouse.cpp +++ b/src/MultiReport/Mouse.cpp @@ -139,4 +139,7 @@ void Mouse_::sendReport() { } } -Mouse_ Mouse; +Mouse_& Mouse() { + static Mouse_ obj; + return obj; +}; diff --git a/src/MultiReport/Mouse.h b/src/MultiReport/Mouse.h index 469be1fd..ec72bcf5 100644 --- a/src/MultiReport/Mouse.h +++ b/src/MultiReport/Mouse.h @@ -77,4 +77,4 @@ class Mouse_ { private: void sendReportUnchecked(); }; -extern Mouse_ Mouse; +extern Mouse_& Mouse(); diff --git a/src/MultiReport/SystemControl.cpp b/src/MultiReport/SystemControl.cpp index 7385b320..c7227d4d 100644 --- a/src/MultiReport/SystemControl.cpp +++ b/src/MultiReport/SystemControl.cpp @@ -92,4 +92,7 @@ void SystemControl_::sendReport(void* data, int length) { HID().SendReport(HID_REPORTID_SYSTEMCONTROL, data, length); } -SystemControl_ SystemControl; +SystemControl_& SystemControl() { + static SystemControl_ obj; + return obj; +}; diff --git a/src/MultiReport/SystemControl.h b/src/MultiReport/SystemControl.h index 1b6fd1b1..2301b04b 100644 --- a/src/MultiReport/SystemControl.h +++ b/src/MultiReport/SystemControl.h @@ -54,4 +54,4 @@ class SystemControl_ { -extern SystemControl_ SystemControl; +extern SystemControl_& SystemControl(); diff --git a/src/SingleReport/SingleAbsoluteMouse.cpp b/src/SingleReport/SingleAbsoluteMouse.cpp index 69998fe6..ca7234f8 100644 --- a/src/SingleReport/SingleAbsoluteMouse.cpp +++ b/src/SingleReport/SingleAbsoluteMouse.cpp @@ -122,4 +122,7 @@ void SingleAbsoluteMouse_::sendReport(void* data, int length) { HIDReportObserver::observeReport(HID_REPORTID_MOUSE_ABSOLUTE, data, length, result); } -SingleAbsoluteMouse_ SingleAbsoluteMouse; +SingleAbsoluteMouse_& SingleAbsoluteMouse() { + static SingleAbsoluteMouse_ obj; + return obj; +}; diff --git a/src/SingleReport/SingleAbsoluteMouse.h b/src/SingleReport/SingleAbsoluteMouse.h index 7a59403e..25b38307 100644 --- a/src/SingleReport/SingleAbsoluteMouse.h +++ b/src/SingleReport/SingleAbsoluteMouse.h @@ -50,4 +50,4 @@ class SingleAbsoluteMouse_ : public PluggableUSBModule, public AbsoluteMouseAPI virtual inline void sendReport(void* data, int length) override; }; -extern SingleAbsoluteMouse_ SingleAbsoluteMouse; +extern SingleAbsoluteMouse_& SingleAbsoluteMouse(); From 3a97d0a6d95b11a486251afa7f6a6f7a794da50d Mon Sep 17 00:00:00 2001 From: Taylor Yu Date: Tue, 29 Nov 2022 15:44:30 -0600 Subject: [PATCH 2/2] WIP remove GD32 `recvControl` workaround Remove the GD32 `recvControl` workaround, because changes to the USB core mean that it's no longer required, and the automatic buffer will cause stack corruption due to being written to after `recvControl` returns. --- src/BootKeyboard/BootKeyboard.cpp | 29 ++--------------------------- src/HID.cpp | 29 ++--------------------------- 2 files changed, 4 insertions(+), 54 deletions(-) diff --git a/src/BootKeyboard/BootKeyboard.cpp b/src/BootKeyboard/BootKeyboard.cpp index b0c0dc29..28a199f6 100644 --- a/src/BootKeyboard/BootKeyboard.cpp +++ b/src/BootKeyboard/BootKeyboard.cpp @@ -179,37 +179,12 @@ bool BootKeyboard_::setup(USBSetup& setup) { // Check if data has the correct length afterwards int length = setup.wLength; - - // ------------------------------------------------------------ - // Workaround for a bug in the GD32 core: - // - // On GD32, when we call `USV_RecvControl`, it casts the (void*) pointer - // we give it to `uint16_t*`, which means that it will alway write an even - // number of bytes to that pointer. Because we don't want to overwrite - // the next byte in memory past `leds`, we use a temporary array that is - // guaranteed to be big enough, and copy the data from that: if (setup.wValueH == HID_REPORT_TYPE_OUTPUT) { if (length == sizeof(leds)) { - uint8_t raw_report_data[2]; - USB_RecvControl(&raw_report_data, length); - leds = raw_report_data[0]; + USB_RecvControl(&leds, length); return true; -// } else { -// char tmp[8]; -// USB_RecvControl(&tmp, length); -// return true; - } + } } - // Once the GD32 core bug is fixed, we can replace the above code with the - // original code below: - // ------------------------------------------------------------ - // if (setup.wValueH == HID_REPORT_TYPE_OUTPUT) { - // if (length == sizeof(leds)) { - // USB_RecvControl(&leds, length); - // return true; - // } - // } - // ------------------------------------------------------------ // Input (set HID report) else if (setup.wValueH == HID_REPORT_TYPE_INPUT) { diff --git a/src/HID.cpp b/src/HID.cpp index 9be216b5..aee06278 100644 --- a/src/HID.cpp +++ b/src/HID.cpp @@ -151,37 +151,12 @@ bool HID_::setup(USBSetup& setup) { if (request == HID_SET_REPORT) { uint16_t length = setup.wLength; - // ------------------------------------------------------------ - // Workaround for a bug in the GD32 core: - // - // On GD32, when we call `USV_RecvControl`, it casts the (void*) pointer - // we give it to `uint16_t*`, which means that it will alway write an even - // number of bytes to that pointer. Because we might be trying to just - // read the `leds` byte, and we don't want to overwrite the next byte in - // memory, instead of giving it the pointer to the `setReportData` member - // variable directly, we have it write into to temporary `raw_report_data` - // array that's guaranteed to be big enough, then copy the data from that - // array into `setReportData`: - uint8_t raw_report_data[sizeof(setReportData) + 1]; if (length == sizeof(setReportData)) { - USB_RecvControl(&raw_report_data, length); - setReportData.reportId = raw_report_data[0]; - setReportData.leds = raw_report_data[1]; + USB_RecvControl(&setReportData, length); } else if (length == sizeof(setReportData.leds)) { - USB_RecvControl(&raw_report_data, length); + USB_RecvControl(&setReportData.leds, length); setReportData.reportId = 0; - setReportData.leds = raw_report_data[0]; } - // Once the GD32 core bug is fixed, we can replace the above code with the - // original code below: - // ------------------------------------------------------------ - // if (length == sizeof(setReportData)) { - // USB_RecvControl(&setReportData, length); - // } else if (length == sizeof(setReportData.leds)) { - // USB_RecvControl(&setReportData.leds, length); - // setReportData.reportId = 0; - // } - // ------------------------------------------------------------ return true; } }