-
Notifications
You must be signed in to change notification settings - Fork 71
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
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
363328d
bugfix for cmake versions.c generation
benedekkupper a0dfb6d
shell: fix missing commands
benedekkupper 5800ef3
bt: left uses directed advertising, right advertises HID only
benedekkupper ee1bca1
Restore the ble advertising type computation.
kareltucek 4e4e265
Use normal advertising as directed doesn't work.
kareltucek 1e05cf3
Aesthetic refactors.
kareltucek 051e7ea
Fix dongle leds.
kareltucek 90cd19e
Fix left side peripheral count.
kareltucek 4524f08
Name advertisements better and fix the left advertisement conditions.
kareltucek 3a20b69
Refactor advertising code...
kareltucek 454c606
Rename the advertisements to be less misleading.
kareltucek 7d17948
Merge remote-tracking branch 'origin/master' into ble-advertising-fix
kareltucek File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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, \ | ||
|
@@ -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); | ||
} | ||
} | ||
|
||
|
@@ -106,26 +123,51 @@ static uint8_t connectedHidCount() { | |
return connectedHids; | ||
} | ||
|
||
uint8_t BtAdvertise_Type() { | ||
#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() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 ); | ||
} | ||
} | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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.
CONFIG_BT_CTLR_SDC_PERIPHERAL_COUNT=1
. With this settings and this code uhk will not work at all over bluetooth (dongle or hid).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...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The trouble is the BtAdvertise_Type() also decides whether or not to advertise at all. Anyways no worries, I will fix that shortly.
I see. I like the change, but this really needs to be mentioned in a comment somewhere ;-). (Will add that.)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.