-
-
Notifications
You must be signed in to change notification settings - Fork 938
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
Added Trip meter to Steps app #764
Conversation
Since this doesn't, and probably shouldn't count laps, it should be thought of as a trip counter instead. While the daily step count resets automatically every day, the trip count would only reset when the user presses a reset button. I think the UI should be tweaked to better accommodate the new features. Here's an example of something I came up with quickly. |
Thanks for your input. I've changed the lap button to a trip reset button and made the UI changes. The below image shows the new behaviour:
|
I think there's an issue with this. The trip steps are only updated when the app is open, so if the app isn't opened, the trip doesn't increase. If at 4 in your newest example we take more steps, so let's say it's at 50 when the steps reset, the trip would still be 36, when it should be 50 right? If I don't check the app for multiple days, it misses the steps from those days. I think the trip counter should be updated similar to how new steps are added. |
Good point - thanks for the catch. I've changed the trip meter to update with the steps rather than from the app. I also found a bug in my logic for detecting new days and fixed that as well. |
Cool feature! A couple other ideas/suggestions:
|
Someone might be on a trip for multiple days and wants to know all the steps taken during that time. Also if they both reset at the same time, they always show the same value by default. There's really no reason to reset it automatically.
Seems like a good idea. I wonder if the numbers should be limited to some maximum value, like six numbers?
The arc widget can't show two arcs, but maybe another arc could be placed on top? How would the progress be calculated though? It doesn't really make sense to compare the trip to the daily goal. |
I hadn't thought of that. Good point. Requiring a manual reset would support both the single day and multi day use case. That would be identical to the UI of the trip meter on a car odometer, I guess.
I might see myself wanting to compare the trip to the current daily progress. If I have two logical "activities" in my day which both require walking, I could think of Back to your idea though, I suppose for the multi day use case, the "goal" could be interpreted as the trip goal instead of the daily goal, in which case being able to compare the two at a glance might make more sense. The arcs might be displayed concentrically so that they can both be seen at all times, regardless of whether the trip count is smaller or larger than the current daily progress. |
Changed the text to left pad + added some more space between the step count and the text. I think it looks pretty good now - check out the picture below: Note that the padding is for 5 characters, so if you set your step goal to 100 000+, the alignment will not work. I figure that having a goal of 100 000+ daily steps is pretty ambitious. We can pad for 6 digits, but that makes the text pretty wide and I don't think that looks as nice. I think there are some good ideas for additional features in this thread .However, to make the review process simpler, I would prefer to keep one feature per PR, and merge this first before adding more features to it. |
Thanks for the review - I've incorporated your suggested changes from above. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good style and format-wise. Needs testing on a physical device.
To produce the above screenshots, I did test the code with a physical device. If anyone else would like to try it as another point of data, that could be good. As an aside, clang-format didn't initially work with VSCode in the container out of the box for me - I had to tweak some settings to get it to use the right executable and format on save. I can create a separate issue to document this. |
Squashed commit of the following: commit 4b4f1a2 Author: Stephanie <[email protected]> Date: Sun Oct 31 11:53:13 2021 -0400 Ran clang-format commit cd747a2 Author: Stephanie <[email protected]> Date: Sat Oct 23 13:41:10 2021 -0400 Renamed confusing variables and general cleanup commit 308ddde Author: Stephanie <[email protected]> Date: Fri Oct 22 19:21:51 2021 -0400 Changed text to left pad commit 5098895 Author: Stephanie <[email protected]> Date: Thu Oct 21 23:37:35 2021 -0400 Moved trip meter update to MotionController and changed trip meter logic commit 3a20b48 Author: Stephanie <[email protected]> Date: Wed Oct 20 14:29:10 2021 -0400 Changed lap counter to trip meter commit 09e29a5 Author: Stephanie <[email protected]> Date: Tue Oct 19 23:42:48 2021 -0400 Added "lap" button to step counter
@stephanie-eng Thank you for this nice first PR! It's very well documented with test cases and pictures! I would be happy if we received more PR like this one as it makes the review much easier! |
Fixes #694
Note that using the lap button does not reset the progress to the daily step goal. The progress bar uses the total step count.
The image below demos the new button after fresh boot with (1) 0 steps, (2) the watch after taking some steps (step count and total updated and equal), (3) pressing the lap button (step count reset, no change to total), and (4) after walking again (step count and total updated, but not equal).
I also checked that large step counts do not cause display issues.