search

fermi-ad / acsys-python

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

acsys.sync ClockEvent missing delay #8

Closed beauremus closed 3 years ago

beauremus commented 3 years ago

@kjhazelwood: It looks like acsys.sync allows a full event request with delay but its not apparent if the system is doing anything with the delay because of the one second resolution. I understand this is likely backed by the clock multicast so the event return is not "real-time" but you could offset the timestamp given by the multicast by the given delay mS to give the user a accurate timestamp. Or, like the java controls does, grab the timestamp of the clockevent, update the timestamp as timestamp+delay, and also compare the clock timestamp to the local clock and then wait for the difference of mS (if delay > 66mS) before returning the clock event message to the user to appear to be "realtime". Though in reality this only works if the delay is greater than 15Hz and is always susceptible to the two machine clocks being apart in time. Maybe a flag for this functionality?

beauremus commented 3 years ago

@kjhazelwood: Also, the clock event class should have a field for delay.

beauremus commented 3 years ago

I tested this after 82144b5413c6ecec623433f5f58629cfac264cdc, and things appear to be working as expected. Let me know if we're still missing something from your request.

We should create a new issue for adding the delay field because it isn't supported in the protocol message.

beauremus commented 3 years ago

@rneswold: Is the code written such that the timestamp is already the clock's timestamp added to the delay? I think I figured that, if someone wanted and $02 event plus 1/2 second, they'd be more interested in the absolute timestamp of the combined event instead of the timestamp of the $02. (plus, I know what delay I added so I could always subtract it from the combined timestamp -- if I really needed just the clock's time.)

rneswold commented 3 years ago

The timestamp is provided by the SYNCD service, which combines the timestamp of the clock event with the delay. I suppose we could add a DRF parser which can deconstruct the event, but I still think -- if you ask for event + delay -- that you're interested in the final timestamp.

I think this issue can be closed, unless anyone wants to continue the discussion.

beauremus commented 3 years ago

@kjhazelwood Let us know if this is ok to close.

kjhazelwood commented 3 years ago

Yes, I believe the returned timestamp being the event timestamp + delay is the correct thing to do. Thanks!