Return to Infineat watch face crashes when charging

[SteveAmor]

Verification

• I searched for similar bug reports (including closed issues) and found none was relevant.

What happened?

Swip off Infineat watch face, placing on charger, wait for screen to go blank and pressing button twice, screen remains blank and resets watchdog

What should happen instead?

Watch should not crash. It should display the watchface when you press the button (which it does for all watch faces except Infineat)

Reproduction steps

Display timeout is 5 secs.
Only happens with the Infineat face
Go through the below quite quickly.
Reproduced 100% of the time.

Swipe watchface to access quick settings
Place on charger
Small charge icon is displayed at top of display
Display switches off after 5 seconds
Press button to switch on display (I wanted to be sure it was charging)
Display doesn't switch on*
Press button again.
Display doesn't switch on.
Remove from charger
Press button to switch on Display
Watchdog timeout occurs
See watchface for a fraction of a second and then white to green pine cone
Last reset = wtdg

More details?

No shake to wake
Backlight low, med, high
24h
Infineat (works as expected on other faces)
Display timeout 5s
Bluetooth connected or off
Not using weather reporting
Watch is 5 years old from hardware point of view.
Battery level was <50% when I first noticed this and was reproduced multiple times at 100% charge.
Button presses were quick clicks. i.e. I didn't hold the button for 7 seconds to force a watchdog timer reset.

Version

Standard V1.15 2105a7b

Companion app

No response

[SteveAmor]

Extensive testing complete and issue updated.

[mark9064]

Can you reproduce this with 1.14 as well? I'm guessing it's a new issue

I've been doing some font work recently so Infineat is completely broken for me right now! I need to get round to recreating the entire external filesystem on my watch to test this, so I'll get round to this when I can

[SteveAmor]

Can you reproduce this with 1.14 as well? I'm guessing it's a new issue

I've been doing some font work recently so Infineat is completely broken for me right now! I need to get round to recreating the entire external filesystem on my watch to test this, so I'll get round to this when I can

I can confirm that this was introduced in #1980.

[SteveAmor]

If I comment out

InfiniTime/src/displayapp/screens/WatchFaceInfineat.cpp

Line 442 in 993118a

SetBatteryLevel(chargingBatteryPercent);

and

InfiniTime/src/displayapp/screens/WatchFaceInfineat.cpp

Line 445 in 993118a

SetBatteryLevel(chargingBatteryPercent);

to disable the charging battery animation it behaves as expected.

I don't know enough to fix this. It may even need to be properly addressed in another part of the codebase.

I actually only need to comment out

InfiniTime/src/displayapp/screens/WatchFaceInfineat.cpp

Line 442 in 993118a

SetBatteryLevel(chargingBatteryPercent);

to prevent it crashing.

Which filters through to this single line causing the problem

InfiniTime/src/displayapp/screens/WatchFaceInfineat.cpp

Line 473 in 993118a

lv_line_set_points(lineBattery, lineBatteryPoints, 2);

[SteveAmor]

Further to my findings in #2241 if I select AOD, and replicate the above steps, I can see that the Infineat display renders correctly after the display goes to AOD and the battery indicator shows it charging. BUT, pressing the button doesn't wake the watch (it's still on the charger base at this time) and will eventually crash if you press the button a few times and then remove it from the charger.

[SteveAmor]

In addition, if notifications are enabled and you place on the charger when you are on the quick setting screen and wait for the display to go off, if you then send a notification it will not display until you lift the watch off the charger.

Also, if chimes are enabled, they do not go off until the watch is lifted off the charger (assuming they should have gone off when on the charger). So, they were blocked at some point.

Note, you need to have the Infineat watch face and place it on the charger, not on the watch face so it automatically moves to the watch face as per #1980 for this edge case where things appear to get blocked until you lift the watch off the charger. All other watch faces function as expected on or off the charger - assuming #1980 triggered a return to the watch face.

[SteveAmor]

As it looked like it could be a blocking issue, if I remove

InfiniTime/src/displayapp/DisplayApp.cpp

Line 319 in 993118a

