fix: do not mix an epoch offset into the FreeRTOS monotonic clock #74

2026-09-24T23:02:25+02:00

tbaumgartl commented

2026-09-24 23:02:25 +02:00

Clock::getClockMonotonic on FreeRTOS returned an epoch offset added on top of the uptime:

*time = Timekeeper::instance()->getMonotonicClockOffset() + getUptime();

monotonicClockOffset is a timeval member with no initializer, and Timekeeper's constructor
only initialises offset. The singleton is heap allocated, so the member holds indeterminate
bytes until Timekeeper::setOffset latches it, which happens on the first Clock::setClock.

Until that point getClockMonotonic returns garbage plus uptime, and at the latch it jumps by
whatever the difference happens to be. Any Countdown or Stopwatch armed before the latch
expires wrongly, in both directions:

forward jump: getCurrentTime() - startTime >= timeout
backward jump: the getCurrentTime() < startTime guard in Countdown::hasTimedOut

On our OBC this is not a race but the normal case. The CoreController sets the clock from its
own task, so everything constructed during object creation and early boot is on the wrong side
of the latch.

Why the offset can go

The monotonicity came from the uptime alone. The offset only made the value look epoch based,
and nothing depends on that:

Countdown uses getCurrentTime() - startTime
Stopwatch uses endTime - startTime
PeriodicHelper::performPeriodicHkGeneration uses now - setSpec.lastGenerated

Those are the only three callers in the framework. All take differences.

The Linux and host implementations already return CLOCK_MONOTONIC_RAW with no offset, so
FreeRTOS was the only OSAL where getClockMonotonic meant something different. The interface
doc in Clock.h also already describes the intended contract: "less suited when the absolute
time is required", with CLOCK_MONOTONIC_RAW named as the reference implementation.

Changes

osal/freertos/Clock.cpp: getClockMonotonic returns getUptime()
osal/freertos/Timekeeper.{h,cpp}: drop monotonicClockOffset, monotonicClockInitialized,
getMonotonicClockOffset and the never defined setMonotonicClockOffset. setOffset is now
a one liner
timemanager/Clock.h: drop the monotonicClockInitialized and monotonicClockOffset statics,
which were declared but never defined or used

Clock::getClock is unchanged and still returns offset plus uptime, so the wall clock is
unaffected.

Compatibility

getClockMonotonic on FreeRTOS now counts from scheduler start instead of the epoch. Code that
compares a monotonic timestamp against a wall clock value would break, but that would already be
broken on Linux, and no such code exists.

`Clock::getClockMonotonic` on FreeRTOS returned an epoch offset added on top of the uptime: ```cpp *time = Timekeeper::instance()->getMonotonicClockOffset() + getUptime(); ``` `monotonicClockOffset` is a `timeval` member with no initializer, and `Timekeeper`'s constructor only initialises `offset`. The singleton is heap allocated, so the member holds indeterminate bytes until `Timekeeper::setOffset` latches it, which happens on the first `Clock::setClock`. Until that point `getClockMonotonic` returns garbage plus uptime, and at the latch it jumps by whatever the difference happens to be. Any `Countdown` or `Stopwatch` armed before the latch expires wrongly, in both directions: - forward jump: `getCurrentTime() - startTime >= timeout` - backward jump: the `getCurrentTime() < startTime` guard in `Countdown::hasTimedOut` On our OBC this is not a race but the normal case. The CoreController sets the clock from its own task, so everything constructed during object creation and early boot is on the wrong side of the latch. ## Why the offset can go The monotonicity came from the uptime alone. The offset only made the value look epoch based, and nothing depends on that: - `Countdown` uses `getCurrentTime() - startTime` - `Stopwatch` uses `endTime - startTime` - `PeriodicHelper::performPeriodicHkGeneration` uses `now - setSpec.lastGenerated` Those are the only three callers in the framework. All take differences. The Linux and host implementations already return `CLOCK_MONOTONIC_RAW` with no offset, so FreeRTOS was the only OSAL where `getClockMonotonic` meant something different. The interface doc in `Clock.h` also already describes the intended contract: "less suited when the absolute time is required", with `CLOCK_MONOTONIC_RAW` named as the reference implementation. ## Changes - `osal/freertos/Clock.cpp`: `getClockMonotonic` returns `getUptime()` - `osal/freertos/Timekeeper.{h,cpp}`: drop `monotonicClockOffset`, `monotonicClockInitialized`, `getMonotonicClockOffset` and the never defined `setMonotonicClockOffset`. `setOffset` is now a one liner - `timemanager/Clock.h`: drop the `monotonicClockInitialized` and `monotonicClockOffset` statics, which were declared but never defined or used `Clock::getClock` is unchanged and still returns offset plus uptime, so the wall clock is unaffected. ## Compatibility `getClockMonotonic` on FreeRTOS now counts from scheduler start instead of the epoch. Code that compares a monotonic timestamp against a wall clock value would break, but that would already be broken on Linux, and no such code exists.

tbaumgartl added 1 commit 2026-09-24 23:02:26 +02:00

fix: do not mix an epoch offset into the FreeRTOS monotonic clock f0e7e8b523

getClockMonotonic added monotonicClockOffset on top of the uptime. That
offset is indeterminate until the first setClock call and jumps when it is
latched, so every Countdown and Stopwatch armed before that point either
expires immediately or measures against garbage.

The monotonicity came from the uptime alone, the offset only made the value
look epoch based. The Linux and host implementations return
CLOCK_MONOTONIC_RAW with no offset, so this also makes the OSALs agree.

All users take differences only: Countdown, Stopwatch and PeriodicHelper.

Also drops the Clock statics monotonicClockInitialized and
monotonicClockOffset, which were declared but never defined or used.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

tbaumgartl requested review from muellerr 2026-09-24 23:02:26 +02:00

muellerr reviewed

2026-09-25 09:54:49 +02:00

src/fsfw/osal/freertos/Clock.cpp Outdated

						
				@@ -49,3 +49,3 @@

				ReturnValue_t Clock::getClockMonotonic(timeval* time) {

				  *time = Timekeeper::instance()->getMonotonicClockOffset() + getUptime();

				  // Ticks since scheduler start, with no epoch offset on top. Same contract as the Linux and

muellerr commented

2026-09-25 09:54:49 +02:00

that comment is unnecessary. there is no need to justify or mention old/deleted/buggy code. maybe just mention that this retrieves the FreeRTOS uptime (which is hopefully fully monotonic)

tbaumgartl marked this conversation as resolved

tbaumgartl added 1 commit 2026-09-25 14:21:49 +02:00

fix comment 0220c0aeea

tbaumgartl merged commit 03d3b6cd68 into main

2026-09-25 14:24:15 +02:00

tbaumgartl deleted branch baumgartl/fix-freertos-monotonic-clock

2026-09-25 14:24:15 +02:00

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KSat/fsfw#74