Bug in setSNTP implementation

[espruino-discuss3]

Posted at 2019-01-29 by SergeP

I've found bug in setSNTP implementation. Now I have simple example. Also I think I know reason.

var wifi=require("Wifi");

var i1=setInterval(()=>console.log(1),2000);
var i2=setInterval(()=>console.log(2),3333);

setTimeout(()=>{
  wifi.setSNTP('pool.ntp.org','0');
},7000);

After setSNTP call both intervals works as if they are with zero delay.
I think it happens because SNTP subsystem uses settimeofday() inside by default and so does not change interval times.
It is possible to redefine SNTP_SET_SYSTEM_TIME macro to call jshSetSystemTime() instead of settimeofday(). I think it will repair the bug. I think also lwipopts.h is good place for it.

[espruino-discuss3]

Posted at 2019-01-30 by Wilberforce

Isn't this because when the espruino starts its at time 0 - 1970 and then you are setting the time - so the intervals immediately expire?

What if you set the time first and then set the intervals?

@gfwilliams - any ideas?

---

Posted at 2019-01-30 by SergeP

Isn't this because when the espruino starts its at time 0 - 1970 and then you are setting the time - so the intervals immediately expire?

Yes. And settimeofday() does not correct them, so they try to catch up the time.
So we need to change it to jshSetSystemTime(), I think. SNTP app allows us to redefine function used to set system time. But I am not sure how to do it with minimal changes.

What if you set the time first and then set the intervals?

It depends. Because SNTP subsystem is asynchronous and so time may be really changed before or after intervals setup. It checks connection, for example, and repeats NTP request every few minutes or every hour.

---

Posted at 2019-01-30 by Wilberforce

So we need to change it to jshSetSystemTime(), I think

I don't understand why this would make a difference? Do you think this would update existing intervals?

---

Posted at 2019-01-31 by SergeP

Sorry, I was wrong. I had not read jshSetSystemTime() implementation and based on fact that my JS SNTP module calls it and works. Now I see that truth is deeper. Now I do not see where intervals are recalculated after time change.

---

Posted at 2019-01-31 by @gfwilliams

Maybe just call jswrap_interactive_setTime?

---

Posted at 2019-01-31 by Wilberforce

@gfwilliams

Will calling jswrap_interactive_setTime adjust the timeout values?

---

Posted at 2019-01-31 by SergeP

@gfwilliams, @wilberforce
Now I do not understand why some of my code still works. :)

I see that jsiLastIdleTime should be changed when we change system time (by any way) to intervals remain in correct state.
But I do not see the change in code, so I understand why setSNTP does not work, but I do not understand why intervals are not broken after I call setTime() in JS. May be it happens (by some unclear way) because I call it from interval or timeout callback?
Now it seems to me there is may be a big general bug in Espruino. What do you think about that?

---

Posted at 2019-02-01 by @gfwilliams

Yes - https://github.com/espruino/Espruino/blob/master/src/jswrap_interactive.c#L372

The timeouts work by repeated decrement based on jsiLastIdleTime, so as long as that is set it's ok.

I do not understand why intervals are not broken after I call setTime() in JS

Because setTime calls jswrap_interactive_setTime which I mentioned above, which sets jsiLastIdleTime

---

Posted at 2019-02-01 by SergeP

@gfwilliams, thank you! Now I see jsiLastIdleTime change!
May be I had written wrong search template.

@wilberforce, it looks like it is easiest way to call jswrap_interactive_setTime(). Or may be to implement similar own function if you prefer to set system time with millisecond precision.

---

Posted at 2019-02-02 by Wilberforce

Issue added here:

espruino/Espruino#1612