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 changed event on central #2741

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

Conversation

ReFil
Copy link
Contributor

@ReFil ReFil commented Jan 2, 2025

Add a new event for when any peripheral connects or disconnects from the central

PR check-list

  • Branch has a clean commit history
  • Additional tests are included, if changing behaviors/core code that is testable.
  • Proper Copyright + License headers added to applicable files (Generally, we stick to "The ZMK Contributors" for copyrights to help avoid churn when files get edited)
  • Pre-commit used to check formatting of files, commit messages, etc.
  • Includes any necessary documentation changes.

@ReFil ReFil requested a review from a team as a code owner January 2, 2025 10:26
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 for working on this! A few thoughts.

Comment on lines 98 to 101
if (CONFIG_ZMK_SPLIT AND (NOT CONFIG_ZMK_SPLIT_ROLE_CENTRAL))
target_sources(app PRIVATE src/events/split_peripheral_status_changed.c)
endif()
target_sources_ifdef(CONFIG_ZMK_SPLIT_ROLE_CENTRAL app PRIVATE src/events/split_central_peripheral_status_changed.c)
Copy link
Contributor

Choose a reason for hiding this comment

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

Honestly, it'd be clearer if using just conditional branches for both, IMHO:

Suggested change
if (CONFIG_ZMK_SPLIT AND (NOT CONFIG_ZMK_SPLIT_ROLE_CENTRAL))
target_sources(app PRIVATE src/events/split_peripheral_status_changed.c)
endif()
target_sources_ifdef(CONFIG_ZMK_SPLIT_ROLE_CENTRAL app PRIVATE src/events/split_central_peripheral_status_changed.c)
if (CONFIG_ZMK_SPLIT)
if(CONFIG_ZMK_SPLIT_ROLE_CENTRAL)
target_sources(app PRIVATE src/events/split_central_peripheral_status_changed.c)
else()
target_sources(app PRIVATE src/events/split_peripheral_status_changed.c)
endif()
endif()

app/include/zmk/split/bluetooth/central.h Outdated Show resolved Hide resolved
@ReFil ReFil force-pushed the peripheral_status_changed branch from 1d433f6 to dff7aaf Compare January 7, 2025 13:03
Add a new event for when any peripheral connects or disconnects from the central
@ReFil ReFil force-pushed the peripheral_status_changed branch from dff7aaf to 1a0b051 Compare January 7, 2025 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants