From facce4f096b5eec901e2f385e1b0fc6be44de2c1 Mon Sep 17 00:00:00 2001 From: arc-alex Date: Mon, 20 Nov 2023 17:44:39 +0100 Subject: [PATCH 1/9] trigger doRoll if clock is less than startAt --- apps/server/src/services/TimerService.ts | 2 +- .../src/services/__tests__/rollUtils.test.ts | 20 +++++++++++++++++++ apps/server/src/services/rollUtils.ts | 7 ++++++- 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/apps/server/src/services/TimerService.ts b/apps/server/src/services/TimerService.ts index 4f3714cc0..64c8ae577 100644 --- a/apps/server/src/services/TimerService.ts +++ b/apps/server/src/services/TimerService.ts @@ -339,7 +339,7 @@ export class TimerService { this.timer.expectedFinish >= this.timer.startedAt ? this.timer.expectedFinish : this.timer.expectedFinish + dayInMs, - + _startAt: this.timer.startedAt, clock: this.timer.clock, secondaryTimer: this.timer.secondaryTimer, secondaryTarget: this.secondaryTarget, diff --git a/apps/server/src/services/__tests__/rollUtils.test.ts b/apps/server/src/services/__tests__/rollUtils.test.ts index 329ece996..5622661c6 100644 --- a/apps/server/src/services/__tests__/rollUtils.test.ts +++ b/apps/server/src/services/__tests__/rollUtils.test.ts @@ -703,4 +703,24 @@ describe('typical scenarios', () => { expect(updateRoll(timers)).toStrictEqual(expected); }); + + it('rolls backwards', () => { + const timers = { + selectedEventId: '1', + current: 11, + _startAt: 10, + _finishAt: 15, + clock: 9, + secondaryTimer: null, + secondaryTarget: null, + }; + + const expected = { + updatedTimer: timers._finishAt - timers.clock, + updatedSecondaryTimer: null, + doRollLoad: true, + isFinished: false, + }; + expect(updateRoll(timers)).toStrictEqual(expected); + }); }); diff --git a/apps/server/src/services/rollUtils.ts b/apps/server/src/services/rollUtils.ts index 98a6feac2..b3c59c7f7 100644 --- a/apps/server/src/services/rollUtils.ts +++ b/apps/server/src/services/rollUtils.ts @@ -153,6 +153,7 @@ type CurrentTimers = { selectedEventId: string | null; current: number | null; _finishAt: number | null; + _startAt: number | null; clock: number | null; secondaryTimer: number | null; secondaryTarget: number | null; @@ -164,7 +165,7 @@ type CurrentTimers = { * @returns {object} object with selection variables */ export const updateRoll = (currentTimers: CurrentTimers) => { - const { selectedEventId, current, _finishAt, clock, secondaryTimer, secondaryTarget } = currentTimers; + const { selectedEventId, current, _finishAt, _startAt, clock, secondaryTimer, secondaryTarget } = currentTimers; // timers let updatedTimer = current; @@ -182,10 +183,14 @@ export const updateRoll = (currentTimers: CurrentTimers) => { updatedTimer -= dayInMs; } + console.log(Math.floor(_startAt / 1000), Math.floor(clock / 1000)); + if (updatedTimer < 0) { isPrimaryFinished = true; // we need a new event doRollLoad = true; + } else if (clock < _startAt) { + doRollLoad = true; } } else if (secondaryTimer >= 0) { // if secondaryTimer is running we are in waiting to roll From 22d89c6fbca5f2035ee1e4abe36421c6c285edfe Mon Sep 17 00:00:00 2001 From: arc-alex Date: Mon, 20 Nov 2023 17:51:50 +0100 Subject: [PATCH 2/9] add _startAt to all tests --- apps/server/src/services/__tests__/rollUtils.test.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/apps/server/src/services/__tests__/rollUtils.test.ts b/apps/server/src/services/__tests__/rollUtils.test.ts index 5622661c6..7c1b4a7c8 100644 --- a/apps/server/src/services/__tests__/rollUtils.test.ts +++ b/apps/server/src/services/__tests__/rollUtils.test.ts @@ -562,6 +562,7 @@ describe('typical scenarios', () => { selectedEventId: '1', current: 10, _finishAt: 15, + _startAt: 9, clock: 11, secondaryTimer: null, secondaryTarget: null, @@ -589,6 +590,7 @@ describe('typical scenarios', () => { selectedEventId: null, current: null, _finishAt: null, + _startAt: null, clock: 11, secondaryTimer: 1, secondaryTarget: 15, @@ -609,6 +611,7 @@ describe('typical scenarios', () => { selectedEventId: '1', current: 10, _finishAt: 11, + _startAt: 9, clock: 12, secondaryTimer: null, secondaryTarget: null, @@ -629,6 +632,7 @@ describe('typical scenarios', () => { selectedEventId: null, current: null, _finishAt: null, + _startAt: null, clock: 16, secondaryTimer: 1, secondaryTarget: 15, @@ -649,6 +653,7 @@ describe('typical scenarios', () => { selectedEventId: null, current: null, _finishAt: null, + _startAt: null, clock: 15, secondaryTimer: 0, secondaryTarget: 15, @@ -669,6 +674,7 @@ describe('typical scenarios', () => { selectedEventId: '1', current: 25, _finishAt: 10 + dayInMs, + _startAt: 10, clock: dayInMs - 10, secondaryTimer: null, secondaryTarget: null, @@ -689,6 +695,7 @@ describe('typical scenarios', () => { selectedEventId: '1', current: dayInMs, _finishAt: 10 + dayInMs, + _startAt: 10, clock: 10, secondaryTimer: null, secondaryTarget: null, From 8cabb347e9ecd47152ff859692350383e32b805a Mon Sep 17 00:00:00 2001 From: arc-alex Date: Mon, 20 Nov 2023 18:24:11 +0100 Subject: [PATCH 3/9] remove test log --- apps/server/src/services/rollUtils.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/apps/server/src/services/rollUtils.ts b/apps/server/src/services/rollUtils.ts index b3c59c7f7..1e82eecb4 100644 --- a/apps/server/src/services/rollUtils.ts +++ b/apps/server/src/services/rollUtils.ts @@ -183,7 +183,6 @@ export const updateRoll = (currentTimers: CurrentTimers) => { updatedTimer -= dayInMs; } - console.log(Math.floor(_startAt / 1000), Math.floor(clock / 1000)); if (updatedTimer < 0) { isPrimaryFinished = true; From e6b4f537af47a1e9a71bcb8d07af8cbc00e30361 Mon Sep 17 00:00:00 2001 From: arc-alex Date: Wed, 22 Nov 2023 19:05:08 +0100 Subject: [PATCH 4/9] rollSkipLimit --- apps/server/src/config/config.js | 1 + .../src/services/__tests__/rollUtils.test.ts | 61 +++++++++++++++++++ apps/server/src/services/rollUtils.ts | 6 +- 3 files changed, 66 insertions(+), 2 deletions(-) diff --git a/apps/server/src/config/config.js b/apps/server/src/config/config.js index f11da3cdc..42ec30e52 100644 --- a/apps/server/src/config/config.js +++ b/apps/server/src/config/config.js @@ -9,4 +9,5 @@ export const config = { filename: 'override.css', }, restoreFile: 'ontime.restore', + rollSkipLimit: 3 * 32, }; diff --git a/apps/server/src/services/__tests__/rollUtils.test.ts b/apps/server/src/services/__tests__/rollUtils.test.ts index 7c1b4a7c8..ced5f1adc 100644 --- a/apps/server/src/services/__tests__/rollUtils.test.ts +++ b/apps/server/src/services/__tests__/rollUtils.test.ts @@ -2,6 +2,7 @@ import { OntimeEvent } from 'ontime-types'; import { dayInMs } from 'ontime-utils'; import { getRollTimers, normaliseEndTime, sortArrayByProperty, updateRoll } from '../rollUtils.js'; +import { config } from '../../config/config.js'; // test sortArrayByProperty() describe('sort simple arrays of objects', () => { @@ -585,6 +586,66 @@ describe('typical scenarios', () => { expect(updateRoll(timers)).toStrictEqual(expected); }); + it('dose not skip isFinished under limit', () => { + const timers = { + selectedEventId: '1', + current: 13, + _finishAt: 15, + _startAt: 9, + clock: 14, + secondaryTimer: null, + secondaryTarget: null, + }; + + const expected = { + updatedTimer: timers._finishAt - timers.clock, + updatedSecondaryTimer: null, + doRollLoad: false, + isFinished: false, + }; + + expect(updateRoll(timers)).toStrictEqual(expected); + + // test that it can jump time + timers._finishAt = 14; + timers.clock += config.rollSkipLimit - 1; + expected.updatedTimer = timers._finishAt - timers.clock; + expected.doRollLoad = true; + expected.isFinished = true; + + expect(updateRoll(timers)).toStrictEqual(expected); + }); + + it('dose skips isFinished over limit', () => { + const timers = { + selectedEventId: '1', + current: 13, + _finishAt: 15, + _startAt: 9, + clock: 14, + secondaryTimer: null, + secondaryTarget: null, + }; + + const expected = { + updatedTimer: timers._finishAt - timers.clock, + updatedSecondaryTimer: null, + doRollLoad: false, + isFinished: false, + }; + + expect(updateRoll(timers)).toStrictEqual(expected); + + // test that it can jump time + timers._finishAt = 14; + timers.clock += config.rollSkipLimit + 1; + expected.updatedTimer = timers._finishAt - timers.clock; + expected.doRollLoad = true; + expected.isFinished = false; + + expect(updateRoll(timers)).toStrictEqual(expected); + }); + it('it updates secondary timer', () => { const timers = { selectedEventId: null, diff --git a/apps/server/src/services/rollUtils.ts b/apps/server/src/services/rollUtils.ts index 1e82eecb4..67c7363fa 100644 --- a/apps/server/src/services/rollUtils.ts +++ b/apps/server/src/services/rollUtils.ts @@ -1,5 +1,6 @@ import { OntimeEvent } from 'ontime-types'; import { dayInMs } from 'ontime-utils'; +import { config } from '../config/config.js'; /** * handle events that span over midnight @@ -167,6 +168,7 @@ type CurrentTimers = { export const updateRoll = (currentTimers: CurrentTimers) => { const { selectedEventId, current, _finishAt, _startAt, clock, secondaryTimer, secondaryTarget } = currentTimers; + console.log(currentTimers); // timers let updatedTimer = current; let updatedSecondaryTimer = secondaryTimer; @@ -183,12 +185,12 @@ export const updateRoll = (currentTimers: CurrentTimers) => { updatedTimer -= dayInMs; } - if (updatedTimer < 0) { - isPrimaryFinished = true; + if (Math.abs(updatedTimer) < config.rollSkipLimit) isPrimaryFinished = true; //Dont trigger Finished if we are over the skip limit // we need a new event doRollLoad = true; } else if (clock < _startAt) { + // we have rolled back befor this evet start so we need a new one doRollLoad = true; } } else if (secondaryTimer >= 0) { From b11041938d43d821be78c12afeb27c806ccfdbc8 Mon Sep 17 00:00:00 2001 From: arc-alex Date: Fri, 24 Nov 2023 14:05:12 +0100 Subject: [PATCH 5/9] self documenting --- apps/server/src/services/rollUtils.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/apps/server/src/services/rollUtils.ts b/apps/server/src/services/rollUtils.ts index 67c7363fa..3a5cd13c4 100644 --- a/apps/server/src/services/rollUtils.ts +++ b/apps/server/src/services/rollUtils.ts @@ -168,7 +168,6 @@ type CurrentTimers = { export const updateRoll = (currentTimers: CurrentTimers) => { const { selectedEventId, current, _finishAt, _startAt, clock, secondaryTimer, secondaryTarget } = currentTimers; - console.log(currentTimers); // timers let updatedTimer = current; let updatedSecondaryTimer = secondaryTimer; @@ -186,7 +185,10 @@ export const updateRoll = (currentTimers: CurrentTimers) => { } if (updatedTimer < 0) { - if (Math.abs(updatedTimer) < config.rollSkipLimit) isPrimaryFinished = true; //Dont trigger Finished if we are over the skip limit + const hasSkipped = Math.abs(updatedTimer) > config.rollSkipLimit; + if (!hasSkipped) { + isPrimaryFinished = true; + } // we need a new event doRollLoad = true; } else if (clock < _startAt) { From 20d5d8b12927321a6ee2ebfb0a2cb6ac7f76c8c5 Mon Sep 17 00:00:00 2001 From: arc-alex Date: Mon, 27 Nov 2023 17:42:42 +0100 Subject: [PATCH 6/9] move skip logic to timerUtils --- apps/server/src/config/config.js | 2 +- apps/server/src/services/TimerService.ts | 9 +- .../src/services/__tests__/rollUtils.test.ts | 88 ------------------- .../src/services/__tests__/timerUtils.test.ts | 24 ++++- apps/server/src/services/rollUtils.ts | 12 +-- apps/server/src/services/timerUtils.ts | 13 +++ 6 files changed, 45 insertions(+), 103 deletions(-) diff --git a/apps/server/src/config/config.js b/apps/server/src/config/config.js index 42ec30e52..158838f2d 100644 --- a/apps/server/src/config/config.js +++ b/apps/server/src/config/config.js @@ -9,5 +9,5 @@ export const config = { filename: 'override.css', }, restoreFile: 'ontime.restore', - rollSkipLimit: 3 * 32, + timeSkipLimit: 3 * 32, }; diff --git a/apps/server/src/services/TimerService.ts b/apps/server/src/services/TimerService.ts index 64c8ae577..47d096bb0 100644 --- a/apps/server/src/services/TimerService.ts +++ b/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 } from './timerUtils.js'; +import { getCurrent, getExpectedFinish, skipedOutOfEvent } from './timerUtils.js'; import { clock } from './Clock.js'; import { logger } from '../classes/Logger.js'; import type { RestorePoint } from './RestoreService.js'; @@ -339,7 +339,6 @@ export class TimerService { this.timer.expectedFinish >= this.timer.startedAt ? this.timer.expectedFinish : this.timer.expectedFinish + dayInMs, - _startAt: this.timer.startedAt, clock: this.timer.clock, secondaryTimer: this.timer.secondaryTimer, secondaryTarget: this.secondaryTarget, @@ -405,7 +404,11 @@ export class TimerService { let shouldNotify = false; if (this.playback === Playback.Roll) { shouldNotify = true; - this.updateRoll(); + if (skipedOutOfEvent(previousTime, this.timer.clock, this.timer.startedAt, this.timer.finishedAt)) { + PlaybackService.roll(); + } else { + this.updateRoll(); + } } else if (this.timer.startedAt !== null) { // we only update timer if a timer has been started shouldNotify = true; diff --git a/apps/server/src/services/__tests__/rollUtils.test.ts b/apps/server/src/services/__tests__/rollUtils.test.ts index ced5f1adc..329ece996 100644 --- a/apps/server/src/services/__tests__/rollUtils.test.ts +++ b/apps/server/src/services/__tests__/rollUtils.test.ts @@ -2,7 +2,6 @@ import { OntimeEvent } from 'ontime-types'; import { dayInMs } from 'ontime-utils'; import { getRollTimers, normaliseEndTime, sortArrayByProperty, updateRoll } from '../rollUtils.js'; -import { config } from '../../config/config.js'; // test sortArrayByProperty() describe('sort simple arrays of objects', () => { @@ -563,7 +562,6 @@ describe('typical scenarios', () => { selectedEventId: '1', current: 10, _finishAt: 15, - _startAt: 9, clock: 11, secondaryTimer: null, secondaryTarget: null, @@ -586,72 +584,11 @@ describe('typical scenarios', () => { expect(updateRoll(timers)).toStrictEqual(expected); }); - it('dose not skip isFinished under limit', () => { - const timers = { - selectedEventId: '1', - current: 13, - _finishAt: 15, - _startAt: 9, - clock: 14, - secondaryTimer: null, - secondaryTarget: null, - }; - - const expected = { - updatedTimer: timers._finishAt - timers.clock, - updatedSecondaryTimer: null, - doRollLoad: false, - isFinished: false, - }; - - expect(updateRoll(timers)).toStrictEqual(expected); - - // test that it can jump time - timers._finishAt = 14; - timers.clock += config.rollSkipLimit - 1; - expected.updatedTimer = timers._finishAt - timers.clock; - expected.doRollLoad = true; - expected.isFinished = true; - - expect(updateRoll(timers)).toStrictEqual(expected); - }); - - it('dose skips isFinished over limit', () => { - const timers = { - selectedEventId: '1', - current: 13, - _finishAt: 15, - _startAt: 9, - clock: 14, - secondaryTimer: null, - secondaryTarget: null, - }; - - const expected = { - updatedTimer: timers._finishAt - timers.clock, - updatedSecondaryTimer: null, - doRollLoad: false, - isFinished: false, - }; - - expect(updateRoll(timers)).toStrictEqual(expected); - - // test that it can jump time - timers._finishAt = 14; - timers.clock += config.rollSkipLimit + 1; - expected.updatedTimer = timers._finishAt - timers.clock; - expected.doRollLoad = true; - expected.isFinished = false; - - expect(updateRoll(timers)).toStrictEqual(expected); - }); - it('it updates secondary timer', () => { const timers = { selectedEventId: null, current: null, _finishAt: null, - _startAt: null, clock: 11, secondaryTimer: 1, secondaryTarget: 15, @@ -672,7 +609,6 @@ describe('typical scenarios', () => { selectedEventId: '1', current: 10, _finishAt: 11, - _startAt: 9, clock: 12, secondaryTimer: null, secondaryTarget: null, @@ -693,7 +629,6 @@ describe('typical scenarios', () => { selectedEventId: null, current: null, _finishAt: null, - _startAt: null, clock: 16, secondaryTimer: 1, secondaryTarget: 15, @@ -714,7 +649,6 @@ describe('typical scenarios', () => { selectedEventId: null, current: null, _finishAt: null, - _startAt: null, clock: 15, secondaryTimer: 0, secondaryTarget: 15, @@ -735,7 +669,6 @@ describe('typical scenarios', () => { selectedEventId: '1', current: 25, _finishAt: 10 + dayInMs, - _startAt: 10, clock: dayInMs - 10, secondaryTimer: null, secondaryTarget: null, @@ -756,7 +689,6 @@ describe('typical scenarios', () => { selectedEventId: '1', current: dayInMs, _finishAt: 10 + dayInMs, - _startAt: 10, clock: 10, secondaryTimer: null, secondaryTarget: null, @@ -771,24 +703,4 @@ describe('typical scenarios', () => { expect(updateRoll(timers)).toStrictEqual(expected); }); - - it('rolls backwards', () => { - const timers = { - selectedEventId: '1', - current: 11, - _startAt: 10, - _finishAt: 15, - clock: 9, - secondaryTimer: null, - secondaryTarget: null, - }; - - const expected = { - updatedTimer: timers._finishAt - timers.clock, - updatedSecondaryTimer: null, - doRollLoad: true, - isFinished: false, - }; - expect(updateRoll(timers)).toStrictEqual(expected); - }); }); diff --git a/apps/server/src/services/__tests__/timerUtils.test.ts b/apps/server/src/services/__tests__/timerUtils.test.ts index 2a6e15c67..9930da561 100644 --- a/apps/server/src/services/__tests__/timerUtils.test.ts +++ b/apps/server/src/services/__tests__/timerUtils.test.ts @@ -1,7 +1,8 @@ import { dayInMs } from 'ontime-utils'; import { TimerType } from 'ontime-types'; -import { getCurrent, getExpectedFinish } from '../timerUtils.js'; +import { getCurrent, getExpectedFinish, skipedOutOfEvent } from '../timerUtils.js'; +import { config } from '../../config/config.js'; describe('getExpectedFinish()', () => { it('is null if we havent started', () => { @@ -354,3 +355,24 @@ describe('getExpectedFinish() and getCurrentTime() combined', () => { expect(current).toBe(8); }); }); + +describe('skipedOutOfEvent()', () => { + it('without added times, they combine to be duration', () => { + const startedAt = 1000; + const duration = 1000; + const pausedTime = 0; + const finishedAt = null; + const addedTime = 0; + const timerType = TimerType.CountDown; + const expectedFinish = startedAt + duration; + const previousTime = 1000; + let clock = previousTime; + expect(skipedOutOfEvent(previousTime, clock, startedAt, expectedFinish)).toBe(false); + clock += config.timeSkipLimit + 10; + expect(skipedOutOfEvent(previousTime, clock, startedAt, expectedFinish)).toBe(false); + clock = expectedFinish + 1; + expect(skipedOutOfEvent(previousTime, clock, startedAt, expectedFinish)).toBe(true); + clock = startedAt - config.timeSkipLimit - 1; + expect(skipedOutOfEvent(previousTime, clock, startedAt, expectedFinish)).toBe(true); + }); +}); diff --git a/apps/server/src/services/rollUtils.ts b/apps/server/src/services/rollUtils.ts index 3a5cd13c4..98a6feac2 100644 --- a/apps/server/src/services/rollUtils.ts +++ b/apps/server/src/services/rollUtils.ts @@ -1,6 +1,5 @@ import { OntimeEvent } from 'ontime-types'; import { dayInMs } from 'ontime-utils'; -import { config } from '../config/config.js'; /** * handle events that span over midnight @@ -154,7 +153,6 @@ type CurrentTimers = { selectedEventId: string | null; current: number | null; _finishAt: number | null; - _startAt: number | null; clock: number | null; secondaryTimer: number | null; secondaryTarget: number | null; @@ -166,7 +164,7 @@ type CurrentTimers = { * @returns {object} object with selection variables */ export const updateRoll = (currentTimers: CurrentTimers) => { - const { selectedEventId, current, _finishAt, _startAt, clock, secondaryTimer, secondaryTarget } = currentTimers; + const { selectedEventId, current, _finishAt, clock, secondaryTimer, secondaryTarget } = currentTimers; // timers let updatedTimer = current; @@ -185,15 +183,9 @@ export const updateRoll = (currentTimers: CurrentTimers) => { } if (updatedTimer < 0) { - const hasSkipped = Math.abs(updatedTimer) > config.rollSkipLimit; - if (!hasSkipped) { - isPrimaryFinished = true; - } + isPrimaryFinished = true; // we need a new event doRollLoad = true; - } else if (clock < _startAt) { - // we have rolled back befor this evet start so we need a new one - doRollLoad = true; } } else if (secondaryTimer >= 0) { // if secondaryTimer is running we are in waiting to roll diff --git a/apps/server/src/services/timerUtils.ts b/apps/server/src/services/timerUtils.ts index 8edad2a7f..5b402650c 100644 --- a/apps/server/src/services/timerUtils.ts +++ b/apps/server/src/services/timerUtils.ts @@ -1,5 +1,6 @@ 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 @@ -64,3 +65,15 @@ export function getCurrent( } return startedAt + duration + addedTime + pausedTime - clock; } + +export function skipedOutOfEvent(previousTime: number, clock: number, startedAt: number, finishAt): boolean { + const skipTime = previousTime - clock; + const hasSkipped = Math.abs(skipTime) > config.timeSkipLimit; + if (hasSkipped) { + if (clock > finishAt || clock < startedAt) { + return true; + } + // otherwise we just skipped within the event + } + return false; +} From ecd151a0a8be670def75179ef260f1bbf204e320 Mon Sep 17 00:00:00 2001 From: arc-alex Date: Tue, 28 Nov 2023 15:27:19 +0100 Subject: [PATCH 7/9] use expectedFinish --- apps/server/src/services/TimerService.ts | 2 +- apps/server/src/services/timerUtils.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/server/src/services/TimerService.ts b/apps/server/src/services/TimerService.ts index 47d096bb0..2d5a717ac 100644 --- a/apps/server/src/services/TimerService.ts +++ b/apps/server/src/services/TimerService.ts @@ -404,7 +404,7 @@ export class TimerService { let shouldNotify = false; if (this.playback === Playback.Roll) { shouldNotify = true; - if (skipedOutOfEvent(previousTime, this.timer.clock, this.timer.startedAt, this.timer.finishedAt)) { + if (skipedOutOfEvent(previousTime, this.timer.clock, this.timer.startedAt, this.timer.expectedFinish)) { PlaybackService.roll(); } else { this.updateRoll(); diff --git a/apps/server/src/services/timerUtils.ts b/apps/server/src/services/timerUtils.ts index 5b402650c..cd9ce5392 100644 --- a/apps/server/src/services/timerUtils.ts +++ b/apps/server/src/services/timerUtils.ts @@ -66,11 +66,11 @@ export function getCurrent( return startedAt + duration + addedTime + pausedTime - clock; } -export function skipedOutOfEvent(previousTime: number, clock: number, startedAt: number, finishAt): boolean { +export function skipedOutOfEvent(previousTime: number, clock: number, startedAt: number, expectedFinish): boolean { const skipTime = previousTime - clock; const hasSkipped = Math.abs(skipTime) > config.timeSkipLimit; if (hasSkipped) { - if (clock > finishAt || clock < startedAt) { + if (clock > expectedFinish || clock < startedAt) { return true; } // otherwise we just skipped within the event From a67b89190d54b8584ee9c0e68f29ee984caba810 Mon Sep 17 00:00:00 2001 From: arc-alex Date: Tue, 28 Nov 2023 16:04:12 +0100 Subject: [PATCH 8/9] guard and test midnight --- .../src/services/__tests__/timerUtils.test.ts | 83 +++++++++++++++++-- apps/server/src/services/timerUtils.ts | 4 + 2 files changed, 78 insertions(+), 9 deletions(-) diff --git a/apps/server/src/services/__tests__/timerUtils.test.ts b/apps/server/src/services/__tests__/timerUtils.test.ts index 9930da561..204eedf57 100644 --- a/apps/server/src/services/__tests__/timerUtils.test.ts +++ b/apps/server/src/services/__tests__/timerUtils.test.ts @@ -357,22 +357,87 @@ describe('getExpectedFinish() and getCurrentTime() combined', () => { }); describe('skipedOutOfEvent()', () => { - it('without added times, they combine to be duration', () => { + it('normal roll out of event', () => { const startedAt = 1000; const duration = 1000; - const pausedTime = 0; - const finishedAt = null; - const addedTime = 0; - const timerType = TimerType.CountDown; const expectedFinish = startedAt + duration; - const previousTime = 1000; + const previousTime = expectedFinish - config.timeSkipLimit / 2; let clock = previousTime; expect(skipedOutOfEvent(previousTime, clock, startedAt, expectedFinish)).toBe(false); - clock += config.timeSkipLimit + 10; + clock += config.timeSkipLimit; expect(skipedOutOfEvent(previousTime, clock, startedAt, expectedFinish)).toBe(false); - clock = expectedFinish + 1; + }); + + it('normal roll backwards out of event', () => { + const startedAt = 1000; + const duration = 1000; + const expectedFinish = startedAt + duration; + const previousTime = startedAt + config.timeSkipLimit / 2; + let clock = previousTime; + expect(skipedOutOfEvent(previousTime, clock, startedAt, expectedFinish)).toBe(false); + clock -= config.timeSkipLimit; + expect(skipedOutOfEvent(previousTime, clock, startedAt, expectedFinish)).toBe(false); + }); + + it('normal roll out of event over midnight', () => { + const startedAt = dayInMs - config.timeSkipLimit; + 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); + }); + + it('normal roll backwards out of event over midnight', () => { + const startedAt = dayInMs - config.timeSkipLimit; + 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); + }); + + it('skip out of event', () => { + const startedAt = 1000; + const duration = 1000; + const expectedFinish = startedAt + duration; + const previousTime = expectedFinish - config.timeSkipLimit / 2; + let clock = previousTime; + expect(skipedOutOfEvent(previousTime, clock, startedAt, expectedFinish)).toBe(false); + clock += config.timeSkipLimit + 1; expect(skipedOutOfEvent(previousTime, clock, startedAt, expectedFinish)).toBe(true); - clock = startedAt - config.timeSkipLimit - 1; + }); + + it('skip backwards out of event', () => { + const startedAt = 1000; + const duration = 1000; + const expectedFinish = startedAt + duration; + const previousTime = startedAt + config.timeSkipLimit / 2; + let clock = previousTime; + expect(skipedOutOfEvent(previousTime, clock, startedAt, expectedFinish)).toBe(false); + clock -= config.timeSkipLimit + 1; + expect(skipedOutOfEvent(previousTime, clock, startedAt, expectedFinish)).toBe(true); + }); + + it('skip out of event over midnight', () => { + const startedAt = dayInMs - config.timeSkipLimit; + 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); + }); + + it('skip backwards out of event over midnight', () => { + const startedAt = dayInMs - config.timeSkipLimit; + 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); }); }); diff --git a/apps/server/src/services/timerUtils.ts b/apps/server/src/services/timerUtils.ts index cd9ce5392..69e62b058 100644 --- a/apps/server/src/services/timerUtils.ts +++ b/apps/server/src/services/timerUtils.ts @@ -67,8 +67,12 @@ export function getCurrent( } 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; From 34e9b1ef074961a060e8535ee16e034efda14041 Mon Sep 17 00:00:00 2001 From: Carlos Valente Date: Thu, 30 Nov 2023 21:35:09 +0100 Subject: [PATCH 9/9] refactor: clarify responsabilities --- apps/server/src/config/config.js | 1 - apps/server/src/services/TimerService.ts | 26 +++-- .../src/services/__tests__/timerUtils.test.ts | 102 ++++++++++-------- apps/server/src/services/timerUtils.ts | 30 +++--- 4 files changed, 94 insertions(+), 65 deletions(-) diff --git a/apps/server/src/config/config.js b/apps/server/src/config/config.js index 158838f2d..f11da3cdc 100644 --- a/apps/server/src/config/config.js +++ b/apps/server/src/config/config.js @@ -9,5 +9,4 @@ export const config = { filename: 'override.css', }, restoreFile: 'ontime.restore', - timeSkipLimit: 3 * 32, }; diff --git a/apps/server/src/services/TimerService.ts b/apps/server/src/services/TimerService.ts index 2d5a717ac..aa939161c 100644 --- a/apps/server/src/services/TimerService.ts +++ b/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; +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 }); diff --git a/apps/server/src/services/__tests__/timerUtils.test.ts b/apps/server/src/services/__tests__/timerUtils.test.ts index 204eedf57..acb17cc1a 100644 --- a/apps/server/src/services/__tests__/timerUtils.test.ts +++ b/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); }); }); diff --git a/apps/server/src/services/timerUtils.ts b/apps/server/src/services/timerUtils.ts index 69e62b058..a657389f5 100644 --- a/apps/server/src/services/timerUtils.ts +++ b/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); }