ImperialCollegeLondon / FINESSE

A graphical user interface for controlling and monitoring an interferometer device
BSD 3-Clause "New" or "Revised" License
3 stars 1 forks source link

DECADES module breaks if empty values returned #536

Closed alexdewar closed 7 months ago

alexdewar commented 8 months ago

I tried running the code for reading sensor values from DECADES that's in main and found that it tended to fall over at random. I printed the raw JSON to see what was happening:

2024-04-03 14:08:29,604 [INFO] Opening device of type decades: Decades
2024-04-03 14:08:29,605 [INFO] Opened device
{'static_pressure': [1007.4050826044669], 'gin_altitude': [154.807468], 'deiced_true_air_temp_c': [20.46769125783345], 'utc_time': [1712149707.0]}
{'static_pressure': [1007.4050826044669], 'gin_altitude': [154.807271], 'deiced_true_air_temp_c': [20.481509967947034], 'utc_time': [1712149708.0]}
{'static_pressure': [1007.4050826044669], 'gin_altitude': [154.807055], 'deiced_true_air_temp_c': [20.50504382928949], 'utc_time': [1712149709.0]}
{'static_pressure': [1007.4050826044669], 'gin_altitude': [154.80696], 'deiced_true_air_temp_c': [20.50851948987787], 'utc_time': [1712149710.0]}
{'static_pressure': [1007.4050826044669], 'gin_altitude': [154.806857], 'deiced_true_air_temp_c': [20.51949088454154], 'utc_time': [1712149711.0]}
{'static_pressure': [1007.4050826044669], 'gin_altitude': [154.806655], 'deiced_true_air_temp_c': [20.566559562269333], 'utc_time': [1712149712.0]}
{'static_pressure': [1007.4050826044669], 'gin_altitude': [154.806397], 'deiced_true_air_temp_c': [20.507346977321447], 'utc_time': [1712149713.0]}
{'static_pressure': [1007.4050826044669], 'gin_altitude': [154.806195], 'deiced_true_air_temp_c': [20.54951590645652], 'utc_time': [1712149714.0]}
{'static_pressure': [], 'gin_altitude': [], 'deiced_true_air_temp_c': [], 'utc_time': []}
2024-04-03 14:08:37,637 [ERROR] Error with device device.decades: Traceback (most recent call last):
  File "/home/alex/code/FINESSE/finesse/hardware/device.py", line 297, in wrapped
    result = func(*args, **kwargs)
             ^^^^^^^^^^^^^^^^^^^^^
  File "/home/alex/code/FINESSE/finesse/hardware/plugins/decades/decades.py", line 56, in _on_reply_received
    return get_decades_data(content)
           ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/alex/code/FINESSE/finesse/hardware/plugins/decades/decades.py", line 36, in get_decades_data
    raise DecadesError(f"Unexpected number of values for sensor {sensor}")
finesse.hardware.plugins.decades.decades.DecadesError: Unexpected number of values for sensor static_pressure

It seems that the server sometimes sends empty arrays for values, which triggers an error condition in get_decades_data. I'm not sure what the expected behaviour is for DECADES (I see one of the Daves sent through a manual, but I haven't read it). Whatever it is supposed to do though, in practice it does seem to send empty arrays semi-regularly so we need to handle this somehow (e.g. by retrying). Any thoughts @jamesturner246?

I've tried @dc2917 's draft PR (#527), which seems to change the DECADES core code among other things. I dunno whether that was done in order to fix this issue or if it's just a happy accident. (It might be that the problem is still there though -- I didn't run that code for long.)

dc2917 commented 8 months ago

Yeah, in my PR I change the time to query from time.time() to time.time() - DECADES_POLL_INTERVAL.

It actually queries data in the time interval frm=epoch_time&to=epoch_time, where epoch_time=time.time() on main vs epoch_time=time.time()-DECADES_POLL_INTERVAL on my branch.

In both cases, the "frm" and "to" times are the same, so only one value should be returned, but if DECADES's update time is out of sync with our poll interval, there is no new data since the "frm" time, so nothing is returned. If we always check since the latest poll time, we should always have new data to obtain.

Maybe we should actually be doing something like

start_time = time.time() - DECADES_POLL_INTERVAL
end_time = time.time()

and querying frm=start_time&to=end_time, and making sure to just select the last data point, in case multiple values are available from that interval.

jamesturner246 commented 7 months ago

In an ideal world, DECADES would have an interface to return the most recent values. Maybe something to request for DECADES v2.

Incidentally, time.time() should be replaced by hardware.plugins.time.get_time(). I'll do it in #535.