mirror of
https://github.com/cpvalente/ontime.git
synced 2026-08-29 10:59:07 +00:00
fix(timer): make all pause maths wrap-aware
pausedAt is now an instant, but getExpectedFinish, getCurrent and getRuntimeOffset were still deriving the pause duration from the time of day clock (clock - pausedAt), which mixes units and produces a garbage result on every paused timer. - derive the ongoing pause from the instant in a single helper, so the duration is correct also when the pause spans midnight - getCurrent no longer special cases a paused timer: it discounts the ongoing pause from the elapsed time, which keeps the midnight correction that the paused branch was missing - reuse timeCore.elapsedTime instead of a local copy of the same logic - type getCurrent as a Duration and getElapsed as Maybe<Duration> to document that these are durations, never points in time - keep _now in parity with the clock in roll() - type the restore point pausedAt as an instant and reject restore points which still carry a time of day, resuming those would corrupt the timer Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019WA6Z38L7VY2oD6B4vG8hn
This commit is contained in:
@@ -1,7 +1,6 @@
|
||||
import { EndAction, Instant, Playback, TimeOfDay, TimeStrategy, TimerPhase, TimerType } from 'ontime-types';
|
||||
import { MILLIS_PER_HOUR, MILLIS_PER_MINUTE, MILLIS_PER_SECOND, dayInMs, millisToString } from 'ontime-utils';
|
||||
|
||||
import * as timeCore from '../../lib/time-core/timeCore.js';
|
||||
import type { RuntimeState } from '../../stores/runtimeState.js';
|
||||
import {
|
||||
findDayOffset,
|
||||
@@ -17,6 +16,13 @@ import {
|
||||
|
||||
const asTimeOfDay = (value: number): RuntimeState['clock'] => value as RuntimeState['clock'];
|
||||
|
||||
/**
|
||||
* Instants are epoch based, we anchor them to a fixed day to keep the tests deterministic
|
||||
*/
|
||||
const midnight = new Date(2026, 0, 1).getTime();
|
||||
const instantToday = (timeOfDay: number): Instant => (midnight + timeOfDay) as Instant;
|
||||
const instantYesterday = (timeOfDay: number): Instant => (midnight - dayInMs + timeOfDay) as Instant;
|
||||
|
||||
describe('getElapsed()', () => {
|
||||
it('returns active elapsed time from startedAt without add-time adjustments', () => {
|
||||
const state = {
|
||||
@@ -54,12 +60,12 @@ describe('getElapsed()', () => {
|
||||
it('uses the current pause start while paused', () => {
|
||||
const state = {
|
||||
clock: 10 * MILLIS_PER_MINUTE,
|
||||
_now: timeCore.toInstant((10 * MILLIS_PER_MINUTE) as TimeOfDay, timeCore.now()),
|
||||
_now: instantToday(10 * MILLIS_PER_MINUTE),
|
||||
timer: {
|
||||
startedAt: 2 * MILLIS_PER_MINUTE,
|
||||
},
|
||||
_timer: {
|
||||
pausedAt: timeCore.toInstant((7 * MILLIS_PER_MINUTE) as TimeOfDay, timeCore.now()),
|
||||
pausedAt: instantToday(7 * MILLIS_PER_MINUTE),
|
||||
pausedDuration: 1 * MILLIS_PER_MINUTE,
|
||||
},
|
||||
} as RuntimeState;
|
||||
@@ -146,6 +152,28 @@ describe('getExpectedFinish()', () => {
|
||||
const calculatedFinish = getExpectedFinish(state);
|
||||
expect(calculatedFinish).toBe(31);
|
||||
});
|
||||
it('adds the time of an ongoing pause which spans midnight', () => {
|
||||
const state = {
|
||||
eventNow: {
|
||||
timeEnd: 1 * MILLIS_PER_HOUR, // 01:00
|
||||
timerType: TimerType.CountDown,
|
||||
},
|
||||
clock: 3 * MILLIS_PER_MINUTE, // 00:03 (after midnight)
|
||||
_now: instantToday(3 * MILLIS_PER_MINUTE),
|
||||
timer: {
|
||||
addedTime: 0,
|
||||
duration: 2 * MILLIS_PER_HOUR,
|
||||
startedAt: 23 * MILLIS_PER_HOUR, // 23:00
|
||||
},
|
||||
_timer: {
|
||||
pausedAt: instantYesterday(23 * MILLIS_PER_HOUR + 58 * MILLIS_PER_MINUTE), // 23:58, before midnight
|
||||
hasFinished: false,
|
||||
},
|
||||
} as RuntimeState;
|
||||
|
||||
// the 5 minute pause pushes the finish from 01:00 to 01:05
|
||||
expect(getExpectedFinish(state)).toBe(1 * MILLIS_PER_HOUR + 5 * MILLIS_PER_MINUTE);
|
||||
});
|
||||
it('added time could be negative', () => {
|
||||
const state = {
|
||||
eventNow: {
|
||||
@@ -402,6 +430,51 @@ describe('getCurrent()', () => {
|
||||
expect(current).toBe(35);
|
||||
});
|
||||
|
||||
it('is frozen while paused', () => {
|
||||
const state = {
|
||||
eventNow: {
|
||||
timeEnd: 10,
|
||||
timerType: TimerType.CountDown,
|
||||
},
|
||||
clock: 5,
|
||||
_now: instantToday(5),
|
||||
timer: {
|
||||
addedTime: 0,
|
||||
duration: 10,
|
||||
startedAt: 0,
|
||||
},
|
||||
_timer: {
|
||||
pausedAt: instantToday(2), // we have been paused for 3ms (see clock)
|
||||
hasFinished: false,
|
||||
},
|
||||
} as RuntimeState;
|
||||
|
||||
// the timer holds the value it had when we paused
|
||||
expect(getCurrent(state)).toBe(8);
|
||||
});
|
||||
it('is frozen while paused, even if the pause spans midnight', () => {
|
||||
const state = {
|
||||
eventNow: {
|
||||
timeEnd: 1 * MILLIS_PER_HOUR, // 01:00
|
||||
timerType: TimerType.CountDown,
|
||||
},
|
||||
clock: 3 * MILLIS_PER_MINUTE, // 00:03 (after midnight)
|
||||
_now: instantToday(3 * MILLIS_PER_MINUTE),
|
||||
timer: {
|
||||
addedTime: 0,
|
||||
duration: 2 * MILLIS_PER_HOUR,
|
||||
startedAt: 23 * MILLIS_PER_HOUR, // 23:00
|
||||
},
|
||||
_timer: {
|
||||
pausedAt: instantYesterday(23 * MILLIS_PER_HOUR + 58 * MILLIS_PER_MINUTE), // 23:58, before midnight
|
||||
hasFinished: false,
|
||||
},
|
||||
} as RuntimeState;
|
||||
|
||||
// we ran for 58 minutes before pausing, so the timer holds at 1h02
|
||||
expect(getCurrent(state)).toBe(1 * MILLIS_PER_HOUR + 2 * MILLIS_PER_MINUTE);
|
||||
});
|
||||
|
||||
describe('on timers of type count-to-end', () => {
|
||||
it('current time is the time to end even if it hasnt started, this is weird, but by design', () => {
|
||||
const state = {
|
||||
@@ -957,13 +1030,14 @@ describe('getRuntimeOffset()', () => {
|
||||
dayOffset: 0,
|
||||
},
|
||||
clock: 150,
|
||||
_now: instantToday(150),
|
||||
timer: {
|
||||
startedAt: 100, // started on time
|
||||
current: 25, // are 25ms into it
|
||||
addedTime: 0,
|
||||
},
|
||||
_timer: {
|
||||
pausedAt: 125, // we have been paused for 25ms (see clock)
|
||||
pausedAt: instantToday(125), // we have been paused for 25ms (see clock)
|
||||
},
|
||||
rundown: {
|
||||
actualStart: 100,
|
||||
@@ -986,17 +1060,14 @@ describe('getRuntimeOffset()', () => {
|
||||
dayOffset: 0,
|
||||
},
|
||||
clock: 3 * MILLIS_PER_MINUTE, // 00:03 (after midnight)
|
||||
_now: timeCore.toInstant((3 * MILLIS_PER_MINUTE) as TimeOfDay, timeCore.now()),
|
||||
_now: instantToday(3 * MILLIS_PER_MINUTE),
|
||||
timer: {
|
||||
startedAt: 23 * MILLIS_PER_HOUR, // started on time at 23:00
|
||||
current: 25, // still counting down
|
||||
addedTime: 0,
|
||||
},
|
||||
_timer: {
|
||||
pausedAt: timeCore.toInstant(
|
||||
(23 * MILLIS_PER_HOUR + 58 * MILLIS_PER_MINUTE) as TimeOfDay,
|
||||
(timeCore.now() - dayInMs) as Instant,
|
||||
), // 23:58, before midnight
|
||||
pausedAt: instantYesterday(23 * MILLIS_PER_HOUR + 58 * MILLIS_PER_MINUTE), // 23:58, before midnight
|
||||
pausedDuration: 0,
|
||||
},
|
||||
rundown: {
|
||||
@@ -1007,7 +1078,11 @@ describe('getRuntimeOffset()', () => {
|
||||
_startDayOffset: 0,
|
||||
} as RuntimeState;
|
||||
|
||||
// paused from 23:58 to 00:03 -> so elapsed should still be 58 minutes
|
||||
// paused from 23:58 to 00:03 -> 5 minutes, regardless of the midnight wrap
|
||||
const { absolute } = getRuntimeOffset(state);
|
||||
expect(absolute).toBe(5 * MILLIS_PER_MINUTE);
|
||||
|
||||
// and the pause is not active time, elapsed is still the 58 minutes we ran before pausing
|
||||
expect(getElapsed(state)).toBe(58 * MILLIS_PER_MINUTE);
|
||||
});
|
||||
|
||||
|
||||
@@ -13,7 +13,7 @@ describe('isRestorePoint()', () => {
|
||||
selectedEventId: '123',
|
||||
startedAt: 1,
|
||||
addedTime: 2,
|
||||
pausedAt: 3,
|
||||
pausedAt: asInstant(1754745600000),
|
||||
firstStart: 1,
|
||||
startEpoch: asInstant(1),
|
||||
currentDay: 0,
|
||||
@@ -64,6 +64,22 @@ describe('isRestorePoint()', () => {
|
||||
expect(isRestorePoint({ ...restorePoint, pausedDuration: '3000' })).toBe(false);
|
||||
});
|
||||
|
||||
it('rejects a pausedAt which is not an instant', () => {
|
||||
const restorePoint = {
|
||||
playback: Playback.Pause,
|
||||
selectedEventId: '123',
|
||||
startedAt: 1,
|
||||
addedTime: 2,
|
||||
// restore points from before pausedAt became an instant contain a time of day
|
||||
pausedAt: 3,
|
||||
firstStart: 1,
|
||||
startEpoch: 1,
|
||||
currentDay: 0,
|
||||
};
|
||||
|
||||
expect(isRestorePoint(restorePoint)).toBe(false);
|
||||
});
|
||||
|
||||
describe('rejects a badly formatted file', () => {
|
||||
it('with invalid playback value', () => {
|
||||
const restorePoint = {
|
||||
|
||||
@@ -16,7 +16,7 @@ describe('restoreService', () => {
|
||||
selectedEventId: 'da5b4',
|
||||
startedAt: 1234,
|
||||
addedTime: 5678,
|
||||
pausedAt: 9087,
|
||||
pausedAt: asInstant(1754745609087),
|
||||
firstStart: 1234,
|
||||
startEpoch: asInstant(1234),
|
||||
currentDay: 0,
|
||||
@@ -55,7 +55,7 @@ describe('restoreService', () => {
|
||||
selectedEventId: 'da5b4',
|
||||
startedAt: 1234,
|
||||
addedTime: 1234,
|
||||
pausedAt: 1234,
|
||||
pausedAt: asInstant(1754745601234),
|
||||
groupStartAt: 10,
|
||||
};
|
||||
|
||||
@@ -83,7 +83,7 @@ describe('restoreService', () => {
|
||||
selectedEventId: '1234',
|
||||
startedAt: 1234,
|
||||
addedTime: 1234,
|
||||
pausedAt: 1234,
|
||||
pausedAt: asInstant(1754745601234),
|
||||
firstStart: 1234,
|
||||
startEpoch: asInstant(1234),
|
||||
currentDay: 0,
|
||||
@@ -101,7 +101,7 @@ describe('restoreService', () => {
|
||||
selectedEventId: '2345',
|
||||
startedAt: 2345,
|
||||
addedTime: 2345,
|
||||
pausedAt: 2345,
|
||||
pausedAt: asInstant(1754745602345),
|
||||
firstStart: 2345,
|
||||
startEpoch: asInstant(2345),
|
||||
currentDay: 0,
|
||||
@@ -112,7 +112,7 @@ describe('restoreService', () => {
|
||||
selectedEventId: '3456',
|
||||
startedAt: 3456,
|
||||
addedTime: 3456,
|
||||
pausedAt: 3456,
|
||||
pausedAt: asInstant(1754745603456),
|
||||
firstStart: 3456,
|
||||
startEpoch: asInstant(3456),
|
||||
currentDay: 1,
|
||||
@@ -134,7 +134,7 @@ describe('restoreService', () => {
|
||||
selectedEventId: '5678',
|
||||
startedAt: 5678,
|
||||
addedTime: 5678,
|
||||
pausedAt: 5678,
|
||||
pausedAt: asInstant(1754745605678),
|
||||
firstStart: 5678,
|
||||
startEpoch: asInstant(5678),
|
||||
currentDay: 0,
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { Playback } from 'ontime-types';
|
||||
import { dayInMs } from 'ontime-utils';
|
||||
|
||||
import { is } from '../../utils/is.js';
|
||||
import type { RestorePoint } from './restore.type.js';
|
||||
@@ -42,7 +43,9 @@ export function isRestorePoint(restorePoint: unknown): restorePoint is RestorePo
|
||||
return false;
|
||||
}
|
||||
|
||||
if (!is.number(restorePoint.pausedAt) && restorePoint.pausedAt !== null) {
|
||||
// pausedAt is an instant, restore points made before this change contained a time of day
|
||||
// we reject those to avoid resuming with a corrupt pause duration
|
||||
if (restorePoint.pausedAt !== null && (!is.number(restorePoint.pausedAt) || restorePoint.pausedAt < dayInMs)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
|
||||
@@ -5,7 +5,8 @@ export type RestorePoint = {
|
||||
selectedEventId: MaybeString;
|
||||
startedAt: MaybeNumber;
|
||||
addedTime: number;
|
||||
pausedAt: MaybeNumber;
|
||||
/** instant the playback was paused at */
|
||||
pausedAt: Maybe<Instant>;
|
||||
pausedDuration?: number;
|
||||
firstStart: MaybeNumber;
|
||||
startEpoch: Maybe<Instant>;
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { Day, MaybeNumber, TimeOfDay, TimerPhase } from 'ontime-types';
|
||||
import { Day, Duration, Maybe, MaybeNumber, TimeOfDay, TimerPhase } from 'ontime-types';
|
||||
import { MILLIS_PER_HOUR, checkIsNow, dayInMs, isPlaybackActive } from 'ontime-utils';
|
||||
|
||||
import * as timeCore from '../lib/time-core/timeCore.js';
|
||||
@@ -32,14 +32,12 @@ export function getExpectedFinish(state: RuntimeState): MaybeNumber {
|
||||
}
|
||||
|
||||
const { countToEnd, timeEnd } = state.eventNow;
|
||||
const { pausedAt } = state._timer;
|
||||
const { clock } = state;
|
||||
|
||||
if (startedAt === null) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const pausedTime = pausedAt != null ? clock - pausedAt : 0;
|
||||
const pausedTime = getOngoingPauseDuration(state);
|
||||
|
||||
if (countToEnd) {
|
||||
return timeEnd + addedTime + pausedTime;
|
||||
@@ -58,11 +56,11 @@ export function getExpectedFinish(state: RuntimeState): MaybeNumber {
|
||||
|
||||
/**
|
||||
* Calculates running countdown
|
||||
* The result is a duration (time left in the timer), never a point in time
|
||||
* @param {RuntimeState} state runtime state
|
||||
* @returns {number} current time for timer
|
||||
* @returns {Duration} time remaining in the running timer
|
||||
*/
|
||||
|
||||
export function getCurrent(state: RuntimeState): number {
|
||||
export function getCurrent(state: RuntimeState): Duration {
|
||||
// eslint-disable-next-line no-unused-labels -- dev code path
|
||||
DEV: {
|
||||
if (state.eventNow === null || state.timer.duration === null) {
|
||||
@@ -71,54 +69,58 @@ export function getCurrent(state: RuntimeState): number {
|
||||
}
|
||||
const { startedAt, duration, addedTime } = state.timer;
|
||||
const { countToEnd, timeStart, timeEnd } = state.eventNow;
|
||||
const { pausedAt } = state._timer;
|
||||
const { clock } = state;
|
||||
|
||||
if (countToEnd) {
|
||||
const isEventOverMidnight = timeStart > timeEnd;
|
||||
const correctDay = isEventOverMidnight ? dayInMs : 0;
|
||||
return correctDay - clock + timeEnd + addedTime;
|
||||
return (correctDay - clock + timeEnd + addedTime) as Duration;
|
||||
}
|
||||
|
||||
if (startedAt === null) {
|
||||
return duration;
|
||||
return duration as Duration;
|
||||
}
|
||||
|
||||
if (pausedAt != null) {
|
||||
return startedAt + duration + addedTime - pausedAt;
|
||||
}
|
||||
// an ongoing pause freezes the timer, so we discount the time spent in it
|
||||
const pausedTime = getOngoingPauseDuration(state);
|
||||
const elapsedSinceStart = timeCore.elapsedTime(clock, startedAt as TimeOfDay);
|
||||
|
||||
const hasPassedMidnight = startedAt > clock;
|
||||
const correctDay = hasPassedMidnight ? dayInMs : 0;
|
||||
return startedAt + duration + addedTime - clock - correctDay;
|
||||
return (duration + addedTime - elapsedSinceStart + pausedTime) as Duration;
|
||||
}
|
||||
|
||||
/**
|
||||
* Calculates active time elapsed since the timer started.
|
||||
* Calculates active time elapsed since the timer started
|
||||
* Time spent paused is not active time, so it is discounted
|
||||
*/
|
||||
export function getElapsed(state: RuntimeState): MaybeNumber {
|
||||
const { clock, _now } = state;
|
||||
export function getElapsed(state: RuntimeState): Maybe<Duration> {
|
||||
const { clock } = state;
|
||||
const { startedAt } = state.timer;
|
||||
const { pausedDuration, pausedAt } = state._timer;
|
||||
const { pausedDuration } = state._timer;
|
||||
|
||||
if (startedAt === null) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const currentPauseDuration = pausedAt !== null ? timeCore.timeSince(_now, pausedAt) : 0;
|
||||
const elapsedSinceStart = timeCore.elapsedTime(clock, startedAt as TimeOfDay);
|
||||
const activeElapsed = elapsedSinceStart - pausedDuration - getOngoingPauseDuration(state);
|
||||
|
||||
const elapsedSinceStart = getTimeSinceStart(clock, startedAt);
|
||||
const activeElapsed = elapsedSinceStart - pausedDuration - currentPauseDuration;
|
||||
|
||||
return Math.max(0, activeElapsed);
|
||||
return Math.max(0, activeElapsed) as Duration;
|
||||
}
|
||||
|
||||
function getTimeSinceStart(clock: TimeOfDay, startedAt: number): number {
|
||||
if (clock < startedAt) {
|
||||
return clock + dayInMs - startedAt;
|
||||
/**
|
||||
* Calculates how long the current pause has been going on for
|
||||
* The pause is tracked as an instant, which makes the calculation
|
||||
* immune to the clock wrapping around midnight
|
||||
* @returns 0 if the playback is not paused
|
||||
*/
|
||||
function getOngoingPauseDuration(state: RuntimeState): Duration {
|
||||
const { pausedAt } = state._timer;
|
||||
|
||||
if (pausedAt == null) {
|
||||
return 0 as Duration;
|
||||
}
|
||||
|
||||
return clock - startedAt;
|
||||
return timeCore.timeSince(state._now, pausedAt);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -154,7 +156,7 @@ export function skippedOutOfEvent(state: RuntimeState, previousTime: number, ski
|
||||
* Negative offset is under time / ahead of schedule
|
||||
*/
|
||||
export function getRuntimeOffset(state: RuntimeState): { absolute: number; relative: number } {
|
||||
const { eventNow, clock, _startDayOffset } = state;
|
||||
const { eventNow, _startDayOffset } = state;
|
||||
const { addedTime, current, startedAt } = state.timer;
|
||||
// nothing to calculate if there are no loaded events or if we havent started
|
||||
if (eventNow === null || startedAt === null || _startDayOffset === null) {
|
||||
@@ -178,8 +180,8 @@ export function getRuntimeOffset(state: RuntimeState): { absolute: number; relat
|
||||
// how long has the event been running over (is a negative number when in over timer so inverted before adding to offset)
|
||||
const overtime = Math.abs(Math.min(current, 0));
|
||||
|
||||
// time the playback was paused, the different from now to when we paused is added to the offset TODO: brakes when crossing midnight
|
||||
const pausedTime = state._timer.pausedAt === null ? 0 : clock - state._timer.pausedAt;
|
||||
// time the playback was paused, the difference from now to when we paused is added to the offset
|
||||
const pausedTime = getOngoingPauseDuration(state);
|
||||
|
||||
// absolute offset is difference between schedule and playback time
|
||||
// in case of count to end, the absolute offset is overtime and added time
|
||||
|
||||
@@ -285,6 +285,10 @@ describe('mutation on runtimeState', () => {
|
||||
vi.setSystemTime('jan 2 00:01');
|
||||
update();
|
||||
expect(getState().timer.elapsed).toBe(8 * MILLIS_PER_MINUTE);
|
||||
// the timer is frozen at the value it had when we paused
|
||||
expect(getState().timer.current).toBe(2 * MILLIS_PER_HOUR - 8 * MILLIS_PER_MINUTE);
|
||||
// we started 50 minutes late and have been paused for 3 minutes
|
||||
expect(getState().offset.absolute).toBe(53 * MILLIS_PER_MINUTE);
|
||||
|
||||
// resume 5 minutes after pausing, having crossed midnight (23:58 -> 00:03)
|
||||
vi.setSystemTime('jan 2 00:03');
|
||||
|
||||
@@ -63,9 +63,11 @@ export type RuntimeState = {
|
||||
// private properties of the timer calculations
|
||||
_timer: {
|
||||
forceFinish: Maybe<TimeOfDay>; // whether we should declare an event as finished, will contain the finish time
|
||||
|
||||
/** Instant the playback was paused at, an instant so that pause durations survive midnight */
|
||||
pausedAt: Maybe<Instant>;
|
||||
|
||||
/** Accumulate pause duration but dose not include the current pause */
|
||||
/** Accumulated duration of past pauses, does not include an ongoing pause */
|
||||
pausedDuration: number;
|
||||
secondaryTarget: Maybe<TimeOfDay>;
|
||||
hasFinished: boolean;
|
||||
@@ -78,6 +80,8 @@ export type RuntimeState = {
|
||||
_end: ExpectedMetadata;
|
||||
_startEpoch: Maybe<Instant>;
|
||||
_startDayOffset: Maybe<Day>;
|
||||
|
||||
/** Current instant, kept in parity with the clock (see setClock) */
|
||||
_now: Instant;
|
||||
};
|
||||
|
||||
@@ -655,9 +659,8 @@ export function roll(
|
||||
}
|
||||
|
||||
// we will need to do some calculations, update the time first
|
||||
const epoch = timeCore.now();
|
||||
const now = timeCore.toTimeOfDay(epoch);
|
||||
runtimeState.clock = now;
|
||||
setClock(runtimeState);
|
||||
const epoch = runtimeState._now;
|
||||
|
||||
// 2. if there is an event armed, we use it
|
||||
if (runtimeState.timer.playback === Playback.Armed || runtimeState.timer.phase === TimerPhase.Pending) {
|
||||
|
||||
Reference in New Issue
Block a user