Implementation of stimulator encoder agent / driver. #612
Conversation
dixilo
changed the title
…s into jsuzuki/stimulator/encoder
…sobs/socs into jsuzuki/stimulator/encoder
for more information, see https://pre-commit.ci
socs/agents/stm_encoder/agent.py
Outdated
| en_st_list.append(_d.state) | ||
|
|
||
| if len(ts_list) != 0: | ||
| data['timestamps'] = ts_list |
There was a problem hiding this comment.
This should go into data['data']['timestamps'].
There was a problem hiding this comment.
This makes sense for the session.data object, but since data is being used in the self.agent.publish_to_feed() call 'timestamps' needs to be at this higher level. See https://ocs.readthedocs.io/en/main/developer/feeds.html#recorded-feed-message-format.
socs/agents/stm_encoder/agent.py
Outdated
| while self.take_data: | ||
| # Data acquisition | ||
| current_time = time.time() | ||
| data = {'timestamps': [], 'block_name': 'stm_enc', 'data': {}} |
There was a problem hiding this comment.
Change to standard timestamp key in the data dictionary, then move the list of timestamps into data (see below).
| data = {'timestamps': [], 'block_name': 'stm_enc', 'data': {}} | |
| current_time = time.time() | |
| data = {'timestamp': current_time, 'block_name': 'stm_enc', 'data': {}} |
There was a problem hiding this comment.
Related to above, plural 'timestamps' is important for the publish to feed call, and when publishing a list of points, needs to also contain a list for the associated value.
|
Great to see this an apologies if these two concerns are premature:
|
|
Thanks for the review!
Data rate is around 250 Hz when rotated at the maximum speed. Do we need separate feeds for aggregator / influx for this situation? |
Thanks for the comment. I'm planning to implement the routine to get utc-tai dynamically. |
Yes, InfluxDB doesn't handle high data rates well. You can publish full data rates to a feed with The Labjack agent has a decent example of setting up the split feeds: socs/socs/agents/labjack/agent.py Lines 206 to 227 in 784371f |
I decided not to include UTC - TAI conversion and only to provide raw TAI values since it is difficult to implement the behavior at the leap second insertion. |
| data = {'timestamp': current_time, 'block_name': 'stim_enc', 'data': {}} | ||
|
|
||
| ts_list = [] | ||
| en_st_list = [] | ||
|
|
||
| while not self._dev.fifo.empty(): | ||
| _d = self._dev.fifo.get() | ||
| ts_list.append(_d.time.tai) | ||
| en_st_list.append(_d.state) | ||
|
|
||
| if len(ts_list) != 0: | ||
| data['data']['timestamps_tai'] = ts_list | ||
| data['data']['state'] = en_st_list | ||
|
|
||
| field_dict = {'timestamp_tai': ts_list[-1], | ||
| 'state': en_st_list[-1]} | ||
|
|
||
| session.data.update(field_dict) | ||
|
|
||
| self.agent.publish_to_feed('stim_enc', data) | ||
| session.data.update({'timestamp': current_time}) | ||
|
|
||
| if current_time - downsample_time > 0.1: | ||
| data_downsampled = {'timestamp': current_time, | ||
| 'block_name': 'stim_enc_downsampled', | ||
| 'data': { | ||
| 'timestamps_tai': ts_list[-1], | ||
| 'state': en_st_list[-1] | ||
| }} | ||
| self.agent.publish_to_feed('stim_enc_downsampled', | ||
| data_downsampled) | ||
| downsample_time = current_time |
There was a problem hiding this comment.
Sorry, I think there's some confusion about the format for publishing to data feeds and for session.data. (Edit: Docs for this format are here.)
For each publish_to_feed call, the format for the message argument needs to be either:
message = {
'block_name': <Key to identify group of co-sampled data>
'timestamp': <ctime of data>
'data': {
'field_name_1': <datapoint1>,
'field_name_2': <datapoint2>
}
}
if your fields are singular data points, or:
message = {
'block_name': <Key to identify group of co-sampled data>
'timestamps': [ctime1, ctime2, ... ]
'data': {
'field_name_1': [data1_1, data1_2, ...],
'field_name_2': [data2_1, data2_2, ...]
}
}
if you're publishing a list of points for each field. So 'timestamp' becomes 'timestamps' and is a list.
For the high rate data, as written currently, it looks like the format is:
message = {
'block_name': 'stim_enc',
'timestamp': current_time
'data': {
'timestamps_tai': [data1_1, data1_2, ...],
'state': [data2_1, data2_2, ...]
}
}
This might cause an issue in the HK aggregator. @mhasself any thoughts on the timestamps published being in TAI here?
There was a problem hiding this comment.
Putting the timestamps into a data field called timestamps_tai is a good solution.
As @BrianJKoopman says, you need to also record a full vector of timestamps, in message['timestamps']. The most appropriate thing would be to grab the system time using time.time() in the while not self._dev.fifo.empty() loop. (Those timestamps are not high precision but they're still useful and relevant... e.g. you can use them to make a very good guess of the [UTC - TAI] offset in processing, later.)
There was a problem hiding this comment.
I have two more general comments on the data feeds here.
In the "slow" feed, for grafana, it will not be that useful to have a heavily downsampled version of the raw data. I would be much more interested for a regular published value of the spin frequency, computed from the last ~2 seconds of data. Instead of 10 Hz, just do 0.5 Hz and do a pulse count / time delta.
Furthermore -- for that slow feed, it's useful to publish the frequency even when the thing isn't spinning. That is helpful to distinguish between "there were no encoder pulses" and "the agent/process wasn't running".
If the slow feed has spin frequency, and is always running ... then I want it in the Aggregator / permanent HK data too!
So ya -- I think you should just record the fast data to aggregator, and then record a slower feed with spin frequency, including when freq=0, for both influx + aggregator.
(In the slow feed, instead of timestamps_tai you could record the most recent value of (timestamp_tai - timestamp) -- that will hover around the current UTC offset (37 or something).)
| while not self._dev.fifo.empty(): | ||
| _d = self._dev.fifo.get() | ||
| ts_list.append(_d.time.tai) | ||
| en_st_list.append(_d.state) |
There was a problem hiding this comment.
How big is the device fifo? Like, how often do you need to clear it out?
The current loop time of 100 Hz means there will be a lot of pythony stuff happening, at 100 Hz. If this is running on a small computer, it could use a lot more CPU than necessary. Is there a way you could empty the fifo less often -- perhaps at 1 Hz, and only call "publish_to_feed" once you have a whole 1 second of data? These efficiencies might not matter; but if you find the agent is doing a lot of work, it will help to increase the size of each data chunk that gets processed.
There was a problem hiding this comment.
Thank you for pointing out. We don't need 100 Hz here at all.
FIFO depth inside the FPGA firmware is 512 and the loop here is taking it into account.
Since the fifo accessed from the agent is a Queue object on Python and the device has 4GB DDR4 memory, 1 Hz reading perfectly works.
I will simply reduce the loop time to 0.5 Hz so that we don't need to care about the different loop time between fast and slow feeds.
Description
This pull request adds reader for stimulator encoder.
This agent run in Kria KR260 inside the stimulator electronics box.
This reader accesses a custom-made IP inside the PL of ZynqMP device.
The firmware code can be found in https://github.com/dixilo/kr260_stm.
Motivation and Context
These codes will be used to read encoder of the stimulator to know exact rotation speed and timing of the signal chopping.
How Has This Been Tested?
We tested the functionality using the stimulator in Princeton.
Types of changes
Checklist: