Skip to content

fix: ignore bad timesources. ignore data that is older than our last good value - #19

Open
herostrat wants to merge 3 commits into
SignalK:masterfrom
herostrat:timeval
Open

herostrat wants to merge 3 commits into
SignalK:masterfrom
herostrat:timeval

Conversation

@herostrat

@herostrat herostrat commented Feb 15, 2026

Copy link
Copy Markdown

This PR adds persistence for the last known good datetime and uses it to guard against bad GPS timestamps.
It also introduces a hard minimum year to prevent obvious time resets.

This "minimum last good time" is stored in the data in the plugin’s data.
This way e.g bad fixes in GPS don't result in bogus data.

should be a fix for #10
and probably #14 also?

@tkurki tkurki left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.....and what do we do with last known good value? Why is it saved?

Comment thread index.js Outdated

const minimumYear = 2026

function getDataDir() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@herostrat

Copy link
Copy Markdown
Author

.....and what do we do with last known good value? Why is it saved?

We use it to "update" the lower range (I choose 2026)

Comment thread index.js
const lastGoodDate = new Date(lastGoodTime)
if (!Number.isNaN(lastGoodDate.getTime()) && Date.now() < lastGoodDate.getTime()) {
const useSudoFallback = typeof options.sudo === 'undefined' || options.sudo
setSystemTime(lastGoodTime, useSudoFallback, 'from last-good time')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this sets the time from last good time on plugin start, right? I don't think that is a good idea, as think of scenario where

  • no GPS datetime available
  • sk server has been off for a day
  • you start sk server
    => time travel to yesterday.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, thats a good point. I didn't think about this scenario.
Yeah I will change that so it doesn't set the system time but only guards against incoming (older) data.

This way wrong data should be ignored and new incoming good data updates the time to a new good value.

@tkurki

tkurki commented Feb 15, 2026

Copy link
Copy Markdown
Member

It would be great if PR title & description would describe the change on a level that an end user can understand, if possible. I am not perfect in that regard, but i try...

@herostrat herostrat changed the title fix: sanity check date and save last known good value fix: ignore bad timesources. ignore data that is older than our last good value Feb 15, 2026
@herostrat

Copy link
Copy Markdown
Author

Yeah that's fair, sorry. I wasn't expecting that fast of an review and am currently trying to find a way to test this.

@tkurki

tkurki commented Feb 15, 2026

Copy link
Copy Markdown
Member

lol, sometimes quick, sometimes takes ages

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants