search

fermi-ad / acsys-python

Python module to access the Fermilab Control System
MIT License
8 stars 4 forks source link

Sync timestamps are offset naive #5

Closed rneswold closed 3 years ago

rneswold commented 3 years ago

@kjhazelwood: DPM returns UTC timestamps that are offset aware. I think this is appropriate. Sync returns UTC offset naive timestamps making the two not comparable unless you convert the naive timestamps using pytz.

rneswold commented 3 years ago

Interesting that I chose different methods to convert the timestamp. 🤨

In both protocols, the timestamp is sent over as a 64-bit integer representing UTC milliseconds since Jan 1, 1970 (i.e. "the Epoch".)

In the acsys.dpm package, I convert it to a datetime like so:

delta = datetime.timedelta(seconds=stamp // 1000,
                           microseconds=(stamp % 1000) * 1000)
tz = datetime.timezone.utc
self._stamp = datetime.datetime(1970, 1, 1, tzinfo=tz) + delta

In acsys.sync, it's done this way:

delta = datetime.timedelta(milliseconds=jj.stamp)
epoch = datetime.datetime.utcfromtimestamp(0)
stamp = epoch + delta

@kjhazelwood, is the problem that there isn't any timezone information in the acsys.sync timestamp? I'll copy DPM's algorithm into the SYNC's code.

kjhazelwood commented 3 years ago

@rneswold I think you are correct, the sync timestamps were missing a tz so were tz naive. I see you pushed a new version, I'll try to test today or tommorrow, thanks.

kjhazelwood commented 3 years ago

I'm getting the follow stacktrace while using the sync service (acsys 0.11.2):

Traceback (most recent call last):
  File "collect_toy_data.py", line 287, in <module>
    args.logging_level)
  File "collect_toy_data.py", line 96, in main
    acsys.run_client(run)
  File "/home/kjh/.local/lib/python3.6/site-packages/acsys/__init__.py", line 791, in run_client
    loop.run_until_complete(client_fut)
  File "/home/kjh/.local/lib/python3.6/site-packages/nest_asyncio.py", line 70, in run_until_complete
    return f.result()
  File "/usr/lib64/python3.6/asyncio/tasks.py", line 182, in _step
    result = coro.throw(exc)
  File "/home/kjh/.local/lib/python3.6/site-packages/acsys/__init__.py", line 769, in __client_main
    await main(con, **kwargs)
  File "collect_toy_data.py", line 222, in run
    async for msg in s:
  File "/usr/local/lib/python3.6/site-packages/aiostream/stream/advanced.py", line 59, in base_combine
    result = task.result()
  File "/usr/lib64/python3.6/asyncio/tasks.py", line 180, in _step
    result = coro.send(None)
  File "/home/kjh/.local/lib/python3.6/site-packages/acsys/sync/__init__.py", line 142, in get_events
    delta = datetime.timedelta(milliseconds=stamp)
UnboundLocalError: local variable 'stamp' referenced before assignment
beauremus commented 3 years ago

I believe v0.11.3 (with 8fe50cef21c4d79707af6cbae96c67dfadda8c00) will fix your issue. Let us know how your test goes!

kjhazelwood commented 3 years ago

Thanks @beauremus, 0.11.3 did indeed fix the error. I'm running my collection script again, I'll compare the timestamps later this afternoon.

rneswold commented 3 years ago

I believe this issue is resolved. The timestamps are being built exactly the same way.

kjhazelwood commented 3 years ago

Sorry for the late response. Yes, this is resolved. Thanks!