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

nice!view: individual profile status #2265

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

snoyer
Copy link

@snoyer snoyer commented Apr 9, 2024

Allow the nice!view widget to display the status of each profile by:

  • not adding a circle around the number if the profile is not bound
  • drawing a dashed circle around the number if a profile is bound but not connected (device not live, explicitly disconnected with BT_DISC, ...)

Example:
niceview-profiles

  • solid circle: connected
  • dashed circle: not connected
  • no circle: not bound
  • filled circle: selected

This requires minor refactoring in BLE code to expose state methods by profile address rather than only by the current profile.

One issue I can see so far is that event's are indeed raised when the current profile changes but not when other profiles change from what I can tell (eg. a device other that the selected one disconnects).

I have tried addressing the BT_DISC case by adding a raise_profile_changed_event(); in zmk_ble_prof_disconnect() and that seems to work but I'm pretty sure that's not the proper way to do it (if there is a proper way to do it currently, see comment). Maybe that needs to be addressed separately from this PR?

@snoyer snoyer requested a review from a team as a code owner April 9, 2024 08:47
app/src/ble.c Outdated
Comment on lines 329 to 332
raise_profile_changed_event();
// `raise_profile_changed_event` actually does `raise_zmk_ble_active_profile_changed`
// should `profile_changed` and `active_profile_changed` be two separate events?

Copy link
Author

Choose a reason for hiding this comment

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

Suggested change
raise_profile_changed_event();
// `raise_profile_changed_event` actually does `raise_zmk_ble_active_profile_changed`
// should `profile_changed` and `active_profile_changed` be two separate events?

@petejohanson, should I remove this so that this PR is only the nice!view stuff and minimal core refactor?

That would leave the widget able to display the states correctly whenever it is currently refreshed (ie. on boot, when the active profile changes) but changes on other profiles would not be automatically shown (ie. BT_DISC, device disconnects on its own, ...).

Having the refresh happen automatically when any profile changes would be left to another PR to raise the appropriate events.

Copy link
Contributor

Choose a reason for hiding this comment

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

@snoyer Yeah, ideally we could review those two pieces independently.. Thanks!

Copy link
Author

Choose a reason for hiding this comment

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

Done. This PR should now only contain changes for the nice!view code to track and display the profiles individually, as well as minimal refactoring in ble.c and ble.h needed to access the profile states individually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants