Introduce a restricted clock-only-mode when not woken with the button #1359

tgc-dk · 2022-10-05T19:38:43Z

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.

minacode · 2022-10-06T23:16:15Z

The restricted mode is only active on watchfaces.

Why is this necessary? Wouldn't this feature be useful regardless of the active app?

tgc-dk · 2022-10-07T08:00:05Z

The restricted mode is only active on watchfaces.

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.
One way to solve this is of course to make the first hardware button push unlock the restricted mode, but IMHO this would probably be a bit confusing for users since it would change how the button-push is handled depending on how the wakeup was done...

minacode · 2022-10-07T11:53:18Z

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.

taukakao · 2022-10-07T23:34:08Z

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.

taukakao · 2022-10-07T23:37:32Z

And because I know it will be mentioned:
No, I don't think this is a weird workaround for the issue of a too sensitive touch panel.
This is for when you currently don't want to interact with your watch but still look at it.

tgc-dk · 2022-10-09T20:48:46Z

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.

minacode · 2022-10-11T10:23:04Z

Keep in mind that the watch can wake itself (for example via notifications).

tgc-dk · 2022-10-16T19:57:46Z

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.
I have only tested this in the simulator so far...
Let me know what you think!
I've attached a small animation that shows the lock in action.

Avamander · 2022-10-16T22:30:04Z

That approach seems good, also very similar to that of say Apple Watch.

minacode · 2022-10-16T23:35:05Z

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?

tgc-dk · 2022-10-17T13:13:58Z

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:

Button: Pretty self explanatory as you say)
WakeUpAction: The actions selected in the wakeup settings, single/double tap, raise, shake
Other: Other things that can wake the watch up, basically apps and notifications.

In reality it might be possible to replace the values with a simple boolean wokenByAction, but I'm not yet sure...

minacode · 2022-10-17T13:23:40Z

I would like that in a comment, in the code. Or better names, I don't know. UserButtonWakeup, UserNonButtonWakeup, SystemAutoWakeup? Sounds weird as well..

NeroBurner

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

src/systemtask/SystemTask.cpp

src/displayapp/widgets/PopupMessage.cpp

src/displayapp/screens/settings/SettingWakeUp.cpp

src/displayapp/widgets/PopupMessage.h

tgc-dk · 2022-11-07T21:23:05Z

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

github-actions · 2023-05-02T19:31:40Z

Build size and comparison to main:

Section	Size	Difference
text	375680B	1152B
data	948B	0B
bss	63504B	16B

pipe01 · 2023-10-26T12:25:12Z

Any updates on this?

tgc-dk · 2023-11-28T13:51:45Z

I've introduced a second page to the WakeUp settings since there was no room left for new checkbox.
Note that the CI simulator build fails due to changes needed in InfiniSim to build with the changes in this PR, see InfiniTimeOrg/InfiniSim#131

@NeroBurner Are you up for a re-review?

riban-bw · 2024-01-19T06:44:09Z

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. I have only tested this in the simulator so far... Let me know what you think! I've attached a small animation that shows the lock in action.

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.

NeroBurner · 2024-01-19T22:32:33Z

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

jgonyea · 2024-01-25T05:23:35Z

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!

tgc-dk · 2024-01-27T17:19:28Z

@riban-bw

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.

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.

@riban-bw @NeroBurner

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

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?

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.

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.

tgc-dk · 2024-01-27T21:11:03Z

@NeroBurner @riban-bw
Using the popup with a lock icon could look like this:

jgonyea · 2024-01-27T22:34:10Z

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: ***@***.***>

tgc-dk · 2024-01-28T21:12:40Z

@NeroBurner
Turns out the new lock icon takes up to much spaces so linking fails. See https://github.com/InfiniTimeOrg/InfiniTime/actions/runs/7688311260/job/20949380052

So I've reverted the icon changes...

Suggestions for how to proceed?

minacode · 2024-01-28T23:01:28Z

Could you use an external font for the icon?
Or a smaller icon? 😄
Or draw the lock manually with LVGL somehow?

mark9064 · 2024-01-28T23:51:59Z

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

tgc-dk · 2024-01-29T11:52:35Z

@mark9064

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 src/FreeRTOSConfig.h changing configTOTAL_HEAP_SIZE from (1024 * 40) to (1024 * 39) ?

minacode · 2024-01-29T18:34:22Z

(1024 * 35) was recommended before by JF to me for the calculator app.

tgc-dk · 2024-01-29T20:41:13Z

Pushed the new changes. The simulator build still fails due to dependent on changes in InfiniTimeOrg/InfiniSim#131

src/touchhandler/TouchHandler.h

NeroBurner · 2024-09-16T20:46:04Z

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) 🙈

src/touchhandler/TouchHandler.h

NeroBurner · 2024-09-30T20:37:12Z

