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

Stopwatch Persistence #783

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

desttinghim
Copy link

@desttinghim desttinghim commented Oct 25, 2021

I was using the stopwatch app while to track the cure time of epoxy (I didn't use the timer because I was working on many items in quick succession). I found a few annoyances with the stopwatch while doing so:

  • The current time is not displayed on the stopwatch screen
  • The stopwatch app could not be left without the stopwatch resetting

I was already planning to dig into the code, so I decided this was a great way to introduce myself to it. With this pull request, the following features are added/removed:

  • The current time is displayed on the stopwatch screen
  • The stopwatch will keep running while other screens are visited
  • The physical button no longer acts as a stop button (this is a personal preference)

To make this work I added StopWatchController to hold the state of the stopwatch, the elapsed time and 2 laps. The stopwatch app then uses this data when it's opened to display the correct values.

output.mp4

@desttinghim desttinghim changed the title Improved stopwatch Stopwatch Persistence Oct 25, 2021
@maksalees
Copy link
Contributor

How about displaying the battery percentage? It is already displayed in the menus and it would fill the empty space on the top right

@desttinghim
Copy link
Author

I figured out why the CI failed: I didn't run make all, and I didn't know about DisplayAppRecovery. I've just pushed a change that should fix that.

@maksalees Not a bad idea, I think I'll try implementing that.

@kieranc suggested in the pinetime-dev chat room that combining the stopwatch, timer, and alarm controllers could be a good idea. I think the idea has some merit, though I don't think that it will reduce the code much since they are quite different functionally. I think that would make a good follow up PR though.

@desttinghim
Copy link
Author

@maksalees I added the battery icon. It was pretty easy all things considered!

Louis Pearson added 11 commits November 30, 2021 05:36
These appear to have been the cause of my issues. Not sure why, I'd have
to know the nrf52832 better to diagnose it.
Also removes OnButtonPressed. I prefer the button's function to stay the
same when possible, but I'll change it if needed for the PR.
- Added StopWatchController to DisplayAppRecovery
- Added StopWatchController to everywhere needed in CMakeLists
- Fixed the reorder warning
@Itai-Nelken
Copy link
Contributor

This pr seems to break DFU. I have been able to consistently reproduce this in builds with this pr.
I added the following pr's one by one: #698, #720, #557, #821, and after I added this one, DFU stopped working. Maybe it somehow conflicts with them?

@desttinghim
Copy link
Author

desttinghim commented Dec 2, 2021

I'm not sure. I have to admit I goofed up and forgot to test if the code ran on the device, so I'll need to take some time and do that soon. What do you mean by "break DFU"? I'm not sure how you are using that here.

EDIT: I can't verify this RN (currently at work) but if it's a build issue it might be a conflict with #821, mostly noting so I can try that when I have time.

@Itai-Nelken
Copy link
Contributor

What do you mean by "break DFU"? I'm not sure how you are using that here.

What is described here: #831 (comment)
But sometimes the 'Error!' doesn't even show, it just goes back to the watchface.

I'll test develop + this pr and see if it's still an issue.

@kieranc
Copy link
Contributor

kieranc commented Dec 6, 2021

What do you mean by "break DFU"? I'm not sure how you are using that here.

What is described here: #831 (comment) But sometimes the 'Error!' doesn't even show, it just goes back to the watchface.

I'll test develop + this pr and see if it's still an issue.

Apologies for the delay but I've flashed several DFUs over builds with this PR included and I haven't had any issues.

@Itai-Nelken
Copy link
Contributor

I tried develop+this pr and DFU works fine.

FintasticMan added a commit to FintasticMan/InfiniTime that referenced this pull request Dec 13, 2021
Squashed commit of the following:

commit ec72f56
Author: Louis Pearson <louispearson@librem.one>
Date:   Tue Oct 26 19:03:46 2021 -0600

    Add battery icon to stopwatch screen

commit 8fbc164
Author: Louis Pearson <louispearson@librem.one>
Date:   Tue Oct 26 18:23:05 2021 -0600

    Make `make all` complete successfully

    - Added StopWatchController to DisplayAppRecovery
    - Added StopWatchController to everywhere needed in CMakeLists
    - Fixed the reorder warning

commit a4a0a4c
Author: Louis Pearson <louispearson@librem.one>
Date:   Mon Oct 25 07:13:40 2021 -0600

    Prevent lap count check when watch is stopped

commit c7c6309
Author: Louis Pearson <louispearson@librem.one>
Date:   Mon Oct 25 06:53:05 2021 -0600

    Run clang-format on files changed in branch

commit 858edfc
Author: Louis Pearson <louispearson@librem.one>
Date:   Mon Oct 25 06:43:16 2021 -0600

    Make function names more descriptive

    Also removes OnButtonPressed. I prefer the button's function to stay the
    same when possible, but I'll change it if needed for the PR.

commit 9ea6bc3
Author: Louis Pearson <louispearson@librem.one>
Date:   Mon Oct 25 06:11:27 2021 -0600

    Removed unsigned ints

    These appear to have been the cause of my issues. Not sure why, I'd have
    to know the nrf52832 better to diagnose it.

commit 2fd2cdb
Author: Louis Pearson <louispearson@librem.one>
Date:   Sun Oct 24 20:41:02 2021 -0600

    Implement lap tracking

commit 04a6b70
Author: Louis Pearson <louispearson@librem.one>
Date:   Sun Oct 24 10:29:34 2021 -0600

    Fix stopwatch display issues

commit ec2b673
Author: Louis Pearson <louispearson@librem.one>
Date:   Sun Oct 24 08:46:38 2021 -0600

    Implement stop watch controller

commit c75102f
Author: Louis Pearson <louispearson@librem.one>
Date:   Sat Oct 23 23:28:07 2021 -0600

    This may have been a bad idea

commit 3a51035
Author: Louis Pearson <louispearson@librem.one>
Date:   Sat Oct 23 21:18:45 2021 -0600

    Add time of day to stopwatch app
@lectrician1 lectrician1 mentioned this pull request Jan 9, 2022
@Itai-Nelken
Copy link
Contributor

something I discovered when I forgot to stop the stopwatch:
as the minutes side gets longer, it pushes the seconds out of the screen.
maybe a limit to 99:99 minutes should be added and when it's reached, the stopwatch app opens and vibrates a few times to notify the user that it can't count more?
(I haven't stopped it yet, it's currently at over 8300 minutes)

@trman
Copy link

trman commented Feb 2, 2022

maybe you could make the stopwatch number less big so when it attain 99:99 it become 1:00:00 with a final max to 99:99:99
(this time it would be the true max)
Not much people will use such stopwatch unless for marathon that are over 1h

@Riksu9000
Copy link
Contributor

@desttinghim Sorry it's taken a long time to review this. If you're still interested in working on this, please rebase this PR on develop. A PR should have minimal changes that are unrelated to the goal, so things like adding the current time, whether or not is a good addition, should not be included in this PR.

@desttinghim
Copy link
Author

@Riksu9000 It's been a while since I've done anything with this. I'll take a look this weekend and see if I can get the code rebased.

@Riksu9000 Riksu9000 marked this pull request as draft July 25, 2022 17:31
@yusufmte
Copy link
Contributor

Do you still intend to rebase/have the time? I definitely this would be an important improvement if it can get shaped up to get merged in!

@pptime02
Copy link

pptime02 commented Nov 1, 2022

I'm currently working on rebasing this and cleaning it up. I hope it'll be ready in a few hours.
Edit: it's online, see #1410

pptime02 added a commit to pptime02/InfiniTime that referenced this pull request Nov 1, 2022
Stopwatch now keeps running in background if screen is left.
Implemented this by introducing a StopWatchController.
Largely based on the work of Louis Pearson (@desttinghim@github.com),
see InfiniTimeOrg#783
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.

8 participants