From b6b3efc14b21117d13ae33a2eda96c5647817d5b Mon Sep 17 00:00:00 2001 From: Joel Challis Date: Tue, 9 Jan 2024 14:01:34 +0000 Subject: [PATCH] Remove console out endpoint (#22304) --- tmk_core/protocol/chibios/usb_main.c | 103 ++++++++++++++------------- tmk_core/protocol/chibios/usb_main.h | 3 - tmk_core/protocol/lufa/lufa.c | 46 ++++-------- tmk_core/protocol/usb_descriptor.c | 20 +----- tmk_core/protocol/usb_descriptor.h | 14 ---- tmk_core/protocol/vusb/vusb.c | 17 ----- tmk_core/protocol/vusb/vusb.h | 1 - 7 files changed, 66 insertions(+), 138 deletions(-) diff --git a/tmk_core/protocol/chibios/usb_main.c b/tmk_core/protocol/chibios/usb_main.c index 66f9ad0318..7b1e641213 100644 --- a/tmk_core/protocol/chibios/usb_main.c +++ b/tmk_core/protocol/chibios/usb_main.c @@ -51,6 +51,11 @@ extern keymap_config_t keymap_config; #endif +#if defined(CONSOLE_ENABLE) +# define RBUF_SIZE 256 +# include "ring_buffer.h" +#endif + /* --------------------------------------------------------- * Global interface variables and declarations * --------------------------------------------------------- @@ -217,6 +222,24 @@ static const USBEndpointConfig digitizer_ep_config = { }; #endif +#ifdef CONSOLE_ENABLE +/* Console endpoint state structure */ +static USBInEndpointState console_ep_state; + +/* Console endpoint initialization structure (IN) - see USBEndpointConfig comment at top of file */ +static const USBEndpointConfig console_ep_config = { + USB_EP_MODE_TYPE_INTR, /* Interrupt EP */ + NULL, /* SETUP packet notification callback */ + dummy_usb_cb, /* IN notification callback */ + NULL, /* OUT notification callback */ + CONSOLE_EPSIZE, /* IN maximum packet size */ + 0, /* OUT maximum packet size */ + &console_ep_state, /* IN Endpoint state */ + NULL, /* OUT endpoint state */ + usb_lld_endpoint_fields /* USB driver specific endpoint fields */ +}; +#endif + #ifdef USB_ENDPOINTS_ARE_REORDERABLE typedef struct { size_t queue_capacity_in; @@ -347,9 +370,6 @@ typedef struct { typedef struct { union { struct { -#ifdef CONSOLE_ENABLE - usb_driver_config_t console_driver; -#endif #ifdef RAW_ENABLE usb_driver_config_t raw_driver; #endif @@ -365,13 +385,6 @@ typedef struct { } usb_driver_configs_t; static usb_driver_configs_t drivers = { -#ifdef CONSOLE_ENABLE -# define CONSOLE_IN_CAPACITY 4 -# define CONSOLE_OUT_CAPACITY 4 -# define CONSOLE_IN_MODE USB_EP_MODE_TYPE_INTR -# define CONSOLE_OUT_MODE USB_EP_MODE_TYPE_INTR - .console_driver = QMK_USB_DRIVER_CONFIG(CONSOLE, 0, true), -#endif #ifdef RAW_ENABLE # ifndef RAW_IN_CAPACITY # define RAW_IN_CAPACITY 4 @@ -509,6 +522,9 @@ static void usb_event_cb(USBDriver *usbp, usbevent_t event) { #endif #if defined(DIGITIZER_ENABLE) && !defined(DIGITIZER_SHARED_EP) usbInitEndpointI(usbp, DIGITIZER_IN_EPNUM, &digitizer_ep_config); +#endif +#ifdef CONSOLE_ENABLE + usbInitEndpointI(usbp, CONSOLE_IN_EPNUM, &console_ep_config); #endif for (int i = 0; i < NUM_USB_DRIVERS; i++) { #ifdef USB_ENDPOINTS_ARE_REORDERABLE @@ -915,50 +931,35 @@ void send_digitizer(report_digitizer_t *report) { #ifdef CONSOLE_ENABLE int8_t sendchar(uint8_t c) { - static bool timed_out = false; - /* The `timed_out` state is an approximation of the ideal `is_listener_disconnected?` state. - * - * When a 5ms timeout write has timed out, hid_listen is most likely not running, or not - * listening to this keyboard, so we go into the timed_out state. In this state we assume - * that hid_listen is most likely not gonna be connected to us any time soon, so it would - * be wasteful to write follow-up characters with a 5ms timeout, it would all add up and - * unncecessarily slow down the firmware. However instead of just dropping the characters, - * we write them with a TIME_IMMEDIATE timeout, which is a zero timeout, - * and this will succeed only if hid_listen gets connected again. When a write with - * TIME_IMMEDIATE timeout succeeds, we know that hid_listen is listening to us again, and - * we can go back to the timed_out = false state, and following writes will be executed - * with a 5ms timeout. The reason we don't just send all characters with the TIME_IMMEDIATE - * timeout is that this could cause bytes to be lost even if hid_listen is running, if there - * is a lot of data being sent over the console. - * - * This logic will work correctly as long as hid_listen is able to receive at least 200 - * bytes per second. On a heavily overloaded machine that's so overloaded that it's - * unusable, and constantly swapping, hid_listen might have trouble receiving 200 bytes per - * second, so some bytes might be lost on the console. - */ - - const sysinterval_t timeout = timed_out ? TIME_IMMEDIATE : TIME_MS2I(5); - const size_t result = chnWriteTimeout(&drivers.console_driver.driver, &c, 1, timeout); - timed_out = (result == 0); - return result; -} - -// Just a dummy function for now, this could be exposed as a weak function -// Or connected to the actual QMK console -static void console_receive(uint8_t *data, uint8_t length) { - (void)data; - (void)length; + rbuf_enqueue(c); + return 0; } void console_task(void) { - uint8_t buffer[CONSOLE_EPSIZE]; - size_t size = 0; - do { - size = chnReadTimeout(&drivers.console_driver.driver, buffer, sizeof(buffer), TIME_IMMEDIATE); - if (size > 0) { - console_receive(buffer, size); - } - } while (size > 0); + if (!rbuf_has_data()) { + return; + } + + osalSysLock(); + if (usbGetDriverStateI(&USB_DRIVER) != USB_ACTIVE) { + osalSysUnlock(); + return; + } + + if (usbGetTransmitStatusI(&USB_DRIVER, CONSOLE_IN_EPNUM)) { + osalSysUnlock(); + return; + } + + // Send in chunks - padded with zeros to 32 + char send_buf[CONSOLE_EPSIZE] = {0}; + uint8_t send_buf_count = 0; + while (rbuf_has_data() && send_buf_count < CONSOLE_EPSIZE) { + send_buf[send_buf_count++] = rbuf_dequeue(); + } + + usbStartTransmitI(&USB_DRIVER, CONSOLE_IN_EPNUM, (const uint8_t *)send_buf, CONSOLE_EPSIZE); + osalSysUnlock(); } #endif /* CONSOLE_ENABLE */ diff --git a/tmk_core/protocol/chibios/usb_main.h b/tmk_core/protocol/chibios/usb_main.h index 07186f76b8..3fd1e84fe8 100644 --- a/tmk_core/protocol/chibios/usb_main.h +++ b/tmk_core/protocol/chibios/usb_main.h @@ -57,7 +57,4 @@ void usb_event_queue_task(void); /* Putchar over the USB console */ int8_t sendchar(uint8_t c); -/* Flush output (send everything immediately) */ -void console_flush_output(void); - #endif /* CONSOLE_ENABLE */ diff --git a/tmk_core/protocol/lufa/lufa.c b/tmk_core/protocol/lufa/lufa.c index 553f69b1e4..22cc0db8ce 100644 --- a/tmk_core/protocol/lufa/lufa.c +++ b/tmk_core/protocol/lufa/lufa.c @@ -184,41 +184,16 @@ static void raw_hid_task(void) { * Console ******************************************************************************/ #ifdef CONSOLE_ENABLE -/** \brief Console Task +/** \brief Console Tasks * * FIXME: Needs doc */ -static void Console_Task(void) { +static void console_flush_task(void) { /* Device must be connected and configured for the task to run */ if (USB_DeviceState != DEVICE_STATE_Configured) return; uint8_t ep = Endpoint_GetCurrentEndpoint(); -# if 0 - // TODO: impl receivechar()/recvchar() - Endpoint_SelectEndpoint(CONSOLE_OUT_EPNUM); - - /* Check to see if a packet has been sent from the host */ - if (Endpoint_IsOUTReceived()) - { - /* Check to see if the packet contains data */ - if (Endpoint_IsReadWriteAllowed()) - { - /* Create a temporary buffer to hold the read in report from the host */ - uint8_t ConsoleData[CONSOLE_EPSIZE]; - - /* Read Console Report Data */ - Endpoint_Read_Stream_LE(&ConsoleData, sizeof(ConsoleData), NULL); - - /* Process Console Report Data */ - //ProcessConsoleHIDReport(ConsoleData); - } - - /* Finalize the stream transfer to send the last packet */ - Endpoint_ClearOUT(); - } -# endif - /* IN packet */ Endpoint_SelectEndpoint(CONSOLE_IN_EPNUM); if (!Endpoint_IsEnabled() || !Endpoint_IsConfigured()) { @@ -237,6 +212,10 @@ static void Console_Task(void) { Endpoint_SelectEndpoint(ep); } + +void console_task(void) { + // do nothing +} #endif /******************************************************************************* @@ -341,7 +320,7 @@ void EVENT_USB_Device_StartOfFrame(void) { count = 0; if (!console_flush) return; - Console_Task(); + console_flush_task(); console_flush = false; } @@ -381,9 +360,6 @@ void EVENT_USB_Device_ConfigurationChanged(void) { #ifdef CONSOLE_ENABLE /* Setup console endpoint */ ConfigSuccess &= Endpoint_ConfigureEndpoint((CONSOLE_IN_EPNUM | ENDPOINT_DIR_IN), EP_TYPE_INTERRUPT, CONSOLE_EPSIZE, 1); -# if 0 - ConfigSuccess &= Endpoint_ConfigureEndpoint((CONSOLE_OUT_EPNUM | ENDPOINT_DIR_OUT), EP_TYPE_INTERRUPT, CONSOLE_EPSIZE, 1); -# endif #endif #ifdef MIDI_ENABLE @@ -627,7 +603,7 @@ int8_t sendchar(uint8_t c) { // The `timed_out` state is an approximation of the ideal `is_listener_disconnected?` state. static bool timed_out = false; - // prevents Console_Task() from running during sendchar() runs. + // prevents console_flush_task() from running during sendchar() runs. // or char will be lost. These two function is mutually exclusive. CONSOLE_FLUSH_SET(false); @@ -812,7 +788,7 @@ static void setup_usb(void) { USB_Init(); - // for Console_Task + // for console_flush_task USB_Device_EnableSOFEvents(); } @@ -876,6 +852,10 @@ void protocol_pre_task(void) { } void protocol_post_task(void) { +#ifdef CONSOLE_ENABLE + console_task(); +#endif + #ifdef MIDI_ENABLE MIDI_Device_USBTask(&USB_MIDI_Interface); #endif diff --git a/tmk_core/protocol/usb_descriptor.c b/tmk_core/protocol/usb_descriptor.c index eb214c0492..0e2e63ad8e 100644 --- a/tmk_core/protocol/usb_descriptor.c +++ b/tmk_core/protocol/usb_descriptor.c @@ -420,14 +420,6 @@ const USB_Descriptor_HIDReport_Datatype_t PROGMEM ConsoleReport[] = { HID_RI_REPORT_COUNT(8, CONSOLE_EPSIZE), HID_RI_REPORT_SIZE(8, 0x08), HID_RI_INPUT(8, HID_IOF_DATA | HID_IOF_VARIABLE | HID_IOF_ABSOLUTE), - - // Data from host - HID_RI_USAGE(8, 0x76), // Vendor Defined - HID_RI_LOGICAL_MINIMUM(8, 0x00), - HID_RI_LOGICAL_MAXIMUM(16, 0x00FF), - HID_RI_REPORT_COUNT(8, CONSOLE_EPSIZE), - HID_RI_REPORT_SIZE(8, 0x08), - HID_RI_OUTPUT(8, HID_IOF_DATA | HID_IOF_VARIABLE | HID_IOF_ABSOLUTE | HID_IOF_NON_VOLATILE), HID_RI_END_COLLECTION(0), }; #endif @@ -677,7 +669,7 @@ const USB_Descriptor_Configuration_t PROGMEM ConfigurationDescriptor = { }, .InterfaceNumber = CONSOLE_INTERFACE, .AlternateSetting = 0x00, - .TotalEndpoints = 2, + .TotalEndpoints = 1, .Class = HID_CSCP_HIDClass, .SubClass = HID_CSCP_NonBootSubclass, .Protocol = HID_CSCP_NonBootProtocol, @@ -704,16 +696,6 @@ const USB_Descriptor_Configuration_t PROGMEM ConfigurationDescriptor = { .EndpointSize = CONSOLE_EPSIZE, .PollingIntervalMS = 0x01 }, - .Console_OUTEndpoint = { - .Header = { - .Size = sizeof(USB_Descriptor_Endpoint_t), - .Type = DTYPE_Endpoint - }, - .EndpointAddress = (ENDPOINT_DIR_OUT | CONSOLE_OUT_EPNUM), - .Attributes = (EP_TYPE_INTERRUPT | ENDPOINT_ATTR_NO_SYNC | ENDPOINT_USAGE_DATA), - .EndpointSize = CONSOLE_EPSIZE, - .PollingIntervalMS = 0x01 - }, #endif #ifdef MIDI_ENABLE diff --git a/tmk_core/protocol/usb_descriptor.h b/tmk_core/protocol/usb_descriptor.h index 1268bdae73..2469990f4d 100644 --- a/tmk_core/protocol/usb_descriptor.h +++ b/tmk_core/protocol/usb_descriptor.h @@ -97,7 +97,6 @@ typedef struct { USB_Descriptor_Interface_t Console_Interface; USB_HID_Descriptor_HID_t Console_HID; USB_Descriptor_Endpoint_t Console_INEndpoint; - USB_Descriptor_Endpoint_t Console_OUTEndpoint; #endif #ifdef MIDI_ENABLE @@ -232,19 +231,6 @@ enum usb_endpoints { #ifdef CONSOLE_ENABLE CONSOLE_IN_EPNUM = NEXT_EPNUM, - -# ifdef PROTOCOL_CHIBIOS -// ChibiOS has enough memory and descriptor to actually enable the endpoint -// It could use the same endpoint numbers, as that's supported by ChibiOS -// But the QMK code currently assumes that the endpoint numbers are different -# ifdef USB_ENDPOINTS_ARE_REORDERABLE -# define CONSOLE_OUT_EPNUM CONSOLE_IN_EPNUM -# else - CONSOLE_OUT_EPNUM = NEXT_EPNUM, -# endif -# else -# define CONSOLE_OUT_EPNUM CONSOLE_IN_EPNUM -# endif #endif #ifdef MIDI_ENABLE diff --git a/tmk_core/protocol/vusb/vusb.c b/tmk_core/protocol/vusb/vusb.c index d09b2f19b7..cfeeed3712 100644 --- a/tmk_core/protocol/vusb/vusb.c +++ b/tmk_core/protocol/vusb/vusb.c @@ -717,13 +717,6 @@ const PROGMEM uchar console_hid_report[] = { 0x95, CONSOLE_BUFFER_SIZE, // Report Count 0x75, 0x08, // Report Size (8) 0x81, 0x02, // Input (Data, Variable, Absolute) - // Data from host - 0x09, 0x76, // Usage (Vendor Defined) - 0x15, 0x00, // Logical Minimum (0x00) - 0x26, 0xFF, 0x00, // Logical Maximum (0x00FF) - 0x95, CONSOLE_BUFFER_SIZE, // Report Count - 0x75, 0x08, // Report Size (8) - 0x91, 0x02, // Output (Data) 0xC0 // End Collection }; #endif @@ -991,16 +984,6 @@ const PROGMEM usbConfigurationDescriptor_t usbConfigurationDescriptor = { .wMaxPacketSize = CONSOLE_EPSIZE, .bInterval = 0x01 }, - .consoleOUTEndpoint = { - .header = { - .bLength = sizeof(usbEndpointDescriptor_t), - .bDescriptorType = USBDESCR_ENDPOINT - }, - .bEndpointAddress = (USBRQ_DIR_HOST_TO_DEVICE | USB_CFG_EP3_NUMBER), - .bmAttributes = 0x03, - .wMaxPacketSize = CONSOLE_EPSIZE, - .bInterval = 0x01 - } # endif }; diff --git a/tmk_core/protocol/vusb/vusb.h b/tmk_core/protocol/vusb/vusb.h index ae17e5e014..4750e95bf2 100644 --- a/tmk_core/protocol/vusb/vusb.h +++ b/tmk_core/protocol/vusb/vusb.h @@ -114,7 +114,6 @@ typedef struct usbConfigurationDescriptor { usbInterfaceDescriptor_t consoleInterface; usbHIDDescriptor_t consoleHID; usbEndpointDescriptor_t consoleINEndpoint; - usbEndpointDescriptor_t consoleOUTEndpoint; #endif } __attribute__((packed)) usbConfigurationDescriptor_t;