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: add persistence #1410

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

pptime02
Copy link

@pptime02 pptime02 commented Nov 1, 2022

Stopwatch now keeps running in background if screen is left.
Implemented this by introducing a StopWatchController.
In additon, now an infinite lap count is possible (but only two laps are stored).
Largely based on the work of Louis Pearson (@desttinghim), see #783. Nevertheless, I removed all changes not related to the stopwatch persistence feature.

The corresponding PR to InfiniSim (for adding the StopWatchController) is InfiniTimeOrg/InfiniSim#75.
Would close #303.

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
@pptime02 pptime02 mentioned this pull request Nov 1, 2022
pptime02 added a commit to pptime02/InfiniSim that referenced this pull request Nov 1, 2022
StopWatchController was introduced with
InfiniTimeOrg/InfiniTime#1410
@pptime02 pptime02 force-pushed the add_stopwatch-persistence_squashed branch from c76d547 to 071f09d Compare November 1, 2022 18:04
@desttinghim
Copy link

Cool! Glad to see this is getting completed. I started working on rebasing it but I didn't want to get a build environment set up - thank you for taking it on 🙂

Copy link
Contributor

@minacode minacode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work on this feature. I scrolled through and found some things regarding code style. They are mostly functions starting with a lowercase character. I may not have found all of them.

src/components/stopwatch/StopWatchController.cpp Outdated Show resolved Hide resolved
src/displayapp/screens/StopWatch.h Outdated Show resolved Hide resolved
src/displayapp/screens/StopWatch.h Outdated Show resolved Hide resolved
src/displayapp/screens/StopWatch.h Outdated Show resolved Hide resolved
src/displayapp/screens/StopWatch.h Outdated Show resolved Hide resolved
src/components/stopwatch/StopWatchController.h Outdated Show resolved Hide resolved
src/components/stopwatch/StopWatchController.cpp Outdated Show resolved Hide resolved
@desttinghim
Copy link

Those are probably leftover from the work I did

pptime02 and others added 5 commits November 2, 2022 07:10
Co-authored-by: Max Friedrich <minacode@users.noreply.github.com>
Co-authored-by: Max Friedrich <minacode@users.noreply.github.com>
Co-authored-by: Max Friedrich <minacode@users.noreply.github.com>
Co-authored-by: Elements6007 <Elements6007@gmail.com>
@pptime02
Copy link
Author

pptime02 commented Nov 2, 2022

Thank you all for taking a look on it. I think I applied all proposals, and found no further lowercase-functions.

@minacode
Copy link
Contributor

minacode commented Nov 2, 2022

What about those struct types that end with _t? Do they fit our code style?

@pptime02
Copy link
Author

pptime02 commented Nov 2, 2022

What about those struct types that end with _t? Do they fit our code style?

I checked other structs by running rg 'struct .*_t \{' InfiniTime/src/ and rg 'struct .* \{' InfiniTime/src/. Seems like the stopwatch was the only place where _t was used, thus I removed the remaining _t it in the last commit.

@Xylemon
Copy link

Xylemon commented Feb 14, 2024

Any chance of this getting rebased against main? This would be very useful.

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.

Stopwatch resets when leaving the app.
5 participants