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

Bangle_setUI_Q3: tweaks (read below) #2571

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

thyttan
Copy link
Contributor

@thyttan thyttan commented Oct 15, 2024

  • Add custom handlers on top of the standard modes as well. Previously this was only possible for mode == "custom".
    • The goal here is to make it possible to move all input handling inside setUI where today some apps add on extra handlers outside of setUI calls.
  • Change the default behaviour of the hardware button to act immediately on press down. Previously it has been acting on button release.
    • This makes the interaction slightly snappier.
    • In addition to the existing btn key a new btnRelease key can now be specified. btnRelease will let you listen to the rising edge of the hardware button.

These are the same changes introduced as customized setui via espruino/BangleApps#3487, espruino/BangleApps#3491 and espruino/BangleApps#3563.

Forum thread: https://forum.espruino.com/conversations/397606/

Before merge:

  • update documentation
  • update the Bangle.js 1 implementation?

- Add custom handlers on top of the standard modes as well. Previously this was only possible for mode == "custom".
  - The goal here is to make it possible to move all input handling inside `setUI` where today some apps add on extra handlers outside of `setUI` calls.
- Change the default behaviour of the hardware button to act immediately on press down. Previously it has been acting on button release.
  - This makes the interaction slightly snappier.
  - In addition to the existing `btn` key a new `btnRelease` key can now be specified. `btnRelease` will let you listen to the rising edge of the hardware button.
@thyttan thyttan marked this pull request as draft October 15, 2024 21:00
@gfwilliams
Copy link
Member

Thanks! Yes, this looks good.

Do you have a Bangle.js 1 to test with? I'd be tempted not to mess with Bangle.js 1 now, but I guess if some apps are using setUI and expecting things to work a certain way then we may have to...

@thyttan
Copy link
Contributor Author

thyttan commented Oct 16, 2024

Do you have a Bangle.js 1 to test with? I'd be tempted not to mess with Bangle.js 1 now, but I guess if some apps are using setUI and expecting things to work a certain way then we may have to...

I don't have a Bangle.js 1 no.

Can we just refrain from modifying apps that support Bangle.js 1 (I haven't checked how many are relevant here) and change those over only when/if we get to tweaking the setUI_F18 implementation?

And add a note to the reference that the behaviour is slightly different across Bangle.js 1 & 2?

@thyttan
Copy link
Contributor Author

thyttan commented Oct 21, 2024

@gfwilliams if my last comment sounds like it works I can modify this PR to accommodate 🙂

@gfwilliams
Copy link
Member

Can we just refrain from modifying apps that support Bangle.js 1 (I haven't checked how many are relevant here) and change those over only when/if we get to tweaking the setUI_F18 implementation?

I'm not quite sure what you mean here. I feel like a lot of the 'core' apps target both, so we can't just update for one and not the other. So I guess something like Run or Messages might be an example here - it's designed for both, but it uses Layout which uses falling edge at the moment? So I guess there's a question of how they'll interact - like what if you bring up E.showMenu from Layout, then go back with the button (which happens on rising edge?). Now the Layout is showing but Layout is listening for the falling edge and gets called on the same button press?

So I think some of those apps will need changes, and then they will at least need testing on a Bangle.js 1

@thyttan
Copy link
Contributor Author

thyttan commented Oct 21, 2024

Can we just refrain from modifying apps that support Bangle.js 1 (I haven't checked how many are relevant here) and change those over only when/if we get to tweaking the setUI_F18 implementation?

I'm not quite sure what you mean here. I feel like a lot of the 'core' apps target both, so we can't just update for one and not the other.

Yes, so those would fall under "apps that support Bangle.js 1" in my thinking, and should not be modified yet.

... So I guess something like Run or Messages might be an example here - it's designed for both, but it uses Layout which uses falling edge at the moment?

EDIT: With setuichange messagesgui seems to always react on rising edge whether in menu, message, message list, etc. For run the button still acts on falling edge.

... So I guess there's a question of how they'll interact - like what if you bring up E.showMenu from Layout, then go back with the button (which happens on rising edge?). Now the Layout is showing but Layout is listening for the falling edge and gets called on the same button press?

Hm - I seem to not have stumbled upon that specific combo in my daily driving of setuichange. It sounds like a valid point though (without actually inspecting the code), and I could see that happening for the messagesgui app if setUI wasn't updated (as would be the case for Bangle.js 1).

As for the run app, I actually think we should not adapt it and just leave it to react on EDIT: FALLING edge. That makes sure a run is not accidentally started when the user just wanted to exit the app by reloading on long press. For that app I don't think the refactoring makes too much sense.

So I think some of those apps will need changes, and then they will at least need testing on a Bangle.js 1

Yes, lets hold off for a while then!

I'll comment here again if I reach the conclusion that the problems would actually not occur.

EDITS: per above

@thyttan
Copy link
Contributor Author

thyttan commented Oct 21, 2024

Edited to correct my writing above....

@gfwilliams
Copy link
Member

... and should not be modified yet.

If they don't need modifying to work with this then great - but I think obviously if they end up breaking then I don't think we have a choice.

Great news on messagesgui - I guess that uses back in Layout which calls straight to setUI

About the Run app, I think maybe that actually is a good reason why we should modify the Bangle.js1 setUI, if only to add btnRelease as an option (even if right now it does the exact same thing as btn). For run it's not needed as we use Layout and that uses setWatch directly, but it may be we hit other apps that rely on setUI(...btn) and we want to be able to change those to btnRelease to be explicit.

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.

2 participants