feat(split): allow central to connect to multiple peripherals #836

xudongzheng · 2021-06-12T16:57:08Z

This addresses #314 and #609.

This might also affect #281 and #718.

DuMaM · 2021-11-07T13:25:09Z

app/src/ble.c

@@ -337,15 +362,20 @@ static int ble_profiles_handle_set(const char *name, size_t len, settings_read_c
        }
    }
 #if IS_ENABLED(CONFIG_ZMK_SPLIT_BLE_ROLE_CENTRAL)
-    else if (settings_name_steq(name, "peripheral_address", &next) && !next) {


I think this line will solve multiple problems i have currently with my split setup

xudongzheng · 2022-07-04T15:25:03Z

Most likely some additional changes would be needed for settings reset. It's not clear how that should be implemented, perhaps just loop and delete ble/peripheral_addresses/[0,7] or just call nvs_clear() to handle peripheral addresses and everything else.

LSChyi · 2022-09-17T12:50:18Z

app/src/ble.c

+        // index and no additional action is necessary.
+        if (!bt_addr_le_cmp(&peripheral_addrs[i], addr)) {
+            return i;
+        }


What if the recorded peripheral appears after slot i+1, and takes ith slot?

I don't quite understand what i+1 you are referring to.

I wonder if the addr is actually recorded at some slot j, where j > i, but somehow the ith slot is open, so the addr takes away the ith slot as well.

This may be a rare case, and jth slot anyhow could be recycled for the addr should trigger disconnected function in normal cases.

Slot 0 is filled, then slot 1 is filled, and so on. There is no code to delete just a single ble/peripheral_addresses/%d setting so it's not possible to have i open and j occupied where i < j.

petejohanson

Couple minor items, but on a whole, LGTM.

app/src/split/bluetooth/central.c

petejohanson · 2022-10-02T03:02:15Z

app/src/split/bluetooth/central.c

-    err = bt_le_scan_start(BT_LE_SCAN_PASSIVE, split_central_device_found);
+    // Start scanning otherwise.
+    is_scanning = true;
+    int err = bt_le_scan_start(BT_LE_SCAN_PASSIVE, split_central_device_found);


This is fine for now, but ideally a future enhancement will add a backoff on scanning, since multi-peripheral setups are more likely to have one of them only on some of the time.

I'm exploring this,

bt_le_scan_start(&test_scan_params, split_central_device_found)

where,

static struct bt_le_scan_param test_scan_params = { .type = BT_LE_SCAN_TYPE_PASSIVE, .options = BT_LE_SCAN_OPT_FILTER_DUPLICATE, .interval = BT_GAP_SCAN_FAST_INTERVAL, .window = BT_GAP_SCAN_FAST_WINDOW, .timeout = 1500, .interval_coded = 0, .window_coded = 0, };

Setting any timeout > 0 will cause zmk to hang. .timeout = 0 will work just fine,

[00:00:00.770,935] <dbg> zmk: zmk_usb_get_conn_state: state: 1 [00:00:00.770,965] <dbg> zmk: get_selected_endpoint: No endpoints are ready. [00:00:01.081,146] <inf> usb_hid: Device configured [00:00:01.081,207] <dbg> zmk: zmk_usb_get_conn_state: state: 3 [00:00:01.081,237] <dbg> zmk: zmk_usb_get_conn_state: state: 3 [00:00:01.081,237] <dbg> zmk: get_selected_endpoint: Only USB is ready. [00:00:01.081,237] <dbg> zmk: zmk_endpoints_send_report: usage page 0x07 [00:00:01.081,268] <dbg> zmk: zmk_endpoints_send_report: usage page 0x0C [00:00:01.081,298] <inf> zmk: Endpoint changed: 0 [00:00:01.081,359] <dbg> zmk: destination_connection: Address pointer 0x20008fdf [00:00:01.081,359] <wrn> zmk: Not sending, no active address for current profile [00:00:01.081,390] <dbg> zmk: destination_connection: Address pointer 0x20008fdf [00:00:01.081,390] <wrn> zmk: Not sending, no active address for current profile

Any ideas? I was hoping to register scan callbacks where after the 15s timeout, scanning is restarted with new params with a longer interval and window and an indefinite timeout.

xudongzheng · 2022-10-04T05:31:23Z

@petejohanson

Is there a reason for this section of code https://github.com/zmkfirmware/zmk/blob/main/app/src/split/bluetooth/central.c#L403-L411? It's not clear to me why split_central_process_connection() would be called there as it's already called from the connect event handler.

I would consider getting rid of the lookup and having slicemk@44e0e7b#diff-18de66983adf7197deaf9e826c4087b486beb147e0ab19445d41a85ba7be9fe0R374-R384.

petejohanson · 2022-10-22T05:30:01Z

@petejohanson

Is there a reason for this section of code https://github.com/zmkfirmware/zmk/blob/main/app/src/split/bluetooth/central.c#L403-L411? It's not clear to me why split_central_process_connection() would be called there as it's already called from the connect event handler.

I would consider getting rid of the lookup and having slicemk@44e0e7b#diff-18de66983adf7197deaf9e826c4087b486beb147e0ab19445d41a85ba7be9fe0R374-R384.

IIRC, at least with Zephyr from several versions ago when I first wrote this, some times we would end up having an existing connection, after the scanning first discovers the advertising info, we and we needed to handle both scenarios:

No connect, create it.
Already connected, process the connection.

If we can test this thoroughly and see this no longer happening with the stack, we should simplify this, for sure.

petejohanson

Few final code nitpicks. I will try to get to testing this soon.

petejohanson · 2022-11-09T05:31:12Z

app/src/ble.c

-            LOG_ERR("Failed to handle peripheral address from settings (err %d)", err);
-            return err;
+        int i = atoi(next);
+        if (i >= ZMK_BLE_SPLIT_PERIPHERAL_COUNT) {


To be bullet proof here, should check for i < 0 as well.

app/src/split/bluetooth/central.c

xudongzheng · 2022-11-09T16:16:42Z

My suggestion is to only handle a new connection when the connected callback happens. This should happen exactly once per new connection.

If there is an existing connection, reserve_peripheral_slot() should fail because each peripheral will resolve to the same index each time - that's in zmk_ble_put_peripheral_addr().

Will have an updated PR shortly.

bezmi · 2022-11-10T01:54:39Z

I have been testing this (on the latest one as of this comment) and it's working well. @xudongzheng would you please consider pushing on top of the current commit instead of re-writing history with the force pushes? Makes it a little easier to explore the changes and can always be squashed when merging. Also, see my comment above RE scan timeout.

xudongzheng · 2022-11-10T03:01:26Z

@bezmi good point, you can see today's changes in d26b0ff on top of the earlier commit

I'm not familiar with the scan timeout. I'd consider explicitly stopping the scanning using a custom timer, which might be necessary regardless for maximum flexibility.

bezmi · 2023-05-04T02:47:50Z

@xudongzheng, I have been testing this PR on my daily driver split keyboard for pretty much as long as it has existed. I often run into a particular latency issue where keypresses from each half will arrive out of order, making it difficult to type quickly. It is very similar in description to #1633. Have you had any such issues and any success in troubleshooting them?

xudongzheng · 2023-05-04T05:48:48Z

@bezmi The first thing to try is placing the dongle right between the two keyboard halves. If it works fine there, it's likely a signal issue and not a firmware issue. You can bump the Bluetooth TX power as is typically recommended for ZMK.

petejohanson · 2023-05-29T05:42:42Z

@xudongzheng Can you please rebase now that your refactor #1815 is in? Code looks good w/ your latest push, will test here then merge, barring any problems appearing. Thanks!

petejohanson · 2023-06-01T06:19:42Z

@xudongzheng See the pre-commit check issue please on this.

xudongzheng · 2023-06-01T13:38:08Z

@petejohanson Oops, somehow missed that. The new commit seems to pass all the commit checks.

petejohanson

Tested on a pair of ZMK Uno/DK kits set up as splits. Works fine, and I know others have leveraged for a while for "dongle mode" as well.

This is a great first step toward fully supported BLE dongle mode, which will need still more docs, and build fixes to make doing so more seamless.

Thanks for sticking w/ this!

xudongzheng force-pushed the dongle-pr branch from d294a5e to a140045 Compare June 12, 2021 16:59

innovaker requested review from petejohanson and innovaker June 12, 2021 18:20

innovaker added the enhancement New feature or request label Jun 12, 2021

DuMaM reviewed Nov 7, 2021

View reviewed changes

bezmi mentioned this pull request May 2, 2022

feat(split): allow multiple peripherals #1282

Closed

xudongzheng force-pushed the dongle-pr branch from a140045 to 1d9a8bf Compare June 17, 2022 21:44

caksoylar mentioned this pull request Aug 5, 2022

Pure USB receiver dongle #1420

Closed

LSChyi reviewed Sep 17, 2022

View reviewed changes

petejohanson requested changes Oct 2, 2022

View reviewed changes

xudongzheng force-pushed the dongle-pr branch from 1d9a8bf to 22f0f5b Compare October 4, 2022 05:28

petejohanson requested changes Nov 9, 2022

View reviewed changes

xudongzheng force-pushed the dongle-pr branch 2 times, most recently from 3aa5b79 to 42d83f5 Compare November 9, 2022 16:25

xudongzheng force-pushed the dongle-pr branch from 42d83f5 to d26b0ff Compare November 10, 2022 02:58

Maksim-Isakau mentioned this pull request Nov 13, 2022

feat(battery): Split battery reporting over BLE GATT #1243

Closed

xudongzheng force-pushed the dongle-pr branch from 8ca9d3b to 3751612 Compare January 14, 2023 22:03

xudongzheng mentioned this pull request Jan 27, 2023

Peripheral connects to wrong central #1637

Closed

xudongzheng force-pushed the dongle-pr branch from 3751612 to f9c9c7c Compare April 9, 2023 21:38

xudongzheng mentioned this pull request Apr 25, 2023

Forward mouse events from USB over Bluetooth #1732

Open

xudongzheng force-pushed the dongle-pr branch from f9c9c7c to f90b614 Compare May 31, 2023 03:05

feat(split): allow central to connect to multiple peripherals

60ed8e2

xudongzheng force-pushed the dongle-pr branch from f90b614 to 60ed8e2 Compare June 1, 2023 12:31

petejohanson self-assigned this Jun 5, 2023

petejohanson added bluetooth Bluetooth related items split labels Jun 5, 2023

petejohanson approved these changes Jun 5, 2023

View reviewed changes

petejohanson merged commit 5d9ae8f into zmkfirmware:main Jun 5, 2023

petejohanson mentioned this pull request Jun 5, 2023

refactor(split): allow central to define connection parameters #1807

Merged

xudongzheng deleted the dongle-pr branch June 6, 2023 01:26

xudongzheng mentioned this pull request Jul 19, 2023

Bluetooth central does not validate peripheral MAC address #609

Closed

xudongzheng mentioned this pull request Sep 27, 2023

Add support for using external dongle with split keyboard #314

Closed

jjhaefner mentioned this pull request Nov 29, 2023

Create BLE-based "Dongle Mode" Documentation #2044

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(split): allow central to connect to multiple peripherals #836

feat(split): allow central to connect to multiple peripherals #836

xudongzheng commented Jun 12, 2021

DuMaM Nov 7, 2021

xudongzheng commented Jul 4, 2022

LSChyi Sep 17, 2022

xudongzheng Sep 17, 2022

LSChyi Sep 17, 2022

xudongzheng Sep 17, 2022 •

edited

Loading

petejohanson left a comment

petejohanson Oct 2, 2022

bezmi Nov 10, 2022 •

edited

Loading

xudongzheng commented Oct 4, 2022

petejohanson commented Oct 22, 2022

petejohanson left a comment

petejohanson Nov 9, 2022

xudongzheng commented Nov 9, 2022

bezmi commented Nov 10, 2022

xudongzheng commented Nov 10, 2022 •

edited

Loading

bezmi commented May 4, 2023

xudongzheng commented May 4, 2023

petejohanson commented May 29, 2023

petejohanson commented Jun 1, 2023

xudongzheng commented Jun 1, 2023

petejohanson left a comment

feat(split): allow central to connect to multiple peripherals #836

feat(split): allow central to connect to multiple peripherals #836

Conversation

xudongzheng commented Jun 12, 2021

DuMaM Nov 7, 2021

Choose a reason for hiding this comment

xudongzheng commented Jul 4, 2022

LSChyi Sep 17, 2022

Choose a reason for hiding this comment

xudongzheng Sep 17, 2022

Choose a reason for hiding this comment

LSChyi Sep 17, 2022

Choose a reason for hiding this comment

xudongzheng Sep 17, 2022 • edited Loading

Choose a reason for hiding this comment

petejohanson left a comment

Choose a reason for hiding this comment

petejohanson Oct 2, 2022

Choose a reason for hiding this comment

bezmi Nov 10, 2022 • edited Loading

Choose a reason for hiding this comment

xudongzheng commented Oct 4, 2022

petejohanson commented Oct 22, 2022

petejohanson left a comment

Choose a reason for hiding this comment

petejohanson Nov 9, 2022

Choose a reason for hiding this comment

xudongzheng commented Nov 9, 2022

bezmi commented Nov 10, 2022

xudongzheng commented Nov 10, 2022 • edited Loading

bezmi commented May 4, 2023

xudongzheng commented May 4, 2023

petejohanson commented May 29, 2023

petejohanson commented Jun 1, 2023

xudongzheng commented Jun 1, 2023

petejohanson left a comment

Choose a reason for hiding this comment

xudongzheng Sep 17, 2022 •

edited

Loading

bezmi Nov 10, 2022 •

edited

Loading

xudongzheng commented Nov 10, 2022 •

edited

Loading