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

Add Timer's Time Remaining to StatusIcons #1967

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JustScott
Copy link
Contributor

@JustScott JustScott commented Jan 14, 2024

(look here for up-to-date images, as the ones below no longer represent the changes in this PR)

Adds a live output of the timer's time remaining, along with an hourGlass symbol to the left. Both of these are placed above the current time and are the same color as the date as to not draw attention away from the current time. The icon and the time remaining are both set to hidden if the timer isn't running.

timer_set_to_show
timer_set_to_show2
timer_set_to_hidden

Looking for opinions on possible changes to the timer's and symbol's font color and/or position.

Copy link

github-actions bot commented Jan 14, 2024

Build size and comparison to main:

Section Size Difference
text 373088B 272B
data 948B 0B
bss 22536B 0B

@FintasticMan FintasticMan added enhancement Enhancement to an existing app/feature UI/UX User interface/User experience labels Jan 17, 2024
@JustScott JustScott force-pushed the digital_watchface_timer branch from 612500d to 5ef3a85 Compare January 17, 2024 21:51
@vkareh
Copy link
Contributor

vkareh commented Jan 20, 2024

@JustScott

Looking for opinions on possible changes to the timer's and symbol's font color and/or position.

I like this feature a lot, as I use the timer regularly. Since you asked for opinions, how about making it part of the StatusIcons? This way you can see it in several other screens. Although maybe this makes it look too busy. Not sure, but just a suggestion.

@JustScott JustScott force-pushed the digital_watchface_timer branch from 5ef3a85 to 071aebf Compare February 1, 2024 14:44
@JustScott
Copy link
Contributor Author

JustScott commented Feb 1, 2024

@vkareh Here's how the timer would look as a StatusIcon:
timer_in_watchface
timer_in_app_selection

There are currently 2 issues with this:

  1. The status icons are only set to update when switching between screens in the Tile.cpp file, which means the time remaining will only update once each time you swipe between screens. This could be changed to update more often like is done on the clock faces, but I'm unsure of how this would effect power consumption.
  2. The timer is shown on the digital watch face screen both in the status icons and above the current time. We could have just avoided placing it in the watch face itself, and only placed it in the status icons with the change suggested in issue 1 above... but this would now take the same position as the new weather widget. So the obvious solution is just to detect if the current open app is the digital watch face, and set the timer to hidden.

I'll implement these fixes/changes, but first I'd like some feedback on whether this is something people actually want. I personally think it could be nice, but honestly I don't see myself needing to know how much time is remaining on a timer in the second it takes me to switch between apps (but some people may).

@vkareh
Copy link
Contributor

vkareh commented Feb 1, 2024

The timer is shown on the digital watch face screen both in the status icons and above the current time. We could have just avoided placing it in the watch face itself, and only placed it in the status icons with the change suggested in issue 1 above... but this would now take the same position as the new weather widget. So the obvious solution is just to detect if the current open app is the digital watch face, and set the timer to hidden.

I've been using a version of the digital face with the weather within the face itself:
InfiniSim_2024-02-01_131052

I think this change would go nicely with the timer in the status bar.

The status icons are only set to update when switching between screens in the Tile.cpp file, which means the time remaining will only update once each time you swipe between screens. This could be changed to update more often like is done on the clock faces, but I'm unsure of how this would effect power consumption.

This is a problem. It could be solved by adding decreasing the task refresh period like this (not tested):

diff --git a/src/displayapp/screens/Tile.cpp b/src/displayapp/screens/Tile.cpp
index 7c585a4b..ffdb9f44 100644
--- a/src/displayapp/screens/Tile.cpp
+++ b/src/displayapp/screens/Tile.cpp
@@ -87,7 +87,7 @@ Tile::Tile(uint8_t screenID,
   btnm1->user_data = this;
   lv_obj_set_event_cb(btnm1, event_handler);
 
-  taskUpdate = lv_task_create(lv_update_task, 5000, LV_TASK_PRIO_MID, this);
+  taskUpdate = lv_task_create(lv_update_task, 500, LV_TASK_PRIO_MID, this);
 
   UpdateScreen();
 }

500ms lets you see the timer seconds real time and it shouldn't really affect power consumption that much, I think, since the status icons only show up in screens that support auto-sleep of the display.

@JustScott
Copy link
Contributor Author

@vkareh Ah I missed the part where the status icons update every 5 seconds. I think moving this timer to the status icons would be the best solution if the weather icon is moved as shown in your image above. I'll try this out.

@JustScott
Copy link
Contributor Author

Alright I moved the timer from right above the time on the digital watchface, to the StatusIcons 'area'.

Also, I'm VERY new to C++, and was just following what looked to be the right way to pass this timer controller around, but I feel it got added to too many places... so feedback on whether the code is correct or not would be much appreciated. To me it seems like it would be best if I could just define the timer controller within StatusIcons.cpp instead of having to pass it to every StatusIcons class call.

@vkareh
Copy link
Contributor

vkareh commented Feb 1, 2024

I created a PR to move the weather widget: #1998

@JustScott JustScott force-pushed the digital_watchface_timer branch from 8e55197 to a36cde7 Compare February 20, 2024 22:14
@JustScott JustScott changed the title Add Timer's Time Remaining to Digitial Watchface Add Timer's Time Remaining to StatusIcons Feb 21, 2024
@JustScott JustScott force-pushed the digital_watchface_timer branch from a36cde7 to 3fa9dbb Compare March 15, 2024 03:48
@JustScott JustScott force-pushed the digital_watchface_timer branch 2 times, most recently from e289435 to da482ba Compare October 9, 2024 18:50
@JustScott JustScott force-pushed the digital_watchface_timer branch from da482ba to 217e5fc Compare December 14, 2024 20:59
When a timer is active the time remaining will be displayed in the
StatusIcons bar along with an hour glass symbol, and will be set to
hidden when the timer's off.
@JustScott JustScott force-pushed the digital_watchface_timer branch from 217e5fc to 8681c2a Compare December 21, 2024 04:49
@JustScott
Copy link
Contributor Author

I've applied the changes from all of vkareh's reviews, and have been using this without issue for quite a while now. Here's some images of how it looks with the digital watch face weather changes. @JF002 would this be something you'd be interested in including in version 1.16.0?

InfiniSim_2024-12-20_225411
InfiniSim_2024-12-20_225412

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement to an existing app/feature UI/UX User interface/User experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants