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

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

Merged
merged 1 commit into from
Jun 20, 2023

Conversation

xudongzheng
Copy link
Contributor

Fixes #1614

@xudongzheng xudongzheng force-pushed the central-params-pr branch 2 times, most recently from 215e355 to 892a3ae Compare May 19, 2023 23:00
@xudongzheng xudongzheng changed the title refactor(split): allow central to specify peripheral connection parameters refactor(split): allow central to define connection parameters May 19, 2023
Copy link
Contributor

@petejohanson petejohanson left a comment

Choose a reason for hiding this comment

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

Code seems reasonable. Has this tested well without any issues, in particular if you fail to flash both sides, and the peripheral still requests different params?

@petejohanson petejohanson self-assigned this May 20, 2023
@petejohanson petejohanson added enhancement New feature or request split labels May 20, 2023
@xudongzheng
Copy link
Contributor Author

In the event that the central is updated to include these options (and kept on the defaults) but peripheral continues running the old firmware, nothing really changes. It's the same as before except central is now getting the initial connection parameters from ZMK_SPLIT_BLE_PREF_* instead of having it hardcoded in central.c.

If the parameters are changed from the defaults on central but peripheral continues running the old firmware: central will initiate a connection with the new custom parameters but peripheral will request the old defaults. In theory central has the ability to reject that request but that seems like unnecessary code that will eventually serve no purpose.

@petejohanson
Copy link
Contributor

This will need rebasing now that #836 has been merged. There's also some CI failures that hopefully will be addressed when you do so. Thanks again.

@xudongzheng xudongzheng force-pushed the central-params-pr branch 2 times, most recently from 4d8e1a1 to 20c7e25 Compare June 6, 2023 03:40
Copy link
Contributor

@petejohanson petejohanson left a comment

Choose a reason for hiding this comment

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

Thanks!

@petejohanson petejohanson merged commit e686fce into zmkfirmware:main Jun 20, 2023
41 checks passed
@xudongzheng xudongzheng deleted the central-params-pr branch June 21, 2023 04:39
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.

Define split connection parameters on central instead of peripheral
2 participants