-
Notifications
You must be signed in to change notification settings - Fork 22
Description
Hi
I'm just looking through the parsing code, and noticed this code in util to create a timestamp. There's a shift to deal with delayed packets:
if result > reference_timestamp + timedelta(hours=12):
# shift timestamp to previous day
result -= timedelta(days=1)
elif result < reference_timestamp - timedelta(hours=12):
# shift timestamp to next day
result += timedelta(days=1)
I understand shifting a day back, but I'm curious why the logic to shift forwards up to 12 hours into the future?
I noticed at some point only 30 minutes into the future was allowed: 0569956
But when the AmbigousTimeError was removed, the 30minutes became 12 hours: a1f84c6
I appreciate a 12 hour delay may be rare, but I wonder if the parsing code should only put packets up to 20 seconds (plus a bit) into the future, given that's how frequently the server sends status message?
I created this test case:
('000001h', datetime(2015, 1, 10, 12, 0, 2), datetime(2015, 1, 10, 0, 0, 1)),
Which fails:
✗ 000001h + ref 2015-01-10T12:00:02 -> 2015-01-11T00:00:01 (expected 2015-01-10T00:00:01)
I think I'd prefer a packet to be marked as 12hrs and 1 second delayed, than to appear to be 12 hours into the future.
It would be reasonable for packets that appear one minute or so past the last server timestamp to be considered "the future", rather than 23hrs 59mins late, but not much more than that.
I'm happy to submit a PR, but wanted to check the reasoning first.