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

Change the Bluetooth API to use a driver framework #22783

Closed
wants to merge 6 commits into from

Conversation

silvinor
Copy link
Contributor

@silvinor silvinor commented Dec 30, 2023

Description

[ Redo of #22746, due to rebase error. ]

Convert the Bluetooth implementation to a RGB Matrix -esque driver.

No attempt has been made to change the 2 driver's code, however bluefruit_le.cpp has been renamed to bluefruit_le_spi.cpp, since there could be a bluefruit_le_uart.cpp in the future.

The old bluetoodth.c API has been replicated in the bluetooth_legacy.c file set in case keyboard makers have build their code on that basis.

Some keyboard dependencies existed, and these have been adjusted to the new code.

  • bioi/..
    • morgan65
    • g60ble
  • handwired/promethium
  • nek_type_a

The "Feture Bluetooth" doc page has also been rewritten to explain usage of the new driver based Bluetooth API.

It should be noted that the RN-42 module was based on BT2.1 that is now deprecated and withdrawn from the SIG and is no longer available. That driver code has not been modified and is left insitue for legacy use. (Newer HC-05 modules use a different module now, so references to the HC-05 have also been removed.)

Types of Changes

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

Issues Fixed or Closed by This PR

Not issues per-se, but these PR's are related

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
Copy link
Contributor

dexter93 commented Jan 15, 2024

looking good. Chances for a led_state= NULL option in the config?

@silvinor
Copy link
Contributor Author

silvinor commented Jan 17, 2024

@dexter93; Strongly doubt faux will adopt this. He did mention on Discord that he wants to redo the BlueFruit driver first... my guess is that that will then expose some deficiencies in the whole approach and he'll change the whole thing up ... I can think of a few just off the cuff.. e.g.

  • Bluetooth assumes batteries - so how do we abstract the battery service to get the battery level ... also assuming that one wants to maybe use a "power meter" IC and not just a ADC pin on the module.
  • Speaking of batteries - how do we make the KB go into low-power-mode (assuming that the specific used MCU support that, some don't), which is "easy", but then how do we wake it up. In the current code the loop just keeps looping ... and murders the battery. The fix is not in this driver ... it's a core consideration.
  • And how do we support tri-mode ... i.e. USB, BT and (something else) - like user-proprietary 2.4 GHz stacks, as example making use of a Nordic nRF52 dongle like this one - so that we can get better than the 90Hz polling rate that BT limits you to. (Which in itself brings a whole world of hurt when considering the security implications of transmitting unencrypted keycodes over a faster protocol.)
  • Do we build in a mechanism for flashing the BT module ... in case that needs updating? Can be done with RAW HID, but that take tip-toeing arround the VIA stuff.

None of the above is impossible - many co's who make KB's (even on QMK) do so already... just: How do you do it
in a versatile manner (i.e. supporting all the MCU's that QMK supports), and without breaking existing uses, or infringing on copyrights and licencing etc. etc.

@dexter93
Copy link
Contributor

@dexter93; Strongly doubt faux will adopt this. He did mention on Discord that he wants to redo the BlueFruit driver first... my guess is that that will then expose some deficiencies in the whole approach and he'll change the whole thing up ... I can think of a few just off the cuff.. e.g.

It's kinda sad to have a whole framework depend on a single device driver.. this one would at least open up the possibility of new/other ones. The BlueFruit driver is assumed to be in rework for.. ages at this point.

  • Bluetooth assumes batteries - so how do we abstract the battery service to get the battery level ... also assuming that one wants to maybe use a "power meter" IC and not just a ADC pin on the module.

Abstract a battery service reporter, make it modular enough to either use ADC ( thanks ChibiOS), or a value straight up from a power IC

  • Speaking of batteries - how do we make the KB go into low-power-mode (assuming that the specific used MCU support that, some don't), which is "easy", but then how do we wake it up. In the current code the loop just keeps looping ... and murders the battery. The fix is not in this driver ... it's a core consideration.

Currently, ChibiOS devices can make use of the CH_CFG_IDLE_ENTER_HOOK() and CH_CFG_IDLE_LEAVE_HOOK() for idle status and entering/leaving low-power states. Obviously this is MCU specific ( wakeup interrupts being the issue), but I have tested it and it works as a POC reliably.

  • And how do we support tri-mode ... i.e. USB, BT and (something else) - like user-proprietary 2.4 GHz stacks, as example making use of a Nordic nRF52 dongle like this one - so that we can get better than the 90Hz polling rate that BT limits you to. (Which in itself brings a whole world of hurt when considering the security implications of transmitting unencrypted keycodes over a faster protocol.)

tri-mode and other-mode: can manage it through outputselect. The issue with nRF and QMK is licencing conflicts ( although at some point it was discussed that blobs might be the answer there on a bring-your-own basis).

  • Do we build in a mechanism for flashing the BT module ... in case that needs updating? Can be done with RAW HID, but that take tip-toeing arround the VIA stuff.

That would assume the BT module advertises RAW HID, which most don't. Usually it's proprietary protocols for OTA flashing, or worse in the case of BCM chips, bluetool requiring heavily guarded .hex's. I don't see a reason to do so in the keyboard firmware.

None of the above is impossible - many co's who make KB's (even on QMK) do so already... just: How do you do it in a versatile manner (i.e. supporting all the MCU's that QMK supports), and without breaking existing uses, or infringing on copyrights and licencing etc. etc.

Sure, most of the above can be implemented. However, with the state of the project and the amount of core maintainers it would require significant effort/time. A PR like this one is a good candidate for a starting point, since it abstracts the driver selection. From then on, specifics and feature for each driver can be added in the future by said co's/mfg's or collaborators/contributors.

Copy link

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 github-actions bot added the stale Issues or pull requests that have become inactive without resolution. label Apr 11, 2024
Copy link

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]

@github-actions github-actions bot closed this May 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core documentation keyboard keymap stale Issues or pull requests that have become inactive without resolution.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants