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

BLE advertising simplification #1122

Merged
merged 12 commits into from
Feb 19, 2025
7 changes: 3 additions & 4 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,9 @@ if(NOT CONFIG_DEVICE_ID)
endif()

project(uhk)
add_library(${PROJECT_NAME})
add_subdirectory(shared)
add_subdirectory(device/src)
add_subdirectory(right/src)
target_link_libraries(${PROJECT_NAME} PUBLIC
zephyr_interface
)
add_subdirectory(device/src)
add_subdirectory(right/src)
add_subdirectory(shared)
4 changes: 2 additions & 2 deletions device/CMakePresets.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
"BOARD": "uhk-80-right",
"BOARD_ROOT": "${sourceParentDir}/",
"mcuboot_OVERLAY_CONFIG": "${sourceDir}/child_image/mcuboot.conf;${sourceDir}/child_image/uhk-80-right.mcuboot.conf",
"EXTRA_CONF_FILE": "${sourceDir}/prj.conf.overlays/nrf_shared.conf;${sourceDir}/prj.conf.overlays/c2usb.conf;${sourceDir}/prj.conf.overlays/uhk-80.conf;${sourceDir}/prj.conf.overlays/ble_nus.conf;${sourceDir}/prj.conf.overlays/ble_nus_client.conf;${sourceDir}/prj.conf.overlays/ble_hid.conf"
"EXTRA_CONF_FILE": "${sourceDir}/prj.conf.overlays/nrf_shared.conf;${sourceDir}/prj.conf.overlays/c2usb.conf;${sourceDir}/prj.conf.overlays/uhk-80.conf;${sourceDir}/prj.conf.overlays/ble_nus.conf;${sourceDir}/prj.conf.overlays/ble_nus_client.conf;${sourceDir}/prj.conf.overlays/ble_hid.conf;${sourceDir}/prj.conf.overlays/right.conf"
}
},
{
Expand Down Expand Up @@ -74,4 +74,4 @@
}
}
]
}
}
6 changes: 2 additions & 4 deletions device/prj.conf.overlays/nrf_shared.conf
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,8 @@ CONFIG_BT_SMP=y

CONFIG_BT_L2CAP_TX_BUF_COUNT=5

# Todo: place these where they belong
# config for right half host switching
CONFIG_BT_MAX_CONN=4
CONFIG_BT_CTLR_SDC_PERIPHERAL_COUNT=3
CONFIG_BT_MAX_CONN=2
CONFIG_BT_CTLR_SDC_PERIPHERAL_COUNT=1
CONFIG_BT_FILTER_ACCEPT_LIST=y


Expand Down
3 changes: 3 additions & 0 deletions device/prj.conf.overlays/right.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# config for right half host switching
CONFIG_BT_MAX_CONN=4
CONFIG_BT_CTLR_SDC_PERIPHERAL_COUNT=3
162 changes: 102 additions & 60 deletions device/src/bt_advertise.c
Original file line number Diff line number Diff line change
@@ -1,21 +1,18 @@
#include "bt_advertise.h"
#include <bluetooth/services/nus.h>
#include <zephyr/bluetooth/gatt.h>
#include "attributes.h"
#include "bt_conn.h"
#include "connections.h"
#include "device.h"
#include "event_scheduler.h"
#include "bt_scan.h"

#undef DEVICE_NAME
#define DEVICE_NAME CONFIG_BT_DEVICE_NAME
#define DEVICE_NAME_LEN (sizeof(DEVICE_NAME) - 1)
#define LEN(NAME) (sizeof(NAME) - 1)

// Advertisement packets

#define AD_NUS_DATA \
#define AD_NUS_DATA(NAME) \
BT_DATA_BYTES(BT_DATA_FLAGS, (BT_LE_AD_GENERAL | BT_LE_AD_NO_BREDR)), \
BT_DATA(BT_DATA_NAME_COMPLETE, DEVICE_NAME, DEVICE_NAME_LEN),
BT_DATA(BT_DATA_NAME_COMPLETE, NAME, LEN(NAME)),

