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.

Sign up for GitHub

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

Prevents blocking when Infineat watchface is charging #2256

Merged
merged 5 commits into from
Feb 26, 2025
Merged

Prevents blocking when Infineat watchface is charging #2256

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7198b5b
Prevent blocking when Infineat watchface is charging
SteveAmor Feb 19, 2025
45f1ec7
Use RTOS timers for annimation
SteveAmor Feb 23, 2025
de7238a
Adds comment on height of animation
SteveAmor Feb 24, 2025
4c822e8
Fixes clang formatting error for comment
SteveAmor Feb 24, 2025
9a06545
Fixes comments
SteveAmor Feb 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/displayapp/screens/WatchFaceInfineat.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,12 +434,13 @@ void WatchFaceInfineat::Refresh() {

batteryPercentRemaining = batteryController.PercentRemaining();
isCharging = batteryController.IsCharging();
if (batteryController.IsCharging()) { // Charging battery animation
chargingBatteryPercent += 1;
if (batteryController.IsCharging() && (uint32_t(lv_tick_get() - savedAnimationTick) > 150)) { // Charging battery animation
Copy link
Member

Choose a reason for hiding this comment

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

Though they're the same, for timing it's recommended to use the RTOS tick with xTaskGetTickCount(). Additionally no type casts are needed, the overflow semantics of unsigned integers are well defined and subtraction works as expected. i.e just xTaskGetTickCount() - savedAnimationTick > pdMS_TO_TICKS(150) (ms to ticks is recommended as the tick rate may change in the future)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback @mark9064. I have made the changes and it works as expected in terms of the same timings as the original code and as per the first commit in this PR.

chargingBatteryPercent += 3; // 100% / pine_small height = 3
Copy link
Member

Choose a reason for hiding this comment

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

Maybe use 100 / lv_obj_get_height(logoPine)?
I think it'd be worth checking that LVGL does report what you'd expect
This would give a constant fillrate of 1 pixel per 150ms regardless of the pine cone size

Copy link
Contributor Author

@SteveAmor SteveAmor Feb 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This does appear to provide the expected value. I didn't know if I should type cast the result to an uint8_t? OK, no need given the division of two integer type values will result in an integer and chargingBatteryPercent is a uint8_t

if (chargingBatteryPercent > 100) {
chargingBatteryPercent = batteryPercentRemaining.Get();
}
SetBatteryLevel(chargingBatteryPercent);
savedAnimationTick = lv_tick_get();
} else if (isCharging.IsUpdated() || batteryPercentRemaining.IsUpdated()) {
chargingBatteryPercent = batteryPercentRemaining.Get();
SetBatteryLevel(chargingBatteryPercent);
Expand Down
1 change: 1 addition & 0 deletions src/displayapp/screens/WatchFaceInfineat.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ namespace Pinetime {

private:
uint32_t savedTick = 0;
uint32_t savedAnimationTick = 0;
Copy link
Member

Choose a reason for hiding this comment

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

This would become TickType_t.

I think it would be better if the variable name was specific that it's the charging animation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think/hope the changes I made address this?

uint8_t chargingBatteryPercent = 101; // not a mistake ;)

Utility::DirtyValue<uint8_t> batteryPercentRemaining {};
Expand Down
Loading