while (!lv_task_handler()) {

it fixes the problem(s) but the UX has the watch face transition glitch which is why it was added.

I assume this will block the Infineat watch face when it's charging because the charging battery animation means that the watch face will never be fully drawn - it's constantly being drawn.

[SteveAmor]

I have a workaround for this which I'll raise a PR for later today.

[minacode]

Is this another data race on a (battery) controller by the system task and the display app?

[SteveAmor]

Is this another data race on a (battery) controller by the system task and the display app?

I don't believe it is a race condition as it is specifically related to this line

InfiniTime/src/displayapp/screens/WatchFaceInfineat.cpp

Line 473 in 993118a

lv_line_set_points(lineBattery, lineBatteryPoints, 2);

which continously draws the watchface so the code is blocked by

InfiniTime/src/displayapp/DisplayApp.cpp

Line 319 in 993118a

while (!lv_task_handler()) {

No other watchfaces have an animation so are not blocked.

[minacode]

The first line which draws the line depends on the battery percentage that is obtained from the battery controller. Can you verify that this value is valid?

The battery controller is used by both the system task and the display app.

[mark9064]

I don't think it's a problem with the percentage - more than the indicator spins the CPU. See #2248 for some more analysis

[SteveAmor]

If you comment out

InfiniTime/src/displayapp/screens/WatchFaceInfineat.cpp

Line 473 in 993118a

lv_line_set_points(lineBattery, lineBatteryPoints, 2);

it doesn't block so it's definitely the line being drawn.

[minacode]

so it's definitely the line being drawn

Yes, but based on which values? Can it break if we pass it nonsense-data?

[mark9064]

It's not the value being drawn that's the problem. It's the fact that it's being drawn every frame, and it takes longer than one frame to draw it. Therefore LVGL sits at 100% CPU and never finishes lv_task_handler with time to spare, which results in the draw watchface spin loop locking up DisplayApp

[SteveAmor]

If you place on charge while on the Infineat watchface and wait for the screen to switch off, the watch acts as normal.

[SteveAmor]

It's not the value being drawn that's the problem. It's the fact that it's being drawn every frame, and it takes longer than one frame to draw it. Therefore LVGL sits at 100% CPU and never finishes lv_task_handler with time to spare, which results in the draw watchface spin loop locking up DisplayApp

The maths to calculate the position of the line will render in exactly the same place for several frames as the small pinecone is considerably less than 100px high. I'll investigate checking if that line position was previously drawn and not render it that frame refresh.

[mark9064]

I suspect the problem is that re-rendering requires loading the pinecone image from the external flash (which is very slow). How long does it take for the pinecone to complete a full cycle from bottom to top (with the screen just on normally while charging)?

[SteveAmor]

I suspect the problem is that re-rendering requires loading the pinecone image from the external flash (which is very slow). How long does it take for the pinecone to complete a full cycle from bottom to top (with the screen just on normally while charging)?

I discharged the watch to 25% (3.73v) and popped it on the charger. It took 3.35 secs (average over 3 times) for the animation to run from 25% to 100%.

Alternatively, battery at 16%. Place on charger. Start timer at top of pine cone and stop when it gets to the top again. 4.16secs. I think this is the time for (getting close to) 100% animation.

[SteveAmor]

It's not the value being drawn that's the problem. It's the fact that it's being drawn every frame, and it takes longer than one frame to draw it. Therefore LVGL sits at 100% CPU and never finishes lv_task_handler with time to spare, which results in the draw watchface spin loop locking up DisplayApp

The maths to calculate the position of the line will render in exactly the same place for several frames as the small pinecone is considerably less than 100px high. I'll investigate checking if that line position was previously drawn and not render it that frame refresh.

I wrapped an if (chargingBatteryPercent % 3 == 0) around

InfiniTime/src/displayapp/screens/WatchFaceInfineat.cpp

Line 442 in 993118a

SetBatteryLevel(chargingBatteryPercent);

and the animation is still slick and it doesn't block. I've not seen the resultant CPU utilisation in Infiniemu though.

What I'm not happy with is the magic number 3. This is 100% / height of pinecone which I'll address with a comment.

I do like the fact the issue is addressed in the watchface code, rather than the display app.

The problem now is that the animation is about twice the speed. It's not too much of a problem at low battery voltage but is pretty quick when there's only a few lines to draw when the battery is almost charged. I'll look at the best way to slow it down a bit.

Should I raise a new PR so both options can be evaluated?

[mark9064]

So it looks like drawing the pinecone takes about 50ms. With the current refresh period at 20ticks = 19.5ms (I'm calling it 20ms), what happens with it drawn every 3rd frame is the following (when screen on as usual):

lv_task_handler()
    0ms draw starts (frame 1)
    50ms draw finishes (frame 1)
    returns 0
lv_task_handler()
    50ms draw starts (frame 2)
    51ms draw finishes (frame 2)
    returns 19
vTaskDelay(19)
lv_task_handler()
    70ms draw starts (frame 3)
    71ms draw finishes (frame 3)
    returns 19
vTaskDelay(19)
lv_task_handler()
    70ms draw starts (frame 4)
    120ms draw finishes (frame 4)
    returns 0

etc.

Theoretically drawing every other frame should be fine. But neither are great solutions: animations should never be tied to framerate.

Instead it'd be better to animate according to some wall-clock frequency e.g 5 times per second. Also, you probably want the time from the bottom of the animation to the top to be constant rather than the number of pixels per second to be constant, as otherwise at high percentages it'll blink between high values rapidly.

To achieve this, the watchface could store the previous time Refresh() has last executed, and also store the time since the animation started. Then if more than 200ms has elapsed since the last execution, you could set the value as the real elapsed time modulo the total animation time (e.g 4 seconds).

Not sure if that made sense, let me know :)

[mark9064]

Should I raise a new PR so both options can be evaluated?

Yeah definitely when we've settled on an implementation I think a new PR would be clearer review-wise

[SteveAmor]

So it looks like drawing the pinecone takes about 50ms. With the current refresh period at 20ticks = 19.5ms (I'm calling it 20ms), what happens with it drawn every 3rd frame is the following (when screen on as usual):

lv_task_handler()
    0ms draw starts (frame 1)
    50ms draw finishes (frame 1)
    returns 0
lv_task_handler()
    50ms draw starts (frame 2)
    51ms draw finishes (frame 2)
    returns 19
vTaskDelay(19)
lv_task_handler()
    70ms draw starts (frame 3)
    71ms draw finishes (frame 3)
    returns 19
vTaskDelay(19)
lv_task_handler()
    70ms draw starts (frame 4)
    120ms draw finishes (frame 4)
    returns 0

etc.

Theoretically drawing every other frame should be fine. But neither are great solutions: animations should never be tied to framerate.

Instead it'd be better to animate according to some wall-clock frequency e.g 5 times per second. Also, you probably want the time from the bottom of the animation to the top to be constant rather than the number of pixels per second to be constant, as otherwise at high percentages it'll blink between high values rapidly.

To achieve this, the watchface could store the previous time Refresh() has last executed, and also store the time since the animation started. Then if more than 200ms has elapsed since the last execution, you could set the value as the real elapsed time modulo the total animation time (e.g 4 seconds).

Not sure if that made sense, let me know :)

Absolutely makes sense and I was also thinking that being tied to framerate is bad. Thank you for providing the above. After many pyhsical battery discharge cycles, using the torch, I hard coded a battery percentage to view the animation and I think it looks better if it's based on pixels per second. Otherwise, when you get to the last few pixels, it will be very slow. (It's by sheer fluke that the animation currently looks to be a good speed).
For a pixels per second approach, I'll test setting the lv_task_create period to 200 and set chargingBatteryPercent += 3; due to the pinecone height.

[minacode]

Nice job, finding that out.

Maybe you can write a (generic) animation class that handles the timing and drawing nicely and can be reused in other places.

There are also animations in lvgl. Maybe take a look at them as well.

[SteveAmor]

Final solution I have created #2256 for is based on lv_tick
Original CPU intensive animation from 0% to 100% was approx 5 secs.
Animation based on lv_tick rendering every 150 msecs results in 0% to 100% in approx 5 secs.

[mark9064]

Fixed with #2256