refactor: clarify responsabilities

apps/server/src/config/config.js

@@ -9,5 +9,4 @@ export const config = {
     filename: 'override.css',
   },
   restoreFile: 'ontime.restore',
-  timeSkipLimit: 3 * 32,
 };

apps/server/src/services/TimerService.ts

@@ -5,7 +5,7 @@ import { eventStore } from '../stores/EventStore.js';
 import { PlaybackService } from './PlaybackService.js';
 import { updateRoll } from './rollUtils.js';
 import { integrationService } from './integration-service/IntegrationService.js';
-import { getCurrent, getExpectedFinish, skipedOutOfEvent } from './timerUtils.js';
+import { getCurrent, getExpectedFinish, skippedOutOfEvent } from './timerUtils.js';
 import { clock } from './Clock.js';
 import { logger } from '../classes/Logger.js';
 import type { RestorePoint } from './RestoreService.js';
@@ -18,10 +18,13 @@ type initialLoadingData = {
 
 type RestoreCallback = (newState: RestorePoint) => Promise<void>;
 
+export const timeSkipLimit = 3 * 32;
+
 export class TimerService {
   private readonly _interval: NodeJS.Timer;
   private _updateInterval: number;
   private _lastUpdate: number | null;
+  private _skipThreshold: number;
 
   playback: Playback;
   timer: TimerState;
@@ -40,11 +43,13 @@ export class TimerService {
    * @param {object} [timerConfig]
    * @param {number} [timerConfig.refresh]
    * @param {number} [timerConfig.updateInterval]
+   * @param {number} [timerConfig.skipThreshold]
    */
-  constructor(timerConfig: { refresh?: number; updateInterval?: number } = {}) {
+  constructor(timerConfig: { refresh: number; updateInterval: number; skipThreshold: number }) {
     this._clear();
-    this._interval = setInterval(() => this.update(), timerConfig?.refresh ?? 1000);
-    this._updateInterval = timerConfig?.updateInterval ?? 1000;
+    this._interval = setInterval(() => this.update(), timerConfig.refresh);
+    this._updateInterval = timerConfig.updateInterval;
+    this._skipThreshold = timerConfig.skipThreshold;
   }
 
   /**
@@ -404,7 +409,15 @@ export class TimerService {
     let shouldNotify = false;
     if (this.playback === Playback.Roll) {
       shouldNotify = true;
-      if (skipedOutOfEvent(previousTime, this.timer.clock, this.timer.startedAt, this.timer.expectedFinish)) {
+      if (
+        skippedOutOfEvent(
+          previousTime,
+          this.timer.clock,
+          this.timer.startedAt,
+          this.timer.expectedFinish,
+          this._skipThreshold,
+        )
+      ) {
         PlaybackService.roll();
       } else {
         this.updateRoll();
@@ -508,4 +521,5 @@ export class TimerService {
 }
 
 // calculate at 30fps, refresh at 1fps
-export const eventTimer = new TimerService({ refresh: 32, updateInterval: 1000 });
+// we consider a skip at 3 lost updates
+export const eventTimer = new TimerService({ refresh: 32, updateInterval: 1000, skipThreshold: 32 * 3 });

apps/server/src/services/__tests__/timerUtils.test.ts

@@ -1,8 +1,7 @@
 import { dayInMs } from 'ontime-utils';
 import { TimerType } from 'ontime-types';
 
-import { getCurrent, getExpectedFinish, skipedOutOfEvent } from '../timerUtils.js';
-import { config } from '../../config/config.js';
+import { getCurrent, getExpectedFinish, skippedOutOfEvent } from '../timerUtils.js';
 
 describe('getExpectedFinish()', () => {
   it('is null if we havent started', () => {
@@ -356,88 +355,105 @@ describe('getExpectedFinish() and getCurrentTime() combined', () => {
   });
 });
 
-describe('skipedOutOfEvent()', () => {
-  it('normal roll out of event', () => {
+describe('skippedOutOfEvent()', () => {
+  const testSkipLimit = 32;
+  it('does not consider an event end as a skip', () => {
     const startedAt = 1000;
     const duration = 1000;
     const expectedFinish = startedAt + duration;
-    const previousTime = expectedFinish - config.timeSkipLimit / 2;
+    const previousTime = expectedFinish - testSkipLimit / 2;
+
     let clock = previousTime;
-    expect(skipedOutOfEvent(previousTime, clock, startedAt, expectedFinish)).toBe(false);
-    clock += config.timeSkipLimit;
-    expect(skipedOutOfEvent(previousTime, clock, startedAt, expectedFinish)).toBe(false);
+    expect(skippedOutOfEvent(previousTime, clock, startedAt, expectedFinish, testSkipLimit)).toBe(false);
+
+    clock += testSkipLimit;
+    expect(skippedOutOfEvent(previousTime, clock, startedAt, expectedFinish, testSkipLimit)).toBe(false);
   });
 
-  it('normal roll backwards out of event', () => {
+  it('allows rolling backwards in an event', () => {
     const startedAt = 1000;
     const duration = 1000;
     const expectedFinish = startedAt + duration;
-    const previousTime = startedAt + config.timeSkipLimit / 2;
+    const previousTime = startedAt + testSkipLimit / 2;
+
     let clock = previousTime;
-    expect(skipedOutOfEvent(previousTime, clock, startedAt, expectedFinish)).toBe(false);
-    clock -= config.timeSkipLimit;
-    expect(skipedOutOfEvent(previousTime, clock, startedAt, expectedFinish)).toBe(false);
+    expect(skippedOutOfEvent(previousTime, clock, startedAt, expectedFinish, testSkipLimit)).toBe(false);
+
+    clock -= testSkipLimit;
+    expect(skippedOutOfEvent(previousTime, clock, startedAt, expectedFinish, testSkipLimit)).toBe(false);
   });
 
-  it('normal roll out of event over midnight', () => {
-    const startedAt = dayInMs - config.timeSkipLimit;
+  it('accounts for crossing midnight', () => {
+    const startedAt = dayInMs - testSkipLimit;
     const expectedFinish = 10;
     const previousTime = dayInMs - 1;
+
     let clock = previousTime;
-    expect(skipedOutOfEvent(previousTime, clock, startedAt, expectedFinish)).toBe(false);
-    clock = config.timeSkipLimit - 2;
-    expect(skipedOutOfEvent(previousTime, clock, startedAt, expectedFinish)).toBe(false);
+    expect(skippedOutOfEvent(previousTime, clock, startedAt, expectedFinish, testSkipLimit)).toBe(false);
+
+    clock = testSkipLimit - 2;
+    expect(skippedOutOfEvent(previousTime, clock, startedAt, expectedFinish, testSkipLimit)).toBe(false);
   });
 
-  it('normal roll backwards out of event over midnight', () => {
-    const startedAt = dayInMs - config.timeSkipLimit;
+  it('allows rolling backwards in an event across midnight', () => {
+    const startedAt = dayInMs - testSkipLimit;
     const expectedFinish = 10;
     const previousTime = startedAt + 1;
+
     let clock = previousTime;
-    expect(skipedOutOfEvent(previousTime, clock, startedAt, expectedFinish)).toBe(false);
-    clock -= config.timeSkipLimit;
-    expect(skipedOutOfEvent(previousTime, clock, startedAt, expectedFinish)).toBe(false);
+    expect(skippedOutOfEvent(previousTime, clock, startedAt, expectedFinish, testSkipLimit)).toBe(false);
+
+    clock -= testSkipLimit;
+    expect(skippedOutOfEvent(previousTime, clock, startedAt, expectedFinish, testSkipLimit)).toBe(false);
   });
 
-  it('skip out of event', () => {
+  it('finds skip forwards out of event', () => {
     const startedAt = 1000;
     const duration = 1000;
     const expectedFinish = startedAt + duration;
-    const previousTime = expectedFinish - config.timeSkipLimit / 2;
+    const previousTime = expectedFinish - testSkipLimit / 2;
+
     let clock = previousTime;
-    expect(skipedOutOfEvent(previousTime, clock, startedAt, expectedFinish)).toBe(false);
-    clock += config.timeSkipLimit + 1;
-    expect(skipedOutOfEvent(previousTime, clock, startedAt, expectedFinish)).toBe(true);
+    expect(skippedOutOfEvent(previousTime, clock, startedAt, expectedFinish, testSkipLimit)).toBe(false);
+
+    clock += testSkipLimit + 1;
+    expect(skippedOutOfEvent(previousTime, clock, startedAt, expectedFinish, testSkipLimit)).toBe(true);
   });
 
-  it('skip backwards out of event', () => {
+  it('finds skip backwards out of event', () => {
     const startedAt = 1000;
     const duration = 1000;
     const expectedFinish = startedAt + duration;
-    const previousTime = startedAt + config.timeSkipLimit / 2;
+    const previousTime = startedAt + testSkipLimit / 2;
+
     let clock = previousTime;
-    expect(skipedOutOfEvent(previousTime, clock, startedAt, expectedFinish)).toBe(false);
-    clock -= config.timeSkipLimit + 1;
-    expect(skipedOutOfEvent(previousTime, clock, startedAt, expectedFinish)).toBe(true);
+    expect(skippedOutOfEvent(previousTime, clock, startedAt, expectedFinish, testSkipLimit)).toBe(false);
+
+    clock -= testSkipLimit + 1;
+    expect(skippedOutOfEvent(previousTime, clock, startedAt, expectedFinish, testSkipLimit)).toBe(true);
   });
 
-  it('skip out of event over midnight', () => {
-    const startedAt = dayInMs - config.timeSkipLimit;
+  it('finds skip forwards out of event across midnight', () => {
+    const startedAt = dayInMs - testSkipLimit;
     const expectedFinish = 10;
     const previousTime = dayInMs - 3;
+
     let clock = previousTime;
-    expect(skipedOutOfEvent(previousTime, clock, startedAt, expectedFinish)).toBe(false);
-    clock = config.timeSkipLimit - 2;
-    expect(skipedOutOfEvent(previousTime, clock, startedAt, expectedFinish)).toBe(true);
+    expect(skippedOutOfEvent(previousTime, clock, startedAt, expectedFinish, testSkipLimit)).toBe(false);
+
+    clock = testSkipLimit - 2;
+    expect(skippedOutOfEvent(previousTime, clock, startedAt, expectedFinish, testSkipLimit)).toBe(true);
   });
 
-  it('skip backwards out of event over midnight', () => {
-    const startedAt = dayInMs - config.timeSkipLimit;
+  it('finds skip backwards out of event across midnight', () => {
+    const startedAt = dayInMs - testSkipLimit;
     const expectedFinish = 10;
     const previousTime = startedAt + 1;
+
     let clock = previousTime;
-    expect(skipedOutOfEvent(previousTime, clock, startedAt, expectedFinish)).toBe(false);
-    clock -= config.timeSkipLimit + 1;
-    expect(skipedOutOfEvent(previousTime, clock, startedAt, expectedFinish)).toBe(true);
+    expect(skippedOutOfEvent(previousTime, clock, startedAt, expectedFinish, testSkipLimit)).toBe(false);
+
+    clock -= testSkipLimit + 1;
+    expect(skippedOutOfEvent(previousTime, clock, startedAt, expectedFinish, testSkipLimit)).toBe(true);
   });
 });

apps/server/src/services/timerUtils.ts

@@ -1,6 +1,5 @@
 import { MaybeNumber, TimerType } from 'ontime-types';
 import { dayInMs } from 'ontime-utils';
-import { config } from '../config/config.js';
 
 /**
  * Calculates expected finish time of a running timer
@@ -66,18 +65,19 @@ export function getCurrent(
   return startedAt + duration + addedTime + pausedTime - clock;
 }
 
-export function skipedOutOfEvent(previousTime: number, clock: number, startedAt: number, expectedFinish): boolean {
-  if (previousTime > dayInMs - config.timeSkipLimit && clock < config.timeSkipLimit) {
-    clock += dayInMs;
-  }
-  const skipTime = previousTime - clock;
-  const hasSkipped = Math.abs(skipTime) > config.timeSkipLimit;
-  expectedFinish = expectedFinish >= startedAt ? expectedFinish : expectedFinish + dayInMs;
-  if (hasSkipped) {
-    if (clock > expectedFinish || clock < startedAt) {
-      return true;
-    }
-    // otherwise we just skipped within the event
-  }
-  return false;
+export function skippedOutOfEvent(
+  previousTime: number,
+  clock: number,
+  startedAt: number,
+  expectedFinish: number,
+  skipLimit: number,
+): boolean {
+  const hasPassedMidnight = previousTime > dayInMs - skipLimit && clock < skipLimit;
+  const adjustedClock = hasPassedMidnight ? clock + dayInMs : clock;
+
+  const timeDifference = previousTime - adjustedClock;
+  const hasSkipped = Math.abs(timeDifference) > skipLimit;
+  const adjustedExpectedFinish = expectedFinish >= startedAt ? expectedFinish : expectedFinish + dayInMs;
+
+  return hasSkipped && (adjustedClock > adjustedExpectedFinish || adjustedClock < startedAt);
 }