zmk icon indicating copy to clipboard operation
zmk copied to clipboard

nice!view: individual profile status

Open snoyer opened this issue 1 year ago • 10 comments

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 avatar Apr 09 '24 08:04 snoyer

@nmunnich thanks for the interest, I don't currently have a dev environment (let alone the hardware) to rebase and test/validate so I just eyeballed the small merge conflict on the off chance it's enough

snoyer avatar Mar 30 '25 13:03 snoyer

Ok, if you could adjust the commits (https://zmk.dev/docs/development/contributing/pull-requests) I'll see if I can find someone with a nice!view willing to validate.

nmunnich avatar Mar 30 '25 13:03 nmunnich

@nmunnich I'm not set up to do that right now so it will have to wait... Alternatively, feel free to edit the PR if you've got write access and want to move faster than I can :)

snoyer avatar Mar 30 '25 13:03 snoyer

Sure, no rush. Give me a poke when you've got it all setup nicely.

nmunnich avatar Mar 30 '25 13:03 nmunnich

@nmunnich I'm back at a proper computer but I'm not sure how to proceed...

According to the guidelines you linked I think the commit message should be something like

feat(display): nice!view individual profile status

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

but am I supposed to rewrite this whole PR's history so it's a single commit? only edit the commit message of the first commit? just add a last commit so the message is there for a maintainer to use when squash-merging the PR?

snoyer avatar Apr 05 '25 11:04 snoyer

It would be easiest for us if you could squash all the commits with the commit message you wrote (which is great, by the way!). I was reminded that we can do the squashing as well so it wouldn't be mandatory in this case, since the PR was opened before we added the PR standards, but it would be easier and nicer for us :)

nmunnich avatar Apr 05 '25 11:04 nmunnich

TBH I was hoping I could wriggle out of having to do the squash myself as I'm not super comfortable with git and was going to pull out the "but this PR predates the guidelines" argument next :)

snoyer avatar Apr 05 '25 12:04 snoyer

You can wriggle out if you want, or you can treat this as a learning opportunity! The way I'd do this (in the command line) would be git rebase -i HEAD~3 make sure the first commit is a pick and mark the other two as s for squash, then once you've saved that the editor should reopen allowing you to edit the commit message. Once that's all done you just git push --force-with-lease

nmunnich avatar Apr 05 '25 12:04 nmunnich

branch running on nice_nano_v2 board with boardsource3x4 nice_view_adapter nice_view shield

nv-profiles

snoyer avatar Apr 20 '25 16:04 snoyer

32ffdc34a3157ec78aeb816b1a254c7 It works on sofle with nice nano.

SixPiinGit avatar May 23 '25 13:05 SixPiinGit

The necessary core changes have been merged separately in #2993 so this PR only affect the nice!view widgets code now.

Could @Nicell and @petejohanson please have a look and decide whether this belong in main or if I should close it?

snoyer avatar Jul 21 '25 07:07 snoyer

Now that the changes to core have been made and all the changes left are in the shield itself, I'm happy to merge this as Nicell has already given his approval. Could you just confirm for me that you re-tested to make sure everything is working as it was before, and then I'll merge?

nmunnich avatar Jul 21 '25 15:07 nmunnich

Could you just confirm for me that you re-tested to make sure everything is working as it was before, and then I'll merge?

I'm not daily-driving it but it's still running the same as far as I can tell, see picture with a nice!nano v2 and custom PCB: nv

I'd rather confirm with fully in-tree supported hardware (e.g. boardsource3x4 shield as in a previous comment) but I don't have any on hand currently.

edit: actually my boardsource3x4 would use the same bodge as the custom PCB in the above picture so I don't know if it would be any better of a test. I don't own anything that has nice!view support out of the box

snoyer avatar Jul 21 '25 17:07 snoyer