Host driver (wireless) rework, phase 1. #24365
Conversation
| #ifdef BLUETOOTH_ENABLE | ||
| if (where_to_send() == OUTPUT_BLUETOOTH) { | ||
| bluetooth_send_keyboard(report); | ||
| return; | ||
| } | ||
| #endif |
There was a problem hiding this comment.
Apparently this change (and other similar changes in this file) removes all usages of where_to_send(), and effectively removes the existing support for the OUTPUT_AUTO mode. Where would this logic go after the rework?
There was a problem hiding this comment.
OUTPUT_AUTO is indeterminate when you have a Bluetooth + 2.4GHz capable board. For that reason, it's been aliased to OUTPUT_NEXT which will cycle through the different connections instead.
There was a problem hiding this comment.
So you don't plan to support the current OUTPUT_AUTO behavior (when USB connection is detected, switch output to USB; when USB connection is dropped, switch back to BT), and would require some keypresses in this case to switch to the wired connection and back?
There was a problem hiding this comment.
That was indeed the intention, but I can see how useful of automatic connection swapping could be.
Keycodes 0.0.6 already have OUTPUT_AUTO removed (well, aliased to QK_OUTPUT_NEXT) but it's not yet merged to master -- I'll make a separate PR to reshuffle and reintroduce OUTPUT_AUTO so that it doesn't result in changes on master.
| .init = NULL, | ||
| .connect = NULL, | ||
| .disconnect = NULL, | ||
| .is_connected = NULL, |
There was a problem hiding this comment.
Should this use usb_connected_state()?
There was a problem hiding this comment.
Probably -- I wouldn't read too much into the current implementation right now; it's raised as a draft for visibility and discussion purposes. Nowhere near ready for merge.
| if (driver && host_get_driver() != driver) { | ||
| host_set_driver(driver); | ||
| set_output(OUTPUT_BLUETOOTH); | ||
| } |
There was a problem hiding this comment.
This probably would need to be refactored to a helper function, and do a bit more:
clear_keyboard(), so that keys are not left reported as held down when the output is off.- Something needs to be done with the NKRO state (many older implementations don't support NKRO over wireless connections). Although the proper way to handle that may be to add a
bool (*nkro_enabled)(void)function to the host driver interface, and change all non-USB code to use that function instead of thekeyboard_protocolglobal variable (the variable itself would still be needed inside the USB driver, but may becomestaticthere). In that case we may not need to touchkeymap_config.nkroin the output switching code (if the wireless connection does not support NKRO, it would look as if the host is using the boot keyboard protocol).
Also what would be the role of set_output() now — just something which triggers the set_output_user() hook? Or should we make set_output() actually switch the host driver, and have just a bunch of set_output() calls in this keycode processing function?
There was a problem hiding this comment.
Things like set_output_xxxx() were discussed on call well before your comment -- most of the plumbing regarding the connected outputs was to transition to a state machine with explicit callbacks whenever modes are changed. The current APIs are insufficient for what was discussed. clear_keyboard() et.al. would be part of the _quantum-level implementations here.
| #ifdef BLUETOOTH_ENABLE | ||
| if (where_to_send() == OUTPUT_BLUETOOTH) { | ||
| bluetooth_send_keyboard(report); | ||
| return; | ||
| } | ||
| #endif |
There was a problem hiding this comment.
So you don't plan to support the current OUTPUT_AUTO behavior (when USB connection is detected, switch output to USB; when USB connection is dropped, switch back to BT), and would require some keypresses in this case to switch to the wired connection and back?
| .init = NULL, | ||
| .connect = NULL, | ||
| .disconnect = NULL, | ||
| .is_connected = NULL, |
There was a problem hiding this comment.
My proposal is to move has_init_executed and is_connected flags and probably others not implemented yet into a state enum. Like it is already done for usb_device_state with callbacks for state changes. As the driver can hardly be connected without running init first.
Description
As per discussion on Discord. Draft raised for visibility, intent is to ensure we can swap around outputs dynamically.
Raw HID needs a new function added.
Types of Changes
Checklist