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

feat(split): Add peripheral is bonded/connected to central #2033

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ReFil
Copy link
Contributor

@ReFil ReFil commented Nov 22, 2023

For easy use by displays and indicators it would be helpful to be able to see if any peripheral at an index is connected and bonded, follow on from #2032

@ReFil ReFil requested a review from a team as a code owner November 22, 2023 15:32
@caksoylar
Copy link
Contributor

Nice, it might be useful for a single display (on central) setup on splits.

@caksoylar caksoylar added the enhancement New feature or request label Nov 23, 2023
@petejohanson petejohanson self-assigned this Nov 28, 2023
@caksoylar
Copy link
Contributor

Is there an existing event to hook up to in order to listen to peripheral changes? If not, might be a good opportunity to add one here.

@ReFil
Copy link
Contributor Author

ReFil commented Dec 22, 2023

There is but it doesn't work quite right. This gets addressed in #2036 as it's needed for that functionality

Comment on lines 10 to 11
bool zmk_split_bt_central_peripheral_is_bonded(uint8_t index);
Copy link
Contributor

Choose a reason for hiding this comment

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

Thoughts on a single API that returns an enum with the state? The "is connected" makes no sense if we don't even have an address bonded to that index, for example, so a single API to just query the "peripheral state" makes more sense, IMHO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that's a good idea in general, my only concern would be that's a divergence from how it's currently handled for central->host connections and peripheral->central connections which both use a pair of bool return functions. Would it be better to change all of them over to a consistent API with one BLE_CONN_STATE enum with connected, bonded but not connected and unbounded values?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thinking about this more i'm concerned changing the other functions would leak to breakage of module and config repo custom display implementations. A stopgap would be implement enum return, then change the bool return functions to reference that enum return function for some period of time then remove them, similar to how the encoder changes were handled

@ReFil ReFil force-pushed the central-is-bonded-connected branch from 62f3593 to f1017f3 Compare March 21, 2024 14:52
@ReFil ReFil force-pushed the central-is-bonded-connected branch from f1017f3 to 5047340 Compare June 18, 2024 17:54
For easy use by displays and indicators it would be helpful to be able to see if any peripheral at an index is connected and bonded
@ReFil ReFil force-pushed the central-is-bonded-connected branch from 5047340 to 5007f17 Compare September 26, 2024 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request split
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants