Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Stale clock information sent into mesh #3000

Closed
wnagele opened this issue Dec 8, 2023 · 3 comments
Closed

[Bug]: Stale clock information sent into mesh #3000

wnagele opened this issue Dec 8, 2023 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@wnagele
Copy link
Contributor

wnagele commented Dec 8, 2023

Category

Other

Hardware

Not Applicable

Firmware Version

up to 2.2.15

Description

I have recently noticed that a lot of nodes in our mesh are sending stale time information into the mesh. This leads to lots of other nodes then syncing onto this incorrect time information.

I have captured some datapoints below - where you can see that these nodes keep sending the same time information over and over again. You can see how the difference to the real time now keeps growing over time.

I have worked on a fix for this here: wnagele@41c190c

I would however appreciate a review of someone more familiar with this code. I am not sure if the real time update should actually be fed in elsewhere. I have just fixed it in the spot where the incorrect information is normally sent into the mesh.

There is another minor fix in there where ignoreRequest wasn't reset and so got stuck whenever that occured until some other event might have cleared it.

One more question that arose for me - right now the code accepts time syncs from anywhere. Would it make sense to limit this to our "trusted" primary channel only? I have added that in my commit too.

Let me know what you think.

Relevant log output

from: SENDING NODE, time: TIME RECEIVED, diff: DIFFERENCE FROM TIME RIGHT NOW

from: !fa6d08f8, time: 2023-11-19 06:20:06, diff: 18 days, 12:47:29.067933
from: !fa6d08f8, time: 2023-11-19 06:20:06, diff: 18 days, 13:02:29.902294
from: !fa6d08f8, time: 2023-11-19 06:20:06, diff: 18 days, 13:17:29.752081
from: !fa6d08f8, time: 2023-11-19 06:20:06, diff: 18 days, 13:32:30.057950
from: !fa6d08f8, time: 2023-11-19 06:20:06, diff: 18 days, 13:47:31.241409
from: !fa6d08f8, time: 2023-11-19 06:20:06, diff: 18 days, 14:17:32.508820
from: !fa6d08f8, time: 2023-11-19 06:20:06, diff: 18 days, 14:32:33.105596
from: !fa6d08f8, time: 2023-11-19 06:20:06, diff: 18 days, 14:47:33.258429
from: !fa6d08f8, time: 2023-11-19 06:20:06, diff: 18 days, 15:02:34.702572
from: !fa6d08f8, time: 2023-11-19 06:20:06, diff: 18 days, 15:17:36.329697

from: !fa6f532c, time: 2023-12-07 08:24:35, diff: 10:45:08.159637
from: !fa6f532c, time: 2023-12-07 08:24:35, diff: 11:15:09.369683
from: !fa6f532c, time: 2023-12-07 08:24:35, diff: 11:30:07.115887
from: !fa6f532c, time: 2023-12-07 08:24:35, diff: 11:45:09.732262
from: !fa6f532c, time: 2023-12-07 08:24:35, diff: 12:00:07.652349
from: !fa6f532c, time: 2023-12-07 08:24:35, diff: 12:15:07.414195
from: !fa6f532c, time: 2023-12-07 08:24:35, diff: 12:30:11.834437
from: !fa6f532c, time: 2023-12-07 08:24:35, diff: 12:45:07.088218
from: !fa6f532c, time: 2023-12-07 08:24:35, diff: 13:00:11.042360
from: !fa6f532c, time: 2023-12-07 08:24:35, diff: 13:15:07.402393

from: !fa6636b8, time: 2023-11-27 16:08:08, diff: 10 days, 3:04:35.376944
from: !fa6636b8, time: 2023-11-27 16:08:08, diff: 10 days, 3:34:36.694854
from: !fa6636b8, time: 2023-11-27 16:08:08, diff: 10 days, 5:19:40.808976
from: !fa6636b8, time: 2023-11-27 16:08:08, diff: 10 days, 5:34:41.842136

from: !fa43807c, time: 2023-12-04 13:46:53, diff: 3 days, 5:37:58.481612
from: !fa43807c, time: 2023-12-04 13:46:53, diff: 3 days, 5:52:58.740817
from: !fa43807c, time: 2023-12-04 13:46:53, diff: 3 days, 6:07:59.355247
from: !fa43807c, time: 2023-12-04 13:46:53, diff: 3 days, 6:22:59.924280
from: !fa43807c, time: 2023-12-04 13:46:53, diff: 3 days, 6:38:01.268041
from: !fa43807c, time: 2023-12-04 13:46:53, diff: 3 days, 6:53:01.601181
from: !fa43807c, time: 2023-12-04 13:46:53, diff: 3 days, 7:08:02.538953

from: !fa6cf444, time: 2023-11-22 06:21:13, diff: 15 days, 13:37:20.742901
from: !fa6cf444, time: 2023-11-22 06:21:13, diff: 15 days, 13:52:21.215575
from: !fa6cf444, time: 2023-11-22 06:21:13, diff: 15 days, 14:07:23.827822
from: !fa6cf444, time: 2023-11-22 06:21:13, diff: 15 days, 14:37:22.089279
from: !fa6cf444, time: 2023-11-22 06:21:13, diff: 15 days, 15:07:23.529582
@wnagele wnagele added the bug Something isn't working label Dec 8, 2023
@thebentern
Copy link
Contributor

Looks PR-worthy :-)

@GUVWAF
Copy link
Member

GUVWAF commented Dec 23, 2023

@wnagele Looks good to me as well. Since indeed localPosition.time may not be available, it makes sense to me to use getValidTime() if the RTC quality is not met. Can you create a Pull Request with your changes?

One more question that arose for me - right now the code accepts time syncs from anywhere. Would it make sense to limit this to our "trusted" primary channel only? I have added that in my commit too.

That's a matter of preference and I think this makes sense.

@wnagele wnagele mentioned this issue Dec 24, 2023
@GUVWAF
Copy link
Member

GUVWAF commented Dec 26, 2023

Fixed by #3035.

@GUVWAF GUVWAF closed this as completed Dec 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants