Skip to content

Commit 0cc1871

Browse files
committed
timer: Fix undefined behavior when capturing expiry time
The timer was calling xTimerGetExpiryTime() after the FreeRTOS timer had expired, which results in undefined behavior according to the FreeRTOS documentation. This change captures the expiry time in the TimerCallback when the timer expires, using xTaskGetTickCount() instead of the undefined xTimerGetExpiryTime() on a stopped timer. The expiry time is now captured before the timer stops running.
1 parent e884a06 commit 0cc1871

File tree

4 files changed

+14
-9
lines changed

4 files changed

+14
-9
lines changed

src/components/timer/Timer.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ bool Timer::IsRunning() {
3030
}
3131

3232
void Timer::SetExpiredTime() {
33-
expired = xTimerGetExpiryTime(timer);
33+
expired = xTaskGetTickCount();
3434
}
3535

3636
void Timer::ResetExpiredTime() {

src/displayapp/DisplayApp.cpp

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ namespace {
6767

6868
void TimerCallback(TimerHandle_t xTimer) {
6969
auto* dispApp = static_cast<DisplayApp*>(pvTimerGetTimerID(xTimer));
70+
dispApp->SetTimerExpired();
7071
dispApp->PushMessage(Display::Messages::TimerDone);
7172
}
7273
}
@@ -364,22 +365,21 @@ void DisplayApp::Refresh() {
364365
case Messages::NewNotification:
365366
LoadNewScreen(Apps::NotificationsPreview, DisplayApp::FullRefreshDirections::Down);
366367
break;
367-
case Messages::TimerDone:
368+
case Messages::TimerDone: {
368369
if (state != States::Running) {
369370
PushMessageToSystemTask(System::Messages::GoToRunning);
370371
}
371372
// Load timer app if not loaded
372373
if (currentApp != Apps::Timer) {
373374
LoadNewScreen(Apps::Timer, DisplayApp::FullRefreshDirections::Up);
374375
}
375-
// Once loaded, set the timer to ringing mode
376-
if (currentApp == Apps::Timer) {
377-
lv_disp_trig_activity(nullptr);
378-
auto* timer = static_cast<Screens::Timer*>(currentScreen.get());
379-
timer->SetTimerRinging();
380-
}
376+
// Set the timer to ringing mode
377+
lv_disp_trig_activity(nullptr);
378+
auto* timerScreen = static_cast<Screens::Timer*>(currentScreen.get());
379+
timerScreen->SetTimerRinging();
381380
motorController.StartRinging();
382381
break;
382+
}
383383
case Messages::AlarmTriggered:
384384
if (currentApp == Apps::Alarm) {
385385
auto* alarm = static_cast<Screens::Alarm*>(currentScreen.get());
@@ -725,6 +725,10 @@ void DisplayApp::Register(Pinetime::Controllers::NavigationService* NavigationSe
725725
this->controllers.navigationService = NavigationService;
726726
}
727727

728+
void DisplayApp::SetTimerExpired() {
729+
timer.SetExpiredTime();
730+
}
731+
728732
void DisplayApp::ApplyBrightness() {
729733
auto brightness = settingsController.GetBrightness();
730734
if (brightness != Controllers::BrightnessController::Levels::Low && brightness != Controllers::BrightnessController::Levels::Medium &&

src/displayapp/DisplayApp.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,8 @@ namespace Pinetime {
8080
void Register(Pinetime::Controllers::MusicService* musicService);
8181
void Register(Pinetime::Controllers::NavigationService* NavigationService);
8282

83+
void SetTimerExpired();
84+
8385
private:
8486
Pinetime::Drivers::St7789& lcd;
8587
const Pinetime::Drivers::Cst816S& touchPanel;

src/displayapp/screens/Timer.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,6 @@ void Timer::SetTimerRinging() {
165165
secondCounter.HideControls();
166166
lv_label_set_text_static(txtPlayPause, "Reset");
167167
lv_obj_set_style_local_bg_color(btnPlayPause, LV_BTN_PART_MAIN, LV_STATE_DEFAULT, LV_COLOR_RED);
168-
timer.SetExpiredTime();
169168
}
170169

171170
void Timer::ToggleRunning() {

0 commit comments

Comments
 (0)