Fix Pomodoro state machine pause/resume functionality - resolve timer synchronization bug

[Copilot]

Problem

The Pomodoro timer pause/resume functionality was broken due to redundant change_state() calls that caused mis-synchronization between the global remaining_ms variable and the timer module's internal tracking.

Root cause: Units confusion in pause/resume operations:

• timer_get_remaining() returns milliseconds
• change_state() expects seconds and multiplies by 1000
• This caused remaining time to be multiplied by 1000 during pause (3 minutes became 3000 minutes!)

Symptoms:

• Timer would not continue progressing correctly after resume
• State machine transitions could fail
• Timer callbacks were not properly restored

Solution

1. Fixed pomodoro_pause():

// Before (buggy):
change_state(POMODORO_PAUSED_WORK, timer_get_remaining()); // Units confusion!

// After (fixed):
current_state = POMODORO_PAUSED_WORK;
timer_pause();
if (state_cb) state_cb(current_state); // Manual UI callback

2. Fixed pomodoro_resume():

// Before (buggy):
change_state(POMODORO_WORK, remaining_ms / 1000); // Interferes with timer

// After (fixed):
current_state = POMODORO_WORK;
timer_resume();
if (state_cb) state_cb(current_state); // Manual UI callback

3. Fixed pomodoro_get_remaining_sec():
Now gets live remaining time from timer module instead of potentially stale global variable:

return timer_get_remaining() / 1000; // Always current, correct units

4. Fixed parameter shadowing in on_timer_tick():

• Renamed parameter to avoid shadowing global variable
• Fixed units in tick callback (now correctly passes seconds to UI)

Key Benefits

• ✅ Pause/Resume works correctly: Timer maintains accurate remaining time
• ✅ No interference: Timer module handles pause/resume internally without state machine conflicts
• ✅ UI callbacks preserved: State changes still trigger UI updates properly
• ✅ No regression: Normal start/reset/expired transitions continue to work
• ✅ Units consistency: All time values handled with correct millisecond/second conversions

Testing

The fix has been validated with comprehensive tests covering:

• Work session pause/resume scenarios
• Break session pause/resume scenarios
• Verification that remaining time is preserved correctly (no 1000x multiplication bug)
• Confirmation that timer progresses normally after resume
• Regression testing for normal state transitions

Example test case:

// Start 5-minute work session
pomodoro_start();

// Work for 2 minutes (180 seconds remaining)
timer_tick_handler();
uint32_t before_pause = pomodoro_get_remaining_sec(); // 180 seconds

// Pause and resume
pomodoro_pause();
pomodoro_resume();

uint32_t after_resume = pomodoro_get_remaining_sec(); // Still 180 seconds ✅

The solution is minimal and surgical, addressing the exact synchronization issues without breaking existing functionality.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• esm.ubuntu.com
  • Triggering command: /usr/lib/apt/methods/https (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

This pull request was created as a result of the following prompt from Copilot chat.

Bug Fix: Pomodoro state machine fails after resume from pause

Currently, when the Pomodoro timer is paused during any session (WORK, SHORT_BREAK, LONG_BREAK) and then resumed, the timer does not continue progressing correctly. The state machine may not advance, and the timer callbacks may not be correctly restored. This is due to redundant use of change_state() and mis-synchronization between the global remaining_ms (in pomodoro.c) and the timer module's internal tracking.

Fix Plan:

• In pomodoro_pause():
  • Only update the Pomodoro state to PAUSED (WORK or BREAK), and fetch the correct remaining time from the timer.
  • Do not call change_state() with a new duration that could overwrite state/timer internals.
  • Pause the timer.
• In pomodoro_resume():
  • Only update the Pomodoro state to running (WORK or correct BREAK type), but do not reset the timer duration or callbacks.
  • Do not call change_state() with a duration; simply set the correct state and unpause the timer.
• Ensure that the timer resumes correctly from its stored remaining time.
• Remove redundant or conflicting assignments to remaining_ms during pause/resume.

Acceptance:

• Pausing in any session works, and resuming continues the timer and state transitions as normal.
• Timer ticks and state callbacks are restored properly after resume.
• No regression to start/reset/expired transitions.

---

💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.