Skip to content

timer: Add ringing and counter #1971

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

timer: Add ringing and counter #1971

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7c65431
timer: Add ringing and counter
vkareh Jan 8, 2024
722978d
Keep screen on during timer buzzing
JustScott Feb 7, 2025
7002f68
timer: Fix undefined behavior when capturing expiry time
vkareh Nov 3, 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
4 changes: 4 additions & 0 deletions src/components/motor/MotorController.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ void MotorController::StopRinging() {
nrf_gpio_pin_set(PinMap::Motor);
}

bool MotorController::IsRinging() {
return (xTimerIsTimerActive(longVib) == pdTRUE);
}

void MotorController::StopMotor(TimerHandle_t /*xTimer*/) {
nrf_gpio_pin_set(PinMap::Motor);
}
1 change: 1 addition & 0 deletions src/components/motor/MotorController.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ namespace Pinetime {
void RunForDuration(uint8_t motorDuration);
void StartRinging();
void StopRinging();
bool IsRinging();

private:
static void Ring(TimerHandle_t xTimer);
Expand Down
16 changes: 13 additions & 3 deletions src/components/timer/Timer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@ void Timer::StartTimer(std::chrono::milliseconds duration) {
}

std::chrono::milliseconds Timer::GetTimeRemaining() {
TickType_t remainingTime = 0;
if (IsRunning()) {
TickType_t remainingTime = xTimerGetExpiryTime(timer) - xTaskGetTickCount();
return std::chrono::milliseconds(remainingTime * 1000 / configTICK_RATE_HZ);
remainingTime = xTimerGetExpiryTime(timer) - xTaskGetTickCount();
Copy link
Member

@mark9064 mark9064 Oct 20, 2024

Choose a reason for hiding this comment

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

On further reflection, we shouldn't change the semantics of this method

The Timer component is meant to be a generic component that multiple applications can be use (it should be just a thin wrapper around FreeRTOS timers)

One idea:

  • When setting a timer, the expiry tick time is stored in this class
  • New method GetTimeSinceExpired which returns xtaskgettickcount - expiry, or 0 (if running or stopped)
  • This method remains pretty much unchanged
  • Inside the timer screen we calculate the value as GetTimeRemaining if the timer is running, else with GetTimeSinceExpired

The only part of this I don't really like is the semantics of GetTimeSinceExpired when it hasn't expired yet or has been stopped, and also GetTimeRemaining when stopped or expired

Another idea:
Method GetTimerStatus which returns a variant of

  • Stopped
  • Running: time to expiry
  • Expired: time since expiry

This way semantics are always clear

Curious to know what you're thinking, any of these make sense? Not sure I've got any perfect solutions here, though I quite like the second one so far (haven't thought about either for too long)

Copy link
Contributor Author

@vkareh vkareh Dec 18, 2024

Choose a reason for hiding this comment

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

The Timer component is meant to be a generic component that multiple applications can be use (it should be just a thin wrapper around FreeRTOS timers)

Is it, though? That's what the xTimer interface is meant to do. This wrapper into a Timer component is only used for the Timer app. That said, I think this Timer class could still work elsewhere in the code as is, if there was a use case for it. If a new use case shows up, and a change is needed, we can do it then - not sure creating a super generic class is useful without knowing what use cases it's meant to address.

That's just my opinion though, and I may be entirely wrong (and I'm just having a hard time finding time to work on this these days - which is why I'm hesitant to get started on a change I probably won't finish).

Copy link
Member

@mark9064 mark9064 Nov 4, 2025

Choose a reason for hiding this comment

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

OK, so I've discussed this with the team a bit and we definitely think it's better as a generic component to avoid coupling to FreeRTOS so tightly. It also makes implementing InfiniSim easier.

This was discussed before in #1650 - in fact the the timer class used to be a controller specific to the timer app, and it was made generic with that PR

So I think we need to change/extend the API provided here with generic usage in mind

Specifically the Timer as implemented in InfiniTime only needs to be a one-shot resettable timer that calls a callback specified when creating the timer.

I'm happy to extend the timer class to fit the use case of this PR if that works for you. I can send up a PR for that, it can get merged in quickly and then you can rebase onto it (we're trying to get 1.16 released soon and it'd be great to land this PR)

How does that sound?

Copy link
Contributor Author

@vkareh vkareh Nov 5, 2025

Choose a reason for hiding this comment

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

@mark9064

I'm happy to extend the timer class to fit the use case of this PR if that works for you. I can send up a PR for that, it can get merged in quickly and then you can rebase onto it (we're trying to get 1.16 released soon and it'd be great to land this PR)

How does that sound?

That sounds good - I'm happy to rebase against that.

} else if (expired > 0) {
remainingTime = xTaskGetTickCount() - expired;
}
return std::chrono::milliseconds(0);
return std::chrono::milliseconds(remainingTime * 1000 / configTICK_RATE_HZ);
}

void Timer::StopTimer() {
Expand All @@ -26,3 +28,11 @@ void Timer::StopTimer() {
bool Timer::IsRunning() {
return (xTimerIsTimerActive(timer) == pdTRUE);
}

void Timer::SetExpiredTime() {
expired = xTaskGetTickCount();
}

void Timer::ResetExpiredTime() {
expired = 0;
}
5 changes: 5 additions & 0 deletions src/components/timer/Timer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,13 @@ namespace Pinetime {

bool IsRunning();

void SetExpiredTime();

void ResetExpiredTime();

private:
TimerHandle_t timer;
TickType_t expired = 0;
};
}
}
21 changes: 14 additions & 7 deletions src/displayapp/DisplayApp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ namespace {

void TimerCallback(TimerHandle_t xTimer) {
auto* dispApp = static_cast<DisplayApp*>(pvTimerGetTimerID(xTimer));
dispApp->SetTimerExpired();
dispApp->PushMessage(Display::Messages::TimerDone);
}
}
Expand Down Expand Up @@ -368,19 +369,21 @@ void DisplayApp::Refresh() {
case Messages::NewNotification:
LoadNewScreen(Apps::NotificationsPreview, DisplayApp::FullRefreshDirections::Down);
break;
case Messages::TimerDone:
case Messages::TimerDone: {
if (state != States::Running) {
PushMessageToSystemTask(System::Messages::GoToRunning);
}
if (currentApp == Apps::Timer) {
lv_disp_trig_activity(nullptr);
auto* timer = static_cast<Screens::Timer*>(currentScreen.get());
timer->Reset();
} else {
// Load timer app if not loaded
if (currentApp != Apps::Timer) {
LoadNewScreen(Apps::Timer, DisplayApp::FullRefreshDirections::Up);
}
motorController.RunForDuration(35);
// Set the timer to ringing mode
lv_disp_trig_activity(nullptr);
auto* timerScreen = static_cast<Screens::Timer*>(currentScreen.get());
timerScreen->SetTimerRinging();
motorController.StartRinging();
break;
}
case Messages::AlarmTriggered:
if (currentApp == Apps::Alarm) {
auto* alarm = static_cast<Screens::Alarm*>(currentScreen.get());
Expand Down Expand Up @@ -729,6 +732,10 @@ void DisplayApp::Register(Pinetime::Controllers::NavigationService* NavigationSe
this->controllers.navigationService = NavigationService;
}

void DisplayApp::SetTimerExpired() {
timer.SetExpiredTime();
}

void DisplayApp::ApplyBrightness() {
auto brightness = settingsController.GetBrightness();
if (brightness != Controllers::BrightnessController::Levels::Low && brightness != Controllers::BrightnessController::Levels::Medium &&
Expand Down
2 changes: 2 additions & 0 deletions src/displayapp/DisplayApp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ namespace Pinetime {
void Register(Pinetime::Controllers::MusicService* musicService);
void Register(Pinetime::Controllers::NavigationService* NavigationService);

void SetTimerExpired();

private:
Pinetime::Drivers::St7789& lcd;
const Pinetime::Drivers::Cst816S& touchPanel;
Expand Down
42 changes: 38 additions & 4 deletions src/displayapp/screens/Timer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ static void btnEventHandler(lv_obj_t* obj, lv_event_t event) {
}
}

Timer::Timer(Controllers::Timer& timerController) : timer {timerController} {
Timer::Timer(Controllers::Timer& timerController, Controllers::MotorController& motorController, System::SystemTask& systemTask)
: timer {timerController}, motorController {motorController}, wakeLock(systemTask) {

lv_obj_t* colonLabel = lv_label_create(lv_scr_act(), nullptr);
lv_obj_set_style_local_text_font(colonLabel, LV_LABEL_PART_MAIN, LV_STATE_DEFAULT, &jetbrains_mono_76);
Expand Down Expand Up @@ -62,7 +63,9 @@ Timer::Timer(Controllers::Timer& timerController) : timer {timerController} {
// Create the label as a child of the button so it stays centered by default
txtPlayPause = lv_label_create(btnPlayPause, nullptr);

if (timer.IsRunning()) {
if (motorController.IsRinging()) {
SetTimerRinging();
} else if (timer.IsRunning()) {
SetTimerRunning();
} else {
SetTimerStopped();
Expand Down Expand Up @@ -103,7 +106,23 @@ void Timer::UpdateMask() {
}

void Timer::Refresh() {
if (timer.IsRunning()) {
if (isRinging) {
DisplayTime();
if (motorController.IsRinging()) {
if (displaySeconds.Get().count() > 10) {
// Stop buzzing after 10 seconds, but continue the counter
motorController.StopRinging();
wakeLock.Release();
} else {
// Keep the screen awake during the first 10 seconds
wakeLock.Lock();
}
}
// Reset timer after 1 minute
if (displaySeconds.Get().count() > 60) {
Reset();
}
} else if (timer.IsRunning()) {
DisplayTime();
} else if (buttonPressing && xTaskGetTickCount() - pressTime > pdMS_TO_TICKS(150)) {
lv_label_set_text_static(txtPlayPause, "Reset");
Expand All @@ -129,16 +148,30 @@ void Timer::SetTimerRunning() {
minuteCounter.HideControls();
secondCounter.HideControls();
lv_label_set_text_static(txtPlayPause, "Pause");
lv_obj_set_style_local_bg_color(btnPlayPause, LV_BTN_PART_MAIN, LV_STATE_DEFAULT, Colors::bgAlt);
}

void Timer::SetTimerStopped() {
isRinging = false;
minuteCounter.ShowControls();
secondCounter.ShowControls();
lv_label_set_text_static(txtPlayPause, "Start");
lv_obj_set_style_local_bg_color(btnPlayPause, LV_BTN_PART_MAIN, LV_STATE_DEFAULT, LV_COLOR_GREEN);
}

void Timer::SetTimerRinging() {
isRinging = true;
minuteCounter.HideControls();
secondCounter.HideControls();
lv_label_set_text_static(txtPlayPause, "Reset");
lv_obj_set_style_local_bg_color(btnPlayPause, LV_BTN_PART_MAIN, LV_STATE_DEFAULT, LV_COLOR_RED);
}

void Timer::ToggleRunning() {
if (timer.IsRunning()) {
if (isRinging) {
motorController.StopRinging();
Reset();
} else if (timer.IsRunning()) {
DisplayTime();
timer.StopTimer();
SetTimerStopped();
Expand All @@ -151,6 +184,7 @@ void Timer::ToggleRunning() {
}

void Timer::Reset() {
timer.ResetExpiredTime();
DisplayTime();
SetTimerStopped();
}
11 changes: 9 additions & 2 deletions src/displayapp/screens/Timer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
#pragma once

#include "displayapp/screens/Screen.h"
#include "components/motor/MotorController.h"
#include "systemtask/SystemTask.h"
#include "systemtask/WakeLock.h"
#include "displayapp/LittleVgl.h"
#include "displayapp/widgets/Counter.h"
#include "utility/DirtyValue.h"
Expand All @@ -14,20 +16,24 @@ namespace Pinetime::Applications {
namespace Screens {
class Timer : public Screen {
public:
Timer(Controllers::Timer& timerController);
Timer(Controllers::Timer& timerController, Controllers::MotorController& motorController, System::SystemTask& systemTask);
~Timer() override;
void Refresh() override;
void Reset();
void ToggleRunning();
void ButtonPressed();
void MaskReset();
void SetTimerRinging();

private:
void SetTimerRunning();
void SetTimerStopped();
void UpdateMask();
void DisplayTime();
Pinetime::Controllers::Timer& timer;
Pinetime::Controllers::MotorController& motorController;

Pinetime::System::WakeLock wakeLock;

lv_obj_t* btnPlayPause;
lv_obj_t* txtPlayPause;
Expand All @@ -42,6 +48,7 @@ namespace Pinetime::Applications {
Widgets::Counter secondCounter = Widgets::Counter(0, 59, jetbrains_mono_76);

bool buttonPressing = false;
bool isRinging = false;
lv_coord_t maskPosition = 0;
TickType_t pressTime = 0;
Utility::DirtyValue<std::chrono::seconds> displaySeconds;
Expand All @@ -54,7 +61,7 @@ namespace Pinetime::Applications {
static constexpr const char* icon = Screens::Symbols::hourGlass;

static Screens::Screen* Create(AppControllers& controllers) {
return new Screens::Timer(controllers.timer);
return new Screens::Timer(controllers.timer, controllers.motorController, *controllers.systemTask);
};

static bool IsAvailable(Pinetime::Controllers::FS& /*filesystem*/) {
Expand Down