#define AD_HID_DATA \
BT_DATA_BYTES(BT_DATA_GAP_APPEARANCE, (CONFIG_BT_DEVICE_APPEARANCE >> 0) & 0xff, \
Expand All @@ -24,75 +21,95 @@
BT_DATA_BYTES(BT_DATA_UUID16_ALL, BT_UUID_16_ENCODE(BT_UUID_HIDS_VAL), \
BT_UUID_16_ENCODE(BT_UUID_BAS_VAL)),

static const struct bt_data adNus[] = {AD_NUS_DATA};

ATTR_UNUSED static const struct bt_data adNusLeft[] = {AD_NUS_DATA("UHK80 Left Nus")};
ATTR_UNUSED static const struct bt_data adNusRight[] = {AD_NUS_DATA("UHK 80 Right")};
static const struct bt_data adHid[] = {AD_HID_DATA};

static const struct bt_data adNusHid[] = {AD_NUS_DATA AD_HID_DATA};

// Scan response packets

#define SD_NUS_DATA BT_DATA_BYTES(BT_DATA_UUID128_ALL, BT_UUID_NUS_VAL),

#define SD_HID_DATA BT_DATA(BT_DATA_NAME_COMPLETE, DEVICE_NAME, DEVICE_NAME_LEN),
#define SD_HID_DATA(NAME) BT_DATA(BT_DATA_NAME_COMPLETE, NAME, LEN(NAME)),

static const struct bt_data sdNus[] = {SD_NUS_DATA};

static const struct bt_data sdHid[] = {SD_HID_DATA};

static const struct bt_data sdNusHid[] = {SD_NUS_DATA SD_HID_DATA};

static struct bt_le_adv_param advertisementParams[] = BT_LE_ADV_CONN;

static void setFilters() {
bt_le_filter_accept_list_clear();
if (DEVICE_IS_UHK80_RIGHT) {
if (BtConn_UnusedPeripheralConnectionCount() <= 1 && SelectedHostConnectionId != ConnectionId_Invalid) {
bt_le_filter_accept_list_add(&HostConnection(SelectedHostConnectionId)->bleAddress);
advertisementParams->options = BT_LE_ADV_OPT_FILTER_CONN;
} else {
advertisementParams->options = BT_LE_ADV_OPT_NONE;
}
static const struct bt_data sdHid[] = {SD_HID_DATA("UHK 80")};

#if DEVICE_IS_UHK80_RIGHT
#define BY_SIDE(LEFT, RIGHT) RIGHT
#else
#define BY_SIDE(LEFT, RIGHT) LEFT
#endif

#define BT_LE_ADV_START(PARAM, AD, SD) bt_le_adv_start(PARAM, AD, ARRAY_SIZE(AD), SD, ARRAY_SIZE(SD));

static const char * advertisingString(uint8_t advType) {
switch (advType) {
case ADVERTISE_NUS:
return "NUS";
case ADVERTISE_HID:
return "HID";
case ADVERTISE_NUS | ADVERTISE_HID:
return "HID \"and NUS\"";
case ADVERTISE_DIRECTED_NUS:
return "Directed NUS";
default:
return "None";
}
}

uint8_t BtAdvertise_Start(uint8_t adv_type)
uint8_t BtAdvertise_Start(adv_config_t advConfig)
{
setFilters();

int err;
const char *adv_type_string;
if (adv_type == (ADVERTISE_NUS | ADVERTISE_HID)) {
adv_type_string = "NUS and HID";
err = bt_le_adv_start(
BT_LE_ADV_CONN, adNusHid, ARRAY_SIZE(adNusHid), sdNusHid, ARRAY_SIZE(sdNusHid));
} else if (adv_type == ADVERTISE_NUS) {
adv_type_string = "NUS";
err = bt_le_adv_start(BT_LE_ADV_CONN, adNus, ARRAY_SIZE(adNus), sdNus, ARRAY_SIZE(sdNus));
} else if (adv_type == ADVERTISE_HID) {
adv_type_string = "HID";
err = bt_le_adv_start(BT_LE_ADV_CONN, adHid, ARRAY_SIZE(adHid), sdHid, ARRAY_SIZE(sdHid));
} else {
printk("Attempted to start advertising without any type! Ignoring.\n");
return 0;
int err = 0;

const char * advTypeString = advertisingString(advConfig.advType);

// Start advertising
static struct bt_le_adv_param advParam;
switch (advConfig.advType) {
case ADVERTISE_HID:
case ADVERTISE_NUS | ADVERTISE_HID:
/* our devices don't check service uuids, so hid advertisement effectively advertises nus too */
advParam = *BT_LE_ADV_CONN_ONE_TIME;
err = BT_LE_ADV_START(&advParam, adHid, sdHid);
break;
case ADVERTISE_NUS:
advParam = *BT_LE_ADV_CONN_ONE_TIME;

err = BT_LE_ADV_START(&advParam, BY_SIDE(adNusLeft, adNusRight), sdNus);
break;
case ADVERTISE_DIRECTED_NUS:
advParam = *BT_LE_ADV_CONN_ONE_TIME;
err = BT_LE_ADV_START(&advParam, BY_SIDE(adNusLeft, adNusRight), sdNus);
break;

//// TODO: fix and reenable this?
// printk("Advertising against %s\n", GetAddrString(advConfig.addr));
// advParam = *BT_LE_ADV_CONN_DIR_LOW_DUTY(advConfig.addr);
// advParam.options |= BT_LE_ADV_OPT_DIR_ADDR_RPA;
// err = BT_LE_ADV_START(&advParam, BY_SIDE(adNusLeft, adNusRight), sdNus);
break;
default:
printk("Adv: Attempted to start advertising without any type! Ignoring.\n");
return 0;
}

// Log it
if (err == 0) {
printk("%s advertising successfully started\n", adv_type_string);
printk("Adv: %s advertising successfully started\n", advTypeString);
return 0;
} else if (err == -EALREADY) {
printk("%s advertising continued\n", adv_type_string);
printk("Adv: %s advertising continued\n", advTypeString);
return 0;
} else {
printk("%s advertising failed to start (err %d), free connections: %d\n", adv_type_string, err, BtConn_UnusedPeripheralConnectionCount());
printk("Adv: %s advertising failed to start (err %d), free connections: %d\n", advTypeString, err, BtConn_UnusedPeripheralConnectionCount());
return err;
}
}

void BtAdvertise_Stop() {
void BtAdvertise_Stop(void) {
int err = bt_le_adv_stop();
if (err) {
printk("Advertising failed to stop (err %d)\n", err);
printk("Adv: Advertising failed to stop (err %d)\n", err);
}
}

Expand All @@ -106,26 +123,51 @@ static uint8_t connectedHidCount() {
return connectedHids;
}

uint8_t BtAdvertise_Type() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't look like something that should have been removed.

  • You are always trying to advertise, leading to errors when the peripheral connection count is reached. My code in turn tries to remedy the situation by increasingle aggressive strategies, including disconnecting current connections. The code was written so that uhk works even with CONFIG_BT_CTLR_SDC_PERIPHERAL_COUNT=1. With this settings and this code uhk will not work at all over bluetooth (dongle or hid).
  • Switchover scenario might need to be taken care of when the limit is reached.

How exactly does the current "simplified" advertising work on right half with respect to dongles? It broadcasts itself as a HID device, and dongles see this and try to connect despite there not being any advertisement for NUS?

I assume I should restore the necessary functionality? Will look into it tomorrow...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Only the content of the advertising is changed, the call sites of these methods control when advertising is started or stopped, I didn't change the logic there.
  2. Although I didn't test the changes with a dongle, I did check the scanning logic, and there I saw no filtering based on service UUIDs, only an address based filtering, that's why I was brave enough to remove it. My original idea of directed advertising when intending to connect to a dongle didn't work - according to mondalaci's feedback - so you are very welcome to take a look at this yourself, or to revert the changes for the right half and continue using HID / NUS advertising content.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Only the content of the advertising is changed, the call sites of these methods control when advertising is started or stopped, I didn't change the logic there.

The trouble is the BtAdvertise_Type() also decides whether or not to advertise at all. Anyways no worries, I will fix that shortly.

  1. I did check the scanning logic, and there I saw no filtering based on service UUIDs, only an address based filtering, that's why I was brave enough to remove it.

I see. I like the change, but this really needs to be mentioned in a comment somewhere ;-). (Will add that.)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, the compatibility is single directional.

When one ble HID is already connected we want to advertise NUS only - as we can't handle multiple ble HIDs at the same time. Unless you or @mondalaci have a better idea how to handle the situation that is.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kareltucek If it's possible to reuse the single available BLE HID connection upon connecting a new BLE HID device, ongoing BLE HID advertising would be preferred.

#define ADVERTISEMENT(TYPE) ((adv_config_t) { .advType = TYPE })
#define ADVERTISEMENT_DIRECT_NUS(ADDR) ((adv_config_t) { .advType = ADVERTISE_DIRECTED_NUS, .addr = ADDR })

adv_config_t BtAdvertise_Config() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(An additional reason to leave this in place is that it documents the scenarios that can happen, and makes logs more explicit - nevermind what advertisement params the backend actually ends up using.)

switch (DEVICE_ID) {
case DeviceId_Uhk80_Left:
return ADVERTISE_NUS;
if (Peers[PeerIdRight].conn == NULL) {
return ADVERTISEMENT_DIRECT_NUS(&Peers[PeerIdRight].addr);
} else {
return ADVERTISEMENT( 0 );
}

case DeviceId_Uhk80_Right:
if (BtConn_UnusedPeripheralConnectionCount() > 0) {
if (connectedHidCount() > 0) {
return ADVERTISE_NUS;
if (BtConn_UnusedPeripheralConnectionCount() <= 1 && SelectedHostConnectionId != ConnectionId_Invalid) {
/* we need to reserve last peripheral slot for a specific target */
connection_type_t selectedConnectionType = Connections_Type(SelectedHostConnectionId);
if (selectedConnectionType == ConnectionType_NusDongle) {
return ADVERTISEMENT_DIRECT_NUS(&HostConnection(SelectedHostConnectionId)->bleAddress);
} else if (selectedConnectionType == ConnectionType_BtHid) {
return ADVERTISEMENT(ADVERTISE_HID);
} else {
printk("Adv: Selected connection is neither BLE HID nor NUS. Can't advertise!");
return ADVERTISEMENT( 0 );
}
}
else if (connectedHidCount() > 0) {
/** we can't handle multiple HID connections, so don't advertise it when one HID is already connected */
return ADVERTISEMENT(ADVERTISE_NUS);
} else {
return ADVERTISE_NUS | ADVERTISE_HID;
/** we can connect both NUS and HID */
return ADVERTISEMENT(ADVERTISE_NUS | ADVERTISE_HID);
}
} else {
printk("Current slot count %d, not advertising\n", BtConn_UnusedPeripheralConnectionCount());
/** advertising needs a peripheral slot. When it is not free and we try to advertise, it will fail, and our code will try to
* disconnect other devices in order to restore proper function. */
printk("Adv: Current slot count is zero, not advertising!\n");
BtConn_ListCurrentConnections();
return 0;
return ADVERTISEMENT( 0 );
}
case DeviceId_Uhk_Dongle:
return 0;
return ADVERTISEMENT( 0 );
default:
printk("unknown device!\n");
return 0;
return ADVERTISEMENT( 0 );
}
}

17 changes: 14 additions & 3 deletions device/src/bt_advertise.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,27 @@
// Includes:

#include <stdint.h>
#include <zephyr/bluetooth/addr.h>
#include "shared/attributes.h"

// Macros:

#define ADVERTISE_NUS (1 << 0)
#define ADVERTISE_HID (1 << 1)
#define ADVERTISE_DIRECTED_NUS (1 << 2)

// Typedefs:

typedef struct {
uint8_t advType;
bt_addr_le_t* addr;
} ATTR_PACKED adv_config_t;


// Functions:

uint8_t BtAdvertise_Start(uint8_t adv_type);
void BtAdvertise_Stop();
uint8_t BtAdvertise_Type();
uint8_t BtAdvertise_Start(adv_config_t advConfig);
void BtAdvertise_Stop(void);
adv_config_t BtAdvertise_Config();

#endif // __BT_ADVERTISE_H__
6 changes: 5 additions & 1 deletion device/src/bt_conn.c
Original file line number Diff line number Diff line change
Expand Up @@ -358,17 +358,19 @@ static void connected(struct bt_conn *conn, uint8_t err) {

if (connectionId == ConnectionId_Invalid) {
connectUnknown(conn);
BtManager_StartScanningAndAdvertisingAsync();
} else {

if (isWanted(conn, connectionType)) {
bt_conn_set_security(conn, BT_SECURITY_L4);
// advertising/scanning needs to be started only after peers are assigned :-/
} else {
printk("Refusing connenction %d (this is not a selected connection)\n", connectionId);
bt_conn_disconnect(conn, BT_HCI_ERR_REMOTE_USER_TERM_CONN);
BtManager_StartScanningAndAdvertisingAsync();
}
}

BtManager_StartScanningAndAdvertisingAsync();


return;
Expand Down Expand Up @@ -455,6 +457,8 @@ static void connectAuthenticatedConnection(struct bt_conn *conn, connection_id_t
bt_conn_disconnect(conn, BT_HCI_ERR_REMOTE_USER_TERM_CONN);
break;
}

BtManager_StartScanningAndAdvertisingAsync();
}

static void securityChanged(struct bt_conn *conn, bt_security_t level, enum bt_security_err err) {
Expand Down
9 changes: 7 additions & 2 deletions device/src/bt_manager.c
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ void BtManager_StartBt() {
}

if (DEVICE_IS_UHK80_LEFT || DEVICE_IS_UHK80_RIGHT) {
BtAdvertise_Start(BtAdvertise_Type());
BtAdvertise_Start(BtAdvertise_Config());
}

if (DEVICE_IS_UHK80_RIGHT || DEVICE_IS_UHK_DONGLE) {
Expand Down Expand Up @@ -124,15 +124,20 @@ void BtManager_StartScanningAndAdvertising() {
printk("Starting %s, try %d!\n", label, try);
}

#ifdef CONFIG_BT_PERIPHERAL
if (leftShouldAdvertise || rightShouldAdvertise) {
err = BtAdvertise_Start(BtAdvertise_Type());
err = BtAdvertise_Start(BtAdvertise_Config());
success &= err == 0;
}
#endif

#ifdef CONFIG_BT_CENTRAL
if (rightShouldScan || dongleShouldScan) {
err = BtScan_Start();
success &= err == 0;
}
#endif


if (!success && try > 0) {
BtConn_DisconnectAll();
Expand Down
2 changes: 1 addition & 1 deletion device/src/bt_pair.c
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ void BtPair_PairPeripheral() {
pairingAsCentral = false;
Settings_Reload();
bt_le_oob_set_sc_flag(true);
BtAdvertise_Start(ADVERTISE_NUS);
BtAdvertise_Start((adv_config_t) { .advType = ADVERTISE_NUS });
printk ("Waiting for central to pair to me.\n");
EventScheduler_Reschedule(k_uptime_get_32() + PAIRING_TIMEOUT, EventSchedulerEvent_EndBtPairing, "Oob pairing timeout.");
}
Expand Down
2 changes: 1 addition & 1 deletion device/src/shell.c
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ void InitShell(void)
SHELL_CMD_ARG(gamepad, NULL, "switch gamepad on/off", cmd_uhk_gamepad, 1, 1),
SHELL_CMD_ARG(passkey, NULL, "send passkey for bluetooth pairing", cmd_uhk_passkey, 2, 0),
SHELL_CMD_ARG(btunpair, NULL, "unpair bluetooth devices", cmd_uhk_btunpair, 1, 1),
[10]=SHELL_CMD_ARG(connections, NULL, "list BLE connections", cmd_uhk_connections, 1, 0),
SHELL_CMD_ARG(connections, NULL, "list BLE connections", cmd_uhk_connections, 1, 0),
SHELL_SUBCMD_SET_END);

SHELL_CMD_REGISTER(uhk, &uhk_cmds, "UHK commands", NULL);
Expand Down
Loading