@tgc-dk I rebased your changes on current main, resolved merge conflicts, squashed commits down to 3 commits

first commit all your changes rebased and squashed
second commit reducing heap size
third default buttonUnlocksOn to false to make it work with current InfiniSim

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]>

mark9064 · 2024-09-30T21:30:00Z

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

tgc-dk · 2024-10-03T08:37:28Z

@tgc-dk I rebased your changes on current main, resolved merge conflicts, squashed commits down to 3 commits
* first commit all your changes rebased and squashed

* second commit reducing heap size

* third default buttonUnlocksOn to false to make it work with current InfiniSim
The rebased branch can be found here: https://github.com/InfiniTimeOrg/InfiniTime/tree/button-unlock_neroburner

Hi @NeroBurner
I've commited you're suggested change, but I currently do not have the "bandwidth" to do much more... Feel free to create a PR based on your branch!
I will probably get some more time later in the year, so I can probably do the merge and resolve conflicts then, but I don't see any reason to wait...

NeroBurner · 2024-10-03T20:47:53Z

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 😅

mark9064 · 2024-10-05T22:28:51Z

Is this ready for review?

tgc-dk · 2024-10-06T13:55:34Z

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 😅

No worries :)

tgc-dk · 2024-10-06T13:56:10Z

Is this ready for review?

I believe so 👍

NeroBurner · 2024-10-06T14:31:07Z

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 ⌛ 😁

mark9064 · 2024-10-06T15:38:40Z

That would be great, as fonts seem to be a ram sucker 👍

NeroBurner · 2024-10-06T18:53:12Z

Got a lock symbol drawn with lvgl. Only two lv_obj needed (in addition to the pop-up background lv_obj). Not pretty, but help welcome :D

comparing f0211e5 (lock with font symbol) against 2a8e953 (lock with lvgl)

