-
-
Notifications
You must be signed in to change notification settings - Fork 953
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
Introduce a restricted clock-only-mode when not woken with the button #1359
base: main
Are you sure you want to change the base?
Conversation
Why is this necessary? Wouldn't this feature be useful regardless of the active app? |
The reasoning is that if you (for example) are playing the "2" game and the screen goes to sleep due to no activity, if you then wake up the screen by shaking your arm, touch input will be ignored and if you push the hardware button you will exit the game, loosing the progress. |
I think this is exactly how this should be done. The watch goes into a lock mode where the first button press does nothing but exiting that mode. Maybe we could have a popup with a lock symbol and something like "press the button to unlock" on touch events so users can better understand what is going on. Another thing is a lock screen. We could always show the watch face on non-button wakeup (with a clue that this is a lock screen) and after pressing the button go back to the current screen. This is closer to what other devices like phones do. |
I agree with @minacode although I don't think a real "lock screen" makes sense. Maybe a visual indicator but a lock screen for a watch that only has a couple of modes is a bit much. One reason I prefer it is because it enables you to have for example the navigation or timer app open on the watch and look at it from time to time. Right now this would always randomly switch to different watch faces on it's own. |
And because I know it will be mentioned: |
Thanks for the into @minacode and @Joheyy! I'll look into the possibility of implementing a "pop up" notifying about the lock and that the hardware button should be used to unlock. |
Keep in mind that the watch can wake itself (for example via notifications). |
I've just pushed a new commit that adds a 'Popup' notification and display it when a touch input is received during 'locked' wakeup. The lock is now active on all screens, though not when an app as caused the wakeup. |
That approach seems good, also very similar to that of say Apple Watch. |
I like it. Can you explain the intuition behind the three WokenBy values? Button explains itself, but what exactly are WakeUpAction and Other? What cases do and don't they cover? |
The WokenBy values are:
In reality it might be possible to replace the values with a simple boolean |
I would like that in a comment, in the code. Or better names, I don't know. UserButtonWakeup, UserNonButtonWakeup, SystemAutoWakeup? Sounds weird as well.. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be a good idea to move the popup to displayapp and use displayapp/Messages.h
to communicate between the systemtask and DisplayApp
Changed as suggested |
Build size and comparison to main:
|
Any updates on this? |
I've introduced a second page to the WakeUp settings since there was no room left for new checkbox. @NeroBurner Are you up for a re-review? |
The popup seems rather intrusive. If we are trying to avoid activity that occurs fairly frequently then it is possible that the popup will obscure the display when the user wants to view the watchface. Could we just use a lock icon instead? This is a user configured feature that would probably be disabled by default so the user must enable it and therefore shoudl understand the purpose of the icon. (It could be reinforced by showing the same icon in the settings screen.) The lock icon could show in a notification space, e.g. top of screen. It could either be always visible when locked, shown on activity or visible and highlight on activity. This may also reduce the memory increase. (One icon may be less than lots of text?) I really like the idea of a lock mechanism (as I too suffer the issue of unexpected activity) but don't want something obscuring the screen. |
I'm in favor of the lock as it doesn't need to be translated and should be easily understood. I like the visual hint to use the lock in the setting screen as well |
I also would love to see this feature added. I don't know how, but I constantly am waking the screen and accidentally tapping on screen elements. This would solve this issue for me! |
Please note that the lock screen first shows up after the screen has been woken up AND a touch gesture has been identified, meaning that the lock screen does not automatically show when the display is on.
This is a valid point, I'll change the lock screen to use an actual icon. Regarding using the lock in the in the settings, how do you envision this to be done? See the screenshot below for the current settings screen, shown is the (new) second settings page under "Wake Up". Where should the lock icon be placed?
If I understand you correctly, this would be nice, but it would require that InfiniTime has a global notification bar for it to be consisted across screens, which it don't, so I think I'll stick with the pop-up for now. |
@NeroBurner @riban-bw |
Could it also have an arrow pointing to the crown button to indicate how to
unlock?
…On Sat, Jan 27, 2024, 16:11 tgc-dk ***@***.***> wrote:
@NeroBurner <https://github.com/NeroBurner> @riban-bw
<https://github.com/riban-bw>
Using the popup with a lock icon could look like this:
InfiniSim_2024-01-27_220733.png (view on web)
<https://github.com/InfiniTimeOrg/InfiniTime/assets/1773050/ba6d0933-867f-452a-856f-93a528b4347e>
—
Reply to this email directly, view it on GitHub
<#1359 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAICFIIU5BA6YKZT2O2UPUDYQVUPHAVCNFSM6AAAAAAQ53RW2GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMJTGMZTMOJSGM>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
If you check the build logs, the difference in RAM usage is just 64B. It's just that infinitime is very close to the memory limit right now - keep the lock change but decrease the heap size a little |
Just to be sure... Would I be right in assuming this can be done by in |
|
Pushed the new changes. The simulator build still fails due to dependent on changes in InfiniTimeOrg/InfiniSim#131 |
Just for visuals a screenshot of how the lock overlay looks on the Analog Watch Face edit: oh you posted exactly the same screenshot already in #1359 (comment) 🙈 |
@tgc-dk I rebased your changes on current main, resolved merge conflicts, squashed commits down to 3 commits
The rebased branch can be found here: https://github.com/InfiniTimeOrg/InfiniTime/tree/button-unlock_neroburner |
Improve the lock and the code Squeeze in button unlock option in Wake Up settings Split the WakeUp settings into 2 pages Replace the text in the popup with a lock icon. Co-authored-by: NeroBurner <[email protected]>
Can I suggest lowering the heap size to 39 rather than 35? Ideally the heap should be as large as possible as long as it compiles |
Hi @NeroBurner |
bb5c06f
to
eecef5e
Compare
eecef5e
to
f0211e5
Compare
On GitHub the PR receiver can force push on the branch to be merged? Didn't expect that, but now I force pushed into your repo and overwrote the branch. I hope this is OK for you 😅 |
Is this ready for review? |
No worries :) |
I believe so 👍 |
I am testing if the RAM increase can be lessened by drawing the clock symbol using LVGL rectangles instead of a font symbol. Just a matter of time ⌛ 😁 |
That would be great, as fonts seem to be a ram sucker 👍 |
Got a lock symbol drawn with lvgl. Only two comparing f0211e5 (lock with font symbol) against 2a8e953 (lock with lvgl)
On For comparison the current main branch with the same HEAP reduction as this PR currently has
Which means with lvgl lock we still add 16B RAM and we reduce FLASH by 544 B ( ❓ ) |
This reverts commit 8f3b7b1.
I can revert the HEAP-size reduction now and we still stay in RAM bounds 🎉
vs current
|
@NeroBurner is this now ready for review? If so could you rebase onto main? |
To avoid random touches ends up changing settings or starting a timer, this introduces a restricted mode where no touch input is accepted unless the hardware button is used to turn on the watch. A settings is introduces to enable this.
The restricted mode is only active on watchfaces.
This is not a perfect solution since it will require the button to be pressed several times to leave the restricted mode once in it.