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

[Feature Request] Custom Bluetooth driver #20233

Closed
2 of 4 tasks
Goshin opened this issue Mar 22, 2023 · 4 comments
Closed
2 of 4 tasks

[Feature Request] Custom Bluetooth driver #20233

Goshin opened this issue Mar 22, 2023 · 4 comments
Labels
enhancement help wanted stale Issues or pull requests that have become inactive without resolution.

Comments

@Goshin
Copy link

Goshin commented Mar 22, 2023

Feature Request Type

  • Core functionality
  • Add-on hardware support (eg. audio, RGB, OLED screen, etc.)
  • Alteration (enhancement/optimization) of existing feature(s)
  • New behavior

Description

Currently, BLUETOOTH_DRIVER can be set to either BluefruitLE, RN42, orcustom. For custom I suppose it, similar to other drivers, would allow developers to implement their own communication with other Bluetooth accessory boards in their keyboards/keymaps.

BLUETOOTH_ENABLE ?= no
VALID_BLUETOOTH_DRIVER_TYPES := BluefruitLE RN42 custom
ifeq ($(strip $(BLUETOOTH_ENABLE)), yes)
ifeq ($(filter $(strip $(BLUETOOTH_DRIVER)),$(VALID_BLUETOOTH_DRIVER_TYPES)),)
$(call CATASTROPHIC_ERROR,Invalid BLUETOOTH_DRIVER,BLUETOOTH_DRIVER="$(BLUETOOTH_DRIVER)" is not a valid Bluetooth driver type)
endif
OPT_DEFS += -DBLUETOOTH_ENABLE
NO_USB_STARTUP_CHECK := yes
COMMON_VPATH += $(DRIVER_PATH)/bluetooth
SRC += outputselect.c bluetooth.c
ifeq ($(strip $(BLUETOOTH_DRIVER)), BluefruitLE)
OPT_DEFS += -DBLUETOOTH_BLUEFRUIT_LE -DHAL_USE_SPI=TRUE
SRC += $(DRIVER_PATH)/bluetooth/bluefruit_le.cpp
QUANTUM_LIB_SRC += analog.c
QUANTUM_LIB_SRC += spi_master.c
endif
ifeq ($(strip $(BLUETOOTH_DRIVER)), RN42)
OPT_DEFS += -DBLUETOOTH_RN42 -DHAL_USE_SERIAL=TRUE
SRC += $(DRIVER_PATH)/bluetooth/rn42.c
QUANTUM_LIB_SRC += uart.c
endif
endif

However, the current implementation only considers the embedded drivers.

void bluetooth_send_keyboard(report_keyboard_t *report) {
#if defined(BLUETOOTH_BLUEFRUIT_LE)
bluefruit_le_send_keyboard(report);
#elif defined(BLUETOOTH_RN42)
rn42_send_keyboard(report);
#endif
}

So I suggest we define the Bluetooth-related functions in drivers/bluetooth/bluetooth.c as weak symbols, which allows developers to override these functions for their keyboards/keymaps.

@@ -23,7 +23,7 @@
 #    include "rn42.h"
 #endif

-void bluetooth_init(void) {
+__attribute__((weak)) void bluetooth_init(void) {
 #if defined(BLUETOOTH_BLUEFRUIT_LE)
     bluefruit_le_init();
 #elif defined(BLUETOOTH_RN42)
@@ -31,13 +31,13 @@ void bluetooth_init(void) {
 #endif
 }

-void bluetooth_task(void) {
+__attribute__((weak)) void bluetooth_task(void) {
 #if defined(BLUETOOTH_BLUEFRUIT_LE)
     bluefruit_le_task();
 #endif
 }

-void bluetooth_send_keyboard(report_keyboard_t *report) {
+__attribute__((weak)) void bluetooth_send_keyboard(report_keyboard_t *report) {
 #if defined(BLUETOOTH_BLUEFRUIT_LE)
     bluefruit_le_send_keyboard(report);

...

Alternatively, we can exclude drivers/bluetooth/bluetooth.c in compiling whenBLUETOOTH_DRIVER is 'custom'. In this way, developers would have to provide their own implementations for all the Bluetooth functions.

diff --git a/builddefs/common_features.mk b/builddefs/common_features.mk
index d9130b5338..9ef926f5a4 100644
@@ -884,7 +884,10 @@ ifeq ($(strip $(BLUETOOTH_ENABLE)), yes)
     OPT_DEFS += -DBLUETOOTH_ENABLE
     NO_USB_STARTUP_CHECK := yes
     COMMON_VPATH += $(DRIVER_PATH)/bluetooth
-    SRC += outputselect.c bluetooth.c
+    SRC += outputselect.c
+    ifneq ($(strip $(BLUETOOTH_DRIVER)), custom)
+        SRC += bluetooth.c
+    endif

     ifeq ($(strip $(BLUETOOTH_DRIVER)), BluefruitLE)
         OPT_DEFS += -DBLUETOOTH_BLUEFRUIT_LE -DHAL_USE_SPI=TRUE 

Other changes that we might need to make:

  • Documentation
  • For a more consistent style, make the default implementations blank and let the drivers override them.
@drashna
Copy link
Member

drashna commented Mar 23, 2023

@fauxpark thoughts?

@fauxpark
Copy link
Member

The Bluetooth API has not been fully solidified, this will be sorted out some time in the future.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had activity in the last 90 days. It will be closed in the next 30 days unless it is tagged properly or other activity occurs.
For maintainers: Please label with bug, in progress, on hold, discussion or to do 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 Jun 24, 2023
@fauxpark
Copy link
Member

This has been somewhat resolved as of #20897

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement help wanted stale Issues or pull requests that have become inactive without resolution.
Projects
None yet
Development

No branches or pull requests

3 participants