usb_device_state: add idle_rate, led and protocol

Previously all usb drivers and platform implementations (expect for our
oddball atsam) tracked the same two global variables:

- keyboard_protocol: to indicate if we are in report or boot protocol
- idle_rate: for the idle_rate of the keyboard endpoint

And a local variable that was exposed trough some indirection:

- leds: for the currently set indicator leds (caps lock etc.)

These have all been moved into the usb_device_state_t struct wich is
accessible by getters and setters.

This reduces code duplication and centralizes the state management
across platforms and drivers.

Signed-off-by: Stefan Kerkmann <[email protected]>

drivers/haptic/solenoid.c

@@ -158,7 +158,7 @@ void solenoid_setup(void) {
 #endif
         gpio_write_pin(solenoid_pads[i], !solenoid_active_state[i]);
         gpio_set_pin_output(solenoid_pads[i]);
-        if ((!HAPTIC_OFF_IN_LOW_POWER) || (usb_device_state == USB_DEVICE_STATE_CONFIGURED)) {
+        if ((!HAPTIC_OFF_IN_LOW_POWER) || (usb_device_state_get_configure_state() == USB_DEVICE_STATE_CONFIGURED)) {
             solenoid_fire(i);
         }
     }

quantum/action_util.c

@@ -318,7 +318,7 @@ void send_nkro_report(void) {
  */
 void send_keyboard_report(void) {
 #ifdef NKRO_ENABLE
-    if (keyboard_protocol && keymap_config.nkro) {
+    if (usb_device_state_get_protocol() == USB_PROTOCOL_REPORT && keymap_config.nkro) {
         send_nkro_report();
     } else {
         send_6kro_report();

quantum/command.c

@@ -230,8 +230,8 @@ static void print_status(void) {
         "timer_read32(): %08lX\n"
 
         , host_keyboard_leds()
-        , keyboard_protocol
-        , keyboard_idle
+        , usb_device_state_get_protocol()
+        , usb_device_state_get_idle_rate()
 #ifdef NKRO_ENABLE
         , keymap_config.nkro
 #endif

quantum/haptic.c

@@ -36,7 +36,7 @@ extern uint8_t split_haptic_play;
 haptic_config_t haptic_config;
 
 static void update_haptic_enable_gpios(void) {
-    if (haptic_config.enable && ((!HAPTIC_OFF_IN_LOW_POWER) || (usb_device_state == USB_DEVICE_STATE_CONFIGURED))) {
+    if (haptic_config.enable && ((!HAPTIC_OFF_IN_LOW_POWER) || (usb_device_state_get_configure_state() == USB_DEVICE_STATE_CONFIGURED))) {
 #if defined(HAPTIC_ENABLE_PIN)
         HAPTIC_ENABLE_PIN_WRITE_ACTIVE();
 #endif

quantum/os_detection.c

@@ -65,15 +65,15 @@ static os_variant_t          reported_os = OS_UNSURE;
 static bool first_report = true;
 
 // to react on USB state changes
-static volatile enum usb_device_state current_usb_device_state  = USB_DEVICE_STATE_INIT;
-static enum usb_device_state          reported_usb_device_state = USB_DEVICE_STATE_INIT;
+static volatile usb_device_state_t current_usb_device_state  = {.configure_state = USB_DEVICE_STATE_INIT};
+static usb_device_state_t          reported_usb_device_state = {.configure_state = USB_DEVICE_STATE_INIT};
 
 // the OS detection might be unstable for a while, "debounce" it
 static volatile bool         debouncing = false;
 static volatile fast_timer_t last_time;
 
 void os_detection_task(void) {
-    if (current_usb_device_state == USB_DEVICE_STATE_CONFIGURED) {
+    if (current_usb_device_state.configure_state == USB_DEVICE_STATE_CONFIGURED) {
         // debouncing goes for both the detected OS as well as the USB state
         if (debouncing && timer_elapsed_fast(last_time) >= OS_DETECTION_DEBOUNCE) {
             debouncing                = false;
@@ -88,7 +88,7 @@ void os_detection_task(void) {
 #ifdef OS_DETECTION_KEYBOARD_RESET
     // resetting the keyboard on the USB device state change callback results in instability, so delegate that to this task
     // only take action if it's been stable at least once, to avoid issues with some KVMs
-    else if (current_usb_device_state == USB_DEVICE_STATE_INIT && reported_usb_device_state != USB_DEVICE_STATE_INIT) {
+    else if (current_usb_device_state.configure_state == USB_DEVICE_STATE_INIT && reported_usb_device_state.configure_state != USB_DEVICE_STATE_INIT) {
         soft_reset_keyboard();
     }
 #endif
@@ -155,15 +155,15 @@ os_variant_t detected_host_os(void) {
 
 void erase_wlength_data(void) {
     memset(&setups_data, 0, sizeof(setups_data));
-    detected_os               = OS_UNSURE;
-    reported_os               = OS_UNSURE;
-    current_usb_device_state  = USB_DEVICE_STATE_INIT;
-    reported_usb_device_state = USB_DEVICE_STATE_INIT;
-    debouncing                = false;
-    first_report              = true;
+    detected_os                               = OS_UNSURE;
+    reported_os                               = OS_UNSURE;
+    current_usb_device_state.configure_state  = USB_DEVICE_STATE_INIT;
+    reported_usb_device_state.configure_state = USB_DEVICE_STATE_INIT;
+    debouncing                                = false;
+    first_report                              = true;
 }
 
-void os_detection_notify_usb_device_state_change(enum usb_device_state usb_device_state) {
+void os_detection_notify_usb_device_state_change(usb_device_state_t usb_device_state) {
     // treat this like any other source of instability
     current_usb_device_state = usb_device_state;
     last_time                = timer_read_fast();

quantum/os_detection.h

@@ -31,7 +31,7 @@ typedef enum {
 void         process_wlength(const uint16_t w_length);
 os_variant_t detected_host_os(void);
 void         erase_wlength_data(void);
-void         os_detection_notify_usb_device_state_change(enum usb_device_state usb_device_state);
+void         os_detection_notify_usb_device_state_change(usb_device_state_t usb_device_state);
 
 void os_detection_task(void);
 

quantum/os_detection/tests/os_detection.cpp

@@ -253,9 +253,11 @@ TEST_F(OsDetectionTest, TestDoNotReportIfUsbUnstable) {
     EXPECT_EQ(detected_host_os(), OS_LINUX);
 }
 
+static usb_device_state_t usb_device_state_configured = {.configure_state = USB_DEVICE_STATE_CONFIGURED};
+
 TEST_F(OsDetectionTest, TestReportAfterDebounce) {
     EXPECT_EQ(check_sequence({0xFF, 0xFF, 0xFF, 0xFE}), OS_LINUX);
-    os_detection_notify_usb_device_state_change(USB_DEVICE_STATE_CONFIGURED);
+    os_detection_notify_usb_device_state_change(usb_device_state_configured);
     os_detection_task();
     assert_not_reported();
 
@@ -291,7 +293,7 @@ TEST_F(OsDetectionTest, TestReportAfterDebounce) {
 
 TEST_F(OsDetectionTest, TestReportAfterDebounceLongWait) {
     EXPECT_EQ(check_sequence({0x12, 0xFF, 0xFF, 0x4, 0x10, 0xFF, 0xFF, 0xFF, 0x4, 0x10, 0x20A, 0x20A, 0x20A, 0x20A, 0x20A, 0x20A}), OS_WINDOWS);
-    os_detection_notify_usb_device_state_change(USB_DEVICE_STATE_CONFIGURED);
+    os_detection_notify_usb_device_state_change(usb_device_state_configured);
     os_detection_task();
     assert_not_reported();
 
@@ -318,7 +320,7 @@ TEST_F(OsDetectionTest, TestReportAfterDebounceLongWait) {
 
 TEST_F(OsDetectionTest, TestReportUnsure) {
     EXPECT_EQ(check_sequence({0x12, 0xFF}), OS_UNSURE);
-    os_detection_notify_usb_device_state_change(USB_DEVICE_STATE_CONFIGURED);
+    os_detection_notify_usb_device_state_change(usb_device_state_configured);
     os_detection_task();
     assert_not_reported();
 
@@ -345,7 +347,7 @@ TEST_F(OsDetectionTest, TestReportUnsure) {
 
 TEST_F(OsDetectionTest, TestDoNotReportIntermediateResults) {
     EXPECT_EQ(check_sequence({0x12, 0xFF}), OS_UNSURE);
-    os_detection_notify_usb_device_state_change(USB_DEVICE_STATE_CONFIGURED);
+    os_detection_notify_usb_device_state_change(usb_device_state_configured);
     os_detection_task();
     assert_not_reported();
 
@@ -356,7 +358,7 @@ TEST_F(OsDetectionTest, TestDoNotReportIntermediateResults) {
 
     // at this stage, the final result has not been reached yet
     EXPECT_EQ(check_sequence({0xFF}), OS_LINUX);
-    os_detection_notify_usb_device_state_change(USB_DEVICE_STATE_CONFIGURED);
+    os_detection_notify_usb_device_state_change(usb_device_state_configured);
     advance_time(OS_DETECTION_DEBOUNCE - 1);
     os_detection_task();
     assert_not_reported();
@@ -365,7 +367,7 @@ TEST_F(OsDetectionTest, TestDoNotReportIntermediateResults) {
 
     // the remainder is processed
     EXPECT_EQ(check_sequence({0x4, 0x10, 0xFF, 0xFF, 0xFF, 0x4, 0x10, 0x20A, 0x20A, 0x20A, 0x20A, 0x20A, 0x20A}), OS_WINDOWS);
-    os_detection_notify_usb_device_state_change(USB_DEVICE_STATE_CONFIGURED);
+    os_detection_notify_usb_device_state_change(usb_device_state_configured);
     advance_time(OS_DETECTION_DEBOUNCE - 1);
     os_detection_task();
     assert_not_reported();

quantum/process_keycode/process_haptic.c

@@ -129,7 +129,7 @@ bool process_haptic(uint16_t keycode, keyrecord_t *record) {
         }
     }
 
-    if (haptic_get_enable() && ((!HAPTIC_OFF_IN_LOW_POWER) || (usb_device_state == USB_DEVICE_STATE_CONFIGURED))) {
+    if (haptic_get_enable() && ((!HAPTIC_OFF_IN_LOW_POWER) || (usb_device_state_get_configure_state() == USB_DEVICE_STATE_CONFIGURED))) {
         if (record->event.pressed) {
             // keypress
             if (haptic_get_feedback() < 2 && get_haptic_enabled_key(keycode, record)) {

tmk_core/protocol/arm_atsam/main_arm_atsam.c

@@ -44,7 +44,7 @@ void    send_extra(report_extra_t *report);
 void deferred_exec_task(void);
 #endif // DEFERRED_EXEC_ENABLE
 
-host_driver_t arm_atsam_driver = {keyboard_leds, send_keyboard, send_nkro, send_mouse, send_extra};
+host_driver_t arm_atsam_driver = {.keyboard_leds = keyboard_leds, .send_keyboard = send_keyboard, .send_nkro = send_nkro, .send_mouse = send_mouse, .send_extra = send_extra};
 
 uint8_t led_states;
 

tmk_core/protocol/arm_atsam/usb/main_usb.c

@@ -23,8 +23,6 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #    include "raw_hid.h"
 #endif
 
-uint8_t keyboard_protocol = 1;
-
 void main_suspend_action(void) {
     ui_powerdown();
 }

tmk_core/protocol/chibios/chibios.c

@@ -62,14 +62,13 @@
  */
 
 /* declarations */
-uint8_t keyboard_leds(void);
-void    send_keyboard(report_keyboard_t *report);
-void    send_nkro(report_nkro_t *report);
-void    send_mouse(report_mouse_t *report);
-void    send_extra(report_extra_t *report);
+void send_keyboard(report_keyboard_t *report);
+void send_nkro(report_nkro_t *report);
+void send_mouse(report_mouse_t *report);
+void send_extra(report_extra_t *report);
 
 /* host struct */
-host_driver_t chibios_driver = {keyboard_leds, send_keyboard, send_nkro, send_mouse, send_extra};
+host_driver_t chibios_driver = {.keyboard_leds = usb_device_state_get_leds, .send_keyboard = send_keyboard, .send_nkro = send_nkro, .send_mouse = send_mouse, .send_extra = send_extra};
 
 #ifdef VIRTSER_ENABLE
 void virtser_task(void);

tmk_core/protocol/chibios/usb_main.c

@@ -54,10 +54,6 @@ extern keymap_config_t keymap_config;
 extern usb_endpoint_in_t  usb_endpoints_in[USB_ENDPOINT_IN_COUNT];
 extern usb_endpoint_out_t usb_endpoints_out[USB_ENDPOINT_OUT_COUNT];
 
-uint8_t _Alignas(2) keyboard_idle     = 0;
-uint8_t _Alignas(2) keyboard_protocol = 1;
-uint8_t keyboard_led_state            = 0;
-
 static bool __attribute__((__unused__)) send_report_buffered(usb_endpoint_in_lut_t endpoint, void *report, size_t size);
 static void __attribute__((__unused__)) flush_report_buffered(usb_endpoint_in_lut_t endpoint, bool padded);
 static bool __attribute__((__unused__)) receive_report(usb_endpoint_out_lut_t endpoint, void *report, size_t size);
@@ -250,10 +246,10 @@ static void set_led_transfer_cb(USBDriver *usbp) {
     if (setup->wLength == 2) {
         uint8_t report_id = set_report_buf[0];
         if ((report_id == REPORT_ID_KEYBOARD) || (report_id == REPORT_ID_NKRO)) {
-            keyboard_led_state = set_report_buf[1];
+            usb_device_state_set_leds(set_report_buf[1]);
         }
     } else {
-        keyboard_led_state = set_report_buf[0];
+        usb_device_state_set_leds(set_report_buf[0]);
     }
 }
 
@@ -269,7 +265,9 @@ static bool usb_requests_hook_cb(USBDriver *usbp) {
                         return usb_get_report_cb(usbp);
                     case HID_REQ_GetProtocol:
                         if (setup->wIndex == KEYBOARD_INTERFACE) {
-                            usbSetupTransfer(usbp, &keyboard_protocol, sizeof(uint8_t), NULL);
+                            static uint8_t keyboard_protocol;
+                            keyboard_protocol = usb_device_state_get_protocol();
+                            usbSetupTransfer(usbp, &keyboard_protocol, sizeof(keyboard_protocol), NULL);
                             return true;
                         }
                         break;
@@ -292,12 +290,12 @@ static bool usb_requests_hook_cb(USBDriver *usbp) {
                         break;
                     case HID_REQ_SetProtocol:
                         if (setup->wIndex == KEYBOARD_INTERFACE) {
-                            keyboard_protocol = setup->wValue.word;
+                            usb_device_state_set_protocol(setup->wValue.lbyte);
                         }
                         usbSetupTransfer(usbp, NULL, 0, NULL);
                         return true;
                     case HID_REQ_SetIdle:
-                        keyboard_idle = setup->wValue.hbyte;
+                        usb_device_state_set_idle_rate(setup->wValue.hbyte);
                         return usb_set_idle_cb(usbp);
                 }
                 break;
@@ -396,11 +394,6 @@ __attribute__((weak)) void restart_usb_driver(USBDriver *usbp) {
  * ---------------------------------------------------------
  */
 
-/* LED status */
-uint8_t keyboard_leds(void) {
-    return keyboard_led_state;
-}
-
 /**
  * @brief Send a report to the host, the report is enqueued into an output
  * queue and send once the USB endpoint becomes empty.
@@ -458,7 +451,7 @@ static bool receive_report(usb_endpoint_out_lut_t endpoint, void *report, size_t
 
 void send_keyboard(report_keyboard_t *report) {
     /* If we're in Boot Protocol, don't send any report ID or other funky fields */
-    if (!keyboard_protocol) {
+    if (usb_device_state_get_protocol() == USB_PROTOCOL_BOOT) {
         send_report(USB_ENDPOINT_IN_KEYBOARD, &report->mods, 8);
     } else {
         send_report(USB_ENDPOINT_IN_KEYBOARD, report, KEYBOARD_REPORT_SIZE);

tmk_core/protocol/host.h

@@ -27,9 +27,6 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 extern "C" {
 #endif
 
-extern uint8_t keyboard_idle;
-extern uint8_t keyboard_protocol;
-
 /* host driver */
 void           host_set_driver(host_driver_t *driver);
 host_driver_t *host_get_driver(void);

tmk_core/protocol/lufa/lufa.c

@@ -72,20 +72,14 @@
 #    define USB_WAIT_FOR_ENUMERATION
 #endif
 
-uint8_t keyboard_idle = 0;
-/* 0: Boot Protocol, 1: Report Protocol(default) */
-uint8_t        keyboard_protocol  = 1;
-static uint8_t keyboard_led_state = 0;
-
 static report_keyboard_t keyboard_report_sent;
 
 /* Host driver */
-static uint8_t keyboard_leds(void);
-static void    send_keyboard(report_keyboard_t *report);
-static void    send_nkro(report_nkro_t *report);
-static void    send_mouse(report_mouse_t *report);
-static void    send_extra(report_extra_t *report);
-host_driver_t  lufa_driver = {keyboard_leds, send_keyboard, send_nkro, send_mouse, send_extra};
+static void   send_keyboard(report_keyboard_t *report);
+static void   send_nkro(report_nkro_t *report);
+static void   send_mouse(report_mouse_t *report);
+static void   send_extra(report_extra_t *report);
+host_driver_t lufa_driver = {.keyboard_leds = usb_device_state_get_leds, .send_keyboard = send_keyboard, .send_nkro = send_nkro, .send_mouse = send_mouse, .send_extra = send_extra};
 
 void send_report(uint8_t endpoint, void *report, size_t size) {
     uint8_t timeout = 255;
@@ -453,10 +447,10 @@ void EVENT_USB_Device_ControlRequest(void) {
                             uint8_t report_id = Endpoint_Read_8();
 
                             if (report_id == REPORT_ID_KEYBOARD || report_id == REPORT_ID_NKRO) {
-                                keyboard_led_state = Endpoint_Read_8();
+                                usb_device_state_set_leds(Endpoint_Read_8());
                             }
                         } else {
-                            keyboard_led_state = Endpoint_Read_8();
+                            usb_device_state_set_leds(Endpoint_Read_8());
                         }
 
                         Endpoint_ClearOUT();
@@ -473,7 +467,7 @@ void EVENT_USB_Device_ControlRequest(void) {
                     Endpoint_ClearSETUP();
                     while (!(Endpoint_IsINReady()))
                         ;
-                    Endpoint_Write_8(keyboard_protocol);
+                    Endpoint_Write_8(usb_device_state_get_protocol());
                     Endpoint_ClearIN();
                     Endpoint_ClearStatusStage();
                 }
@@ -486,7 +480,7 @@ void EVENT_USB_Device_ControlRequest(void) {
                     Endpoint_ClearSETUP();
                     Endpoint_ClearStatusStage();
 
-                    keyboard_protocol = (USB_ControlRequest.wValue & 0xFF);
+                    usb_device_state_set_protocol(USB_ControlRequest.wValue & 0xFF);
                     clear_keyboard();
                 }
             }
@@ -497,7 +491,7 @@ void EVENT_USB_Device_ControlRequest(void) {
                 Endpoint_ClearSETUP();
                 Endpoint_ClearStatusStage();
 
-                keyboard_idle = ((USB_ControlRequest.wValue & 0xFF00) >> 8);
+                usb_device_state_set_idle_rate(USB_ControlRequest.wValue >> 8);
             }
 
             break;
@@ -506,7 +500,7 @@ void EVENT_USB_Device_ControlRequest(void) {
                 Endpoint_ClearSETUP();
                 while (!(Endpoint_IsINReady()))
                     ;
-                Endpoint_Write_8(keyboard_idle);
+                Endpoint_Write_8(usb_device_state_get_idle_rate());
                 Endpoint_ClearIN();
                 Endpoint_ClearStatusStage();
             }
@@ -522,21 +516,14 @@ void EVENT_USB_Device_ControlRequest(void) {
 /*******************************************************************************
  * Host driver
  ******************************************************************************/
-/** \brief Keyboard LEDs
- *
- * FIXME: Needs doc
- */
-static uint8_t keyboard_leds(void) {
-    return keyboard_led_state;
-}
 
 /** \brief Send Keyboard
  *
  * FIXME: Needs doc
  */
 static void send_keyboard(report_keyboard_t *report) {
     /* If we're in Boot Protocol, don't send any report ID or other funky fields */
-    if (!keyboard_protocol) {
+    if (usb_device_state_get_protocol() == USB_PROTOCOL_BOOT) {
         send_report(KEYBOARD_IN_EPNUM, &report->mods, 8);
     } else {
         send_report(KEYBOARD_IN_EPNUM, report, KEYBOARD_REPORT_SIZE);