Don't create blank frame when starting an acquisition and EraseOnStart is enabled - #87
Don't create blank frame when starting an acquisition and EraseOnStart is enabled#87ericonr wants to merge 3 commits into
Conversation
e5e93a8 to
d6a8360
Compare
this name more accurately reflects the function's purpose. it also avoids encoding in the driver code knowledge about the existence of ROI plugins connected to it.
while clearing API buffers and driver statistics can be reasonable to do when starting an acquisition, also sending a blank frame is not. when acquisition is started, clients like the HDF5 and ROI plugins are already enabled, so they will process this blank frame, leading to user confusion and data loss. for the HDF5 plugin, the resulting file will have an initial frame with empty data, and, if it's configured without knowledge of the empty frame, the last real frame will be discarded. for ROI plugins, CA clients should be able to expect a number of monitor events equal to the number of acquisitions, leading to similar issues. the blank frame creation was added by 734caa0 (update code to better match APS-update, but with trig_b / deadtime corrections, 2016-09-28). EraseOnStart was enabled by default for deployments using the provided startup snippets by 8354d65 (add dbpf for EraseOnStart to start as Yes, 2021-09-08). fix this issue by adding a sendFrame parameter to erase() and clearDriverAndPlugins(). This allows them to differentiate between being called by someone writing to the Erase PV (where we want to send a blank frame) and someone writing to the Acquire PV when EraseOnStart is enabled (where we don't want a blank frame).
d6a8360 to
17ff1bf
Compare
|
Rebase done! |
|
@ericonr this is still a pretty big set of changes. I would want to test it before merging, but the only xspress3 I have access to is in production into August. It would be nice if QD could test this on multiple systems. |
|
It is my understanding that this doesn't touch anything for specific hardware, but I would appreciate further testing, for sure. @TomQD-94 do you think you could test this? |
|
@newville I split the changes into dedicated commits for each part. That might make it easier to review! https://github.com/epics-modules/xspress3/pull/87/commits |
|
@ericonr Yes, thanks, I don't disagree with that. The changes look fine to me - I don't have any objections, but the changes to some of the signatures give me a little pause, that perhaps some unidentified corner of the code may need adjusting. If someone else is willing to try this out and give it an OK, I'm fine with merging. I won't be able to do that myself for a few weeks. I don't expect there to be problems and have no objections to the changes (in fact, great!! thanks for finding and fixing this annoying misfeature!!), just being cautious. |
2 participants
This PR is based on the formatting performed in #86
Fixes #53