Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Core] usb_device_state: consolidate usb state handling across implementations #24258

Merged
merged 3 commits into from
Oct 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion drivers/haptic/solenoid.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
3 changes: 3 additions & 0 deletions keyboards/kikoslab/ellora65/keyboard.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@
"pid": "0xE88F",
"device_version": "0.0.1"
},
"build": {
"lto": true
},
"features": {
"bootmagic": true,
"command": false,
Expand Down
3 changes: 3 additions & 0 deletions keyboards/rgbkb/pan/info.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@
"pid": "0x8C9C",
"device_version": "0.0.2"
},
"build": {
"lto": true
},
"features": {
"bootmagic": true,
"encoder": true,
Expand Down
1 change: 0 additions & 1 deletion keyboards/takashicompany/minidivide/keyboard.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
"knight": true,
"rainbow_mood": true,
"rainbow_swirl": true,
"rgb_test": true,
"snake": true,
"static_gradient": true,
"twinkle": true
Expand Down
3 changes: 1 addition & 2 deletions keyboards/work_louder/loop/info.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,7 @@
"pixel_rain": true,
"pixel_flow": true,
"pixel_fractal": true,
"typing_heatmap": true,
"digital_rain": true
"typing_heatmap": true
},
"driver": "ws2812",
"max_brightness": 120,
Expand Down
3 changes: 2 additions & 1 deletion quantum/action_util.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
#include "action_layer.h"
#include "timer.h"
#include "keycode_config.h"
#include "usb_device_state.h"
#include <string.h>

extern keymap_config_t keymap_config;
Expand Down Expand Up @@ -318,7 +319,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();
Expand Down
5 changes: 3 additions & 2 deletions quantum/command.c
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
#include "led.h"
#include "command.h"
#include "quantum.h"
#include "usb_device_state.h"
#include "version.h"

#ifdef BACKLIGHT_ENABLE
Expand Down Expand Up @@ -230,8 +231,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
Expand Down
2 changes: 1 addition & 1 deletion quantum/haptic.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
26 changes: 13 additions & 13 deletions quantum/os_detection.c
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ static volatile os_variant_t reported_os = OS_UNSURE;
static volatile bool first_report = true;

// to react on USB state changes
static volatile enum usb_device_state current_usb_device_state = USB_DEVICE_STATE_NO_INIT;
static volatile enum usb_device_state maxprev_usb_device_state = USB_DEVICE_STATE_NO_INIT;
static volatile struct usb_device_state current_usb_device_state = {.configure_state = USB_DEVICE_STATE_NO_INIT};
static volatile struct usb_device_state maxprev_usb_device_state = {.configure_state = USB_DEVICE_STATE_NO_INIT};

// the OS detection might be unstable for a while, "debounce" it
static volatile bool debouncing = false;
Expand All @@ -88,7 +88,7 @@ void os_detection_task(void) {
return;
}
#endif
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;
Expand Down Expand Up @@ -163,19 +163,19 @@ 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_NO_INIT;
maxprev_usb_device_state = USB_DEVICE_STATE_NO_INIT;
debouncing = false;
last_time = 0;
first_report = true;
detected_os = OS_UNSURE;
reported_os = OS_UNSURE;
current_usb_device_state.configure_state = USB_DEVICE_STATE_NO_INIT;
maxprev_usb_device_state.configure_state = USB_DEVICE_STATE_NO_INIT;
debouncing = false;
last_time = 0;
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(struct usb_device_state usb_device_state) {
// treat this like any other source of instability
if (maxprev_usb_device_state < current_usb_device_state) {
maxprev_usb_device_state = current_usb_device_state;
if (maxprev_usb_device_state.configure_state < current_usb_device_state.configure_state) {
maxprev_usb_device_state.configure_state = current_usb_device_state.configure_state;
}
current_usb_device_state = usb_device_state;
last_time = timer_read_fast();
Expand Down
2 changes: 1 addition & 1 deletion quantum/os_detection.h
Original file line number Diff line number Diff line change
Expand Up @@ -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(struct usb_device_state usb_device_state);

void os_detection_task(void);

Expand Down
14 changes: 8 additions & 6 deletions quantum/os_detection/tests/os_detection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -253,9 +253,11 @@ TEST_F(OsDetectionTest, TestDoNotReportIfUsbUnstable) {
EXPECT_EQ(detected_host_os(), OS_LINUX);
}

static struct usb_device_state 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();

Expand Down Expand Up @@ -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();

Expand All @@ -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();

Expand All @@ -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();

Expand All @@ -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();
Expand All @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion quantum/process_keycode/process_haptic.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
11 changes: 5 additions & 6 deletions tmk_core/protocol/chibios/chibios.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
24 changes: 9 additions & 15 deletions tmk_core/protocol/chibios/usb_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -168,6 +164,7 @@ void usb_event_queue_task(void) {
break;
case USB_EVENT_RESET:
usb_device_state_set_reset();
usb_device_state_set_protocol(USB_PROTOCOL_REPORT);
break;
default:
// Nothing to do, we don't handle it.
Expand Down Expand Up @@ -250,10 +247,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]);
}
}

Expand All @@ -269,7 +266,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;
Expand All @@ -292,12 +291,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;
Expand Down Expand Up @@ -396,11 +395,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.
Expand Down Expand Up @@ -458,7 +452,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);
Expand Down
3 changes: 0 additions & 3 deletions tmk_core/protocol/host.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading