InfiniTimeOrg / InfiniTime

Firmware for Pinetime smartwatch written in C++ and based on FreeRTOS
GNU General Public License v3.0
2.63k stars 902 forks source link

Add Timer's Time Remaining to StatusIcons #1967

Open JustScott opened 5 months ago

JustScott commented 5 months ago

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.

github-actions[bot] commented 5 months ago

Build checks have not completed. Possible reasons for this are:

  1. The checks need to be approved by a maintainer
  2. The branch has conflicts
  3. The firmware build has failed
vkareh commented 5 months ago

@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 commented 4 months ago

@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 commented 4 months ago

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 commented 4 months ago

@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 commented 4 months ago

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 commented 4 months ago

I created a PR to move the weather widget: https://github.com/InfiniTimeOrg/InfiniTime/pull/1998