Bluetooth API: add system and led state support

[dexter93]

expose hooks for drivers to pick up

Implementation currently doing nothing, it's up to the bt drivers to implement it

Implementation example: SonixQMK#292

Description

Types of Changes

• Core
• Bugfix
• New feature
• Enhancement/optimization
• Keyboard (addition or update)
• Keymap/layout/userspace (addition or update)
• Documentation

Issues Fixed or Closed by This PR

Checklist

• My code follows the code style of this project: C, Python
• I have read the PR Checklist document and have made the appropriate changes.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• I have tested the changes and verified that they work and don't break anything (as well as I can manage).

[dexter93]

remaining build failures seem to be ws2812 related in keymaps/userspace, not related to the PR

[ephb]

Is there any reason why this can not be merged or is only the review outstanding?

[fauxpark]

bluetooth_send_system() looks fine, the LED state stuff is a little trickier due to syncing issues.

Also, there's a merge conflict.

[dexter93]

PR rebased. Keyboard specifics removed, waiting on CI to pick up any failures and will follow up with patches

[dexter93]

bluetooth_send_system() looks fine, the LED state stuff is a little trickier due to syncing issues.

Also, there's a merge conflict.

syncing issues?

[fauxpark]

The LED state is not really a singular thing. Each keyboard interface has its own. This is not as much of an issue on Windows, where shift, lock and indicator state is shared across all connected keyboards - if you press Caps Lock, the host will tell all keyboards to turn the Caps Lock LED on, including both the 6KRO and NKRO interfaces of a QMK-based keyboard.

However macOS tracks this state separately for each connected device and interface. If you press Caps Lock, the LED turns on, but it is only for the 6KRO interface. If you then press NK_TOGG, the LED state will no longer match the OS, and even though the LED is on, typing will not give you uppercase letters. This problem extends to things like Bluetooth here.

[dexter93]

The LED state is not really a singular thing. Each keyboard interface has its own. This is not as much of an issue on Windows, where shift, lock and indicator state is shared across all connected keyboards - if you press Caps Lock, the host will tell all keyboards to turn the Caps Lock LED on, including both the 6KRO and NKRO interfaces of a QMK-based keyboard.

However macOS tracks this state separately for each connected device and interface. If you press Caps Lock, the LED turns on, but it is only for the 6KRO interface. If you then press NK_TOGG, the LED state will no longer match the OS, and even though the LED is on, typing will not give you uppercase letters. This problem extends to things like Bluetooth here.

I see, not surprised. However, that seems like a general issue not relevant to this PR. This one just enables state transmission over Bluetooth ( also, iirc most bluetooth HID modules do 6KRO - so, the syncing issue maybe not be triggered?)

[fauxpark]

( also, iirc most bluetooth HID modules do 6KRO - so, the syncing issue maybe not be triggered?)

Read what I said again. It's not about that.

If you have a QMK keyboard with NKRO and Bluetooth enabled, what you have, from macOS's point of view at least, is three different keyboards in a single physical device, which do not sync the LED state when you switch between 6KRO, NKRO and Bluetooth.

[sigprof]

Is there any actual problem with the LED state introduced here? led_task() gets invoked on every pass of the main loop; it will call host_keyboard_leds() and pick up any updates if the return value of where_to_send() had changed, so switching between USB and Bluetooth should not result in a stale LED state. The problem with separate LED states between 6KRO and NKRO might be real, but it's probably out of scope of this PR.

Also for Bluetooth another use case which may be considered here is when USB and Bluetooth are connected to completely different hosts.

[fauxpark]

Is there any actual problem with the LED state introduced here? led_task() gets invoked on every pass of the main loop; it will call host_keyboard_leds() and pick up any updates if the return value of where_to_send() had changed, so switching between USB and Bluetooth should not result in a stale LED state.

The host will only send LED updates when it needs to. It doesn't know when the keyboard switches between 6KRO/NKRO/Bluetooth/anything else.

[sigprof]

The host will only send LED updates when it needs to. It doesn't know when the keyboard switches between 6KRO/NKRO/Bluetooth/anything else.

The USB protocol code would maintain its keyboard_led_state variable internally even when the Bluetooth connection is selected, so there should not be any sync issues with that. The implementation of bluetooth_led_state() should also behave like that (if the BT connection is kept, the Bluetooth driver should still get the LED reports from the host and update its LED state accordingly, and if the connection is broken, the host would need to set the LED state again anyway). Then switching between the USB and Bluetooth output would immediately update the keyboard LEDs to either the USB or Bluetooth state.

This leaves the 6KRO/NKRO case, but that part may be handled inside the USB driver (or even the Bluetooth driver, if some Bluetooth implementation would have NKRO support).

[dexter93]

broken userspace fixed on keyboards/bioi, remaining build failures are AVR size related on spidey3 config, out of the context of this PR.

[fauxpark]

Merge conflicts.

Why don't we just get bluetooth_send_system() in for now, as that's more straightforward, and we can look at the LED state stuff after.

[dexter93]

Merge conflicts.

Why don't we just get bluetooth_send_system() in for now, as that's more straightforward, and we can look at the LED state stuff after.

The merge conflicts appear to be on layout/userspace - will drop those since they're not really relevant to the PR.
I don't see why the LED state should matter at this level honestly - given it's a Mac issue in general with multiple keyboard interfaces. Perhaps it would be more appropriate to figure out a way to tackle that behavior for every scenario - and not just block the bt one?

[github-actions]

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with bug, awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

[ephb]

Is the last word here that this is basically not wanted until the ```keyboard_led_state```` is reworked?

[fauxpark]

I'd prefer to get the bluetooth_send_system() part of this PR split out as that can go in immediately; I want to do some testing for the LED state stuff when I have time. That's the only thing "blocking" this PR.

[github-actions]

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with bug, awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

[github-actions]

Thank you for your contribution!
This pull request has been automatically closed because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, or re-open when it's ready.
// [stale-action-closed]