feat(ble): add behavior to disconnect from BLE profile #1638
Conversation
|
I want to specify another motivation if I may: |
There was a problem hiding this comment.
One comment on the code. This also needs to update the BT behavior docs page to document the new command. See https://zmk.dev/docs/behaviors/bluetooth#bluetooth-command-defines
Adds new functionality and a behaviour to disconnect an active BLE connection. The motivation for this is that for some devices like phones, the presence of an active BLE connection results in the onscreen keyboard being selected.
chrisandreae
force-pushed
the
bt-disconnect-behavior
branch
from
fa28ab8 to
14767c0
Compare
docs/docs/behaviors/bluetooth.md
Outdated
| | `BT_NXT` | Switch to the next profile, cycling through to the first one when the end is reached. | | ||
| | `BT_PRV` | Switch to the previous profile, cycling through to the last one when the beginning is reached. | | ||
| | `BT_SEL` | Select the 0-indexed profile by number. Please note: this definition must include a number as an argument in the keymap to work correctly. eg. `BT_SEL 0` | | ||
| | `BT_DISCONNECT` | Disconnect from the 0-indexed profile by number, if it's currently connected and inactive. Please note: this definition must include a number as an | |
There was a problem hiding this comment.
The length of this define kind of sticks out from the other ones. What do you think about abbreviating it, e.g. BT_DIS or BT_DSC?
There was a problem hiding this comment.
I'm torn, because those abbreviations aren't especially transparent; you'd have to check the docs to interpret them. NXT, PRV, and SEL all don't have that problem, they're quite self-explanatory.
There was a problem hiding this comment.
I honestly think BT_DISC is descriptive enough, at least as descriptive as BT_SEL is, and as long as it's clearly documented, should be fine.
There was a problem hiding this comment.
Okay, BT_DISC works for me, I'll make the change
docs/docs/behaviors/bluetooth.md
Outdated
|
|
||
| An inactive connected profile can be explicitly disconnected using the `BT_DISCONNECT` behavior. This can be helpful in | ||
| cases when host devices behave differently when a bluetooth keyboard is connected, for example by hiding their on-screen | ||
| keyboard. |
There was a problem hiding this comment.
I might not have understood #1998 correctly, but does it imply that this shouldn't be used with the currently active profile when USB endpoint is active? If so, can we add a warning note on that here?
There was a problem hiding this comment.
Well, if the disconnected profile is the currently active one, it'll successfully disconnect but then immediately reconnect, because the currently active BT profile always advertises—even if the USB output is selected. #1998 is about a follow-up to make the pause in advertising when switching away from a BT profile consistent, whether that's switching to another BT profile or switching to USB.
I mentioned it to Pete, and he said he'd prefer to leave that to a follow-up because he has a BT refactor in the works that they'd interfere with, and to add an issue to keep track of it.
I don't have a strong opinion about whether the automatic re-connection should be documented explicitly here. If the advertising behavior is changed later, the documentation can always be updated to match.
There was a problem hiding this comment.
@chrisandreae I would document that expected behavior here for now, and we can update later as needed. Omitting it may just lead to some confusion on how this works today for the active profile.
There was a problem hiding this comment.
Sure, I've done that. Does this look OK?
Adds new functionality and a behavior to disconnect an active BLE connection. The motivation for this is that for some devices like phones, the presence of an active BLE connection results in the onscreen keyboard being selected.
This behavior allows the active connection for a given profile to be explicitly disconnected, by calling
bt_conn_disconnect(). The connection will be re-established (viaupdate_advertising()), when that profile is re-selected.I believe this behavior is complementary to the proposed automatic disconnection of non-selected devices. Automatic disconnection without a timeout could be frustrating to use when actively switching between devices, as there's a slight but noticeable delay on re-establishing the connnection. On the other hand if a timeout is used, there might be times when the user wishes to disconnect before the timeout has elapsed. This behaviour would permit that.
Questions:
In the case that
OUT_USBis selected, there is still a BLE profile which is considered active and selected, and is being advertised for. If the user tries to disconnect from that profile, it will be immediately re-connected from the advertisement. This would be awkward in the case that the user was wanting to switch between their computer via USB and their phone via BLE. I'm wondering if it would be most consistent to stop advertising for the selected BLE profile in the case thatOUT_USBis selected and the USB connection is present and active. This would make selecting the USB output more consistent with selecting a different BLE output.