Skip to content

Commit 8ad9ed8

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 181776a commit 8ad9ed8

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
@@ -68,6 +68,7 @@ namespace {
6868

6969
void TimerCallback(TimerHandle_t xTimer) {
7070
auto* dispApp = static_cast<DisplayApp*>(pvTimerGetTimerID(xTimer));
71+
dispApp->SetTimerExpired();
7172
dispApp->PushMessage(Display::Messages::TimerDone);
7273
}
7374
}
@@ -368,22 +369,21 @@ void DisplayApp::Refresh() {
368369
case Messages::NewNotification:
369370
LoadNewScreen(Apps::NotificationsPreview, DisplayApp::FullRefreshDirections::Down);
370371
break;
371-
case Messages::TimerDone:
372+
case Messages::TimerDone: {
372373
if (state != States::Running) {
373374
PushMessageToSystemTask(System::Messages::GoToRunning);
374375
}
375376
// Load timer app if not loaded
376377
if (currentApp != Apps::Timer) {
377378
LoadNewScreen(Apps::Timer, DisplayApp::FullRefreshDirections::Up);
378379
}
379-
// Once loaded, set the timer to ringing mode
380-
if (currentApp == Apps::Timer) {
381-
lv_disp_trig_activity(nullptr);
382-
auto* timer = static_cast<Screens::Timer*>(currentScreen.get());
383-
timer->SetTimerRinging();
384-
}
380+
// Set the timer to ringing mode
381+
lv_disp_trig_activity(nullptr);
382+
auto* timerScreen = static_cast<Screens::Timer*>(currentScreen.get());
383+
timerScreen->SetTimerRinging();
385384
motorController.StartRinging();
386385
break;
386+
}
387387
case Messages::AlarmTriggered:
388388
if (currentApp == Apps::Alarm) {
389389
auto* alarm = static_cast<Screens::Alarm*>(currentScreen.get());
@@ -732,6 +732,10 @@ void DisplayApp::Register(Pinetime::Controllers::NavigationService* NavigationSe
732732
this->controllers.navigationService = NavigationService;
733733
}
734734

735+
void DisplayApp::SetTimerExpired() {
736+
timer.SetExpiredTime();
737+
}
738+
735739
void DisplayApp::ApplyBrightness() {
736740
auto brightness = settingsController.GetBrightness();
737741
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
@@ -82,6 +82,8 @@ namespace Pinetime {
8282
void Register(Pinetime::Controllers::MusicService* musicService);
8383
void Register(Pinetime::Controllers::NavigationService* NavigationService);
8484

85+
void SetTimerExpired();
86+
8587
private:
8688
Pinetime::Drivers::St7789& lcd;
8789
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)