lock with font:
- default config:
  - FLASH: 376024 B -- 78.22%
  - RAM: 58KB -- 90.62% (auto formatted to KB, don't know how to get B)
- WatchFace::Terminal
  - FLASH: 300028 B -- 63.21%
  - RAM: 59184 B -- 90.31%
- full analysis: build_firmware_memory_2024-10-06_button_unlock_font.txt
lock with lvgl:
- default config:
  - FLASH: 375700 B -- 79.16%
  - RAM: 59336 B -- 90.54%
- WatchFace::Terminal
  - FLASH: 299688 B -- 63.14%
  - RAM: 59120 B -- 90.21%
- full analysis: build_firmware_memory_2024-10-06_button_unlock_lvgl.txt

On WatchFace::Terminal we're saving 340B FLASH and 64B of RAM when drawing with LVGL instead of using the font (this is a 0.1% reduction in FLASH)

For comparison the current main branch with the same HEAP reduction as this PR currently has (1024 * 35) we have the following numbers.

main with HEAP size 35KB:
- default config:
  - FLASH: 374548 B -- 78.91%
  - RAM: 59320 B -- 90.52 %
- WatchFace::Terminal
  - FLASH: 300232 B -- 63.26%
  - RAM: 59104 B -- 90.19%

Which means with lvgl lock we still add 16B RAM and we reduce FLASH by 544 B ( ❓ )

This reverts commit 8f3b7b1.

NeroBurner · 2024-10-06T18:56:51Z

I can revert the HEAP-size reduction now and we still stay in RAM bounds 🎉

all default Apps/WatchFaces
USERAPP_TYPES:STRING=Apps::StopWatch, Apps::Alarm, Apps::Timer, Apps::Steps, Apps::HeartRate, Apps::Music, Apps::Paint, Apps::Paddle, Apps::Twos, Apps::Dice, Apps::Metronome, Apps::Navigation, Apps::Weather
WATCHFACE_TYPES:STRING=WatchFace::Digital, WatchFace::Analog, WatchFace::PineTimeStyle, WatchFace::Terminal, WatchFace::Infineat, WatchFace::CasioStyleG7710
           FLASH:      375700 B     474632 B     79.16%
             RAM:       64456 B        64 KB     98.35%
WatchFace::Terminal
           FLASH:      299688 B     474632 B     63.14%
             RAM:       64240 B        64 KB     98.02%

vs current main branch this PR is based on a2356f2

all default Apps/WatchFaces
USERAPP_TYPES:STRING=Apps::StopWatch, Apps::Alarm, Apps::Timer, Apps::Steps, Apps::HeartRate, Apps::Music, Apps::Paint, Apps::Paddle, Apps::Twos, Apps::Dice, Apps::Metronome, Apps::Navigation, Apps::Weather
WATCHFACE_TYPES:STRING=WatchFace::Digital, WatchFace::Analog, WatchFace::PineTimeStyle, WatchFace::Terminal, WatchFace::Infineat, WatchFace::CasioStyleG7710
           FLASH:      374548 B     474632 B     78.91%
             RAM:       64440 B        64 KB     98.33%
WatchFace::Terminal
           FLASH:      298520 B     474632 B     62.90%
             RAM:       64224 B        64 KB     98.00%

NeroBurner requested changes Nov 6, 2022

View reviewed changes

tgc-dk requested a review from NeroBurner November 7, 2022 21:23

NeroBurner added the enhancement Enhancement to an existing app/feature label Nov 29, 2022

tgc-dk force-pushed the button-unlock branch from 68abebc to 309dd05 Compare May 2, 2023 19:21

tgc-dk force-pushed the button-unlock branch from 309dd05 to 6f6d870 Compare May 3, 2023 20:18

jfowl mentioned this pull request Jun 15, 2023

Add option to wake up normally, but require button press for interaction #1765

Closed

KaffeinatedKat mentioned this pull request Sep 30, 2023

Separate "turn on screen" from "unlock touchscreen" #1839

Open

1 task

tgc-dk mentioned this pull request Nov 28, 2023

Fix for PR: Introduce a restricted clock-only-mode when not woken with the button InfiniTimeOrg/InfiniSim#131

Open

NeroBurner added this to the 1.15.0 milestone Jan 28, 2024

NeroBurner reviewed Jan 29, 2024

View reviewed changes

src/touchhandler/TouchHandler.h Outdated Show resolved Hide resolved

tgc-dk requested a review from NeroBurner May 5, 2024 18:53

NeroBurner reviewed Sep 30, 2024

View reviewed changes

src/touchhandler/TouchHandler.h Outdated Show resolved Hide resolved

tgc-dk and others added 2 commits September 30, 2024 22:55

Decrease heapsize to make room for the new icon

8f3b7b1

NeroBurner force-pushed the button-unlock branch from bb5c06f to eecef5e Compare October 3, 2024 20:19

TouchHandler: default buttonUnlocksOn to false

f0211e5

NeroBurner force-pushed the button-unlock branch from eecef5e to f0211e5 Compare October 3, 2024 20:24

NeroBurner added 2 commits October 6, 2024 20:13

PopupMessage: draw lock with lvgl, hard code size

0954e3b

Remove new font, draw with lvgl

2a8e953

Revert "Decrease heapsize to make room for the new icon"

42bcd56

This reverts commit 8f3b7b1.

Introduce a restricted clock-only-mode when not woken with the button #1359

Are you sure you want to change the base?

Introduce a restricted clock-only-mode when not woken with the button #1359

Conversation

tgc-dk commented Oct 5, 2022

minacode commented Oct 6, 2022 • edited Loading

tgc-dk commented Oct 7, 2022

minacode commented Oct 7, 2022

taukakao commented Oct 7, 2022

taukakao commented Oct 7, 2022

tgc-dk commented Oct 9, 2022

minacode commented Oct 11, 2022

tgc-dk commented Oct 16, 2022

Avamander commented Oct 16, 2022

minacode commented Oct 16, 2022

tgc-dk commented Oct 17, 2022

minacode commented Oct 17, 2022

NeroBurner left a comment

Choose a reason for hiding this comment

tgc-dk commented Nov 7, 2022

github-actions bot commented May 2, 2023 • edited Loading

pipe01 commented Oct 26, 2023

tgc-dk commented Nov 28, 2023

riban-bw commented Jan 19, 2024 • edited Loading

NeroBurner commented Jan 19, 2024

jgonyea commented Jan 25, 2024

tgc-dk commented Jan 27, 2024

tgc-dk commented Jan 27, 2024

jgonyea commented Jan 27, 2024 via email

tgc-dk commented Jan 28, 2024

minacode commented Jan 28, 2024

mark9064 commented Jan 28, 2024 • edited Loading

tgc-dk commented Jan 29, 2024

minacode commented Jan 29, 2024

tgc-dk commented Jan 29, 2024

NeroBurner commented Sep 16, 2024 • edited Loading

NeroBurner commented Sep 30, 2024

mark9064 commented Sep 30, 2024 • edited Loading

tgc-dk commented Oct 3, 2024

NeroBurner commented Oct 3, 2024

mark9064 commented Oct 5, 2024

tgc-dk commented Oct 6, 2024

tgc-dk commented Oct 6, 2024

NeroBurner commented Oct 6, 2024

mark9064 commented Oct 6, 2024

NeroBurner commented Oct 6, 2024

NeroBurner commented Oct 6, 2024 • edited Loading

minacode commented Oct 6, 2022 •

edited

Loading

github-actions bot commented May 2, 2023 •

edited

Loading

riban-bw commented Jan 19, 2024 •

edited

Loading

mark9064 commented Jan 28, 2024 •

edited

Loading

NeroBurner commented Sep 16, 2024 •

edited

Loading

mark9064 commented Sep 30, 2024 •

edited

Loading

NeroBurner commented Oct 6, 2024 •

edited

Loading