NSLS-II / nslsii

NSLS-II related devices

BSD 3-Clause "New" or "Revised" License

10 stars 21 forks source link

MNT: do not try to erase spectrum in trigger #97

Closed tacaswell closed 3 years ago

tacaswell commented 3 years ago

This has been observed to lock up the IOC for ~2s.

We applied this fix in the profile at BMM with @bruceravel

codecov-commenter commented 3 years ago

Codecov Report

Merging #97 into master will increase coverage by 0.56%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master      #97      +/-   ##
==========================================
+ Coverage   56.61%   57.17%   +0.56%     
==========================================
  Files          13       13              
  Lines         892      892              
==========================================
+ Hits          505      510       +5     
+ Misses        387      382       -5

Impacted Files	Coverage Δ
nslsii/_version.py	`44.80% <0.00%> (+1.79%)`	:arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 18c0632...8eda86b. Read the comment docs.

tacaswell commented 3 years ago

I also added a commit to make it clear that you need to set the total number of points before the device can be used.

mrakitin commented 3 years ago

That's an unexpected source of the issue. Nice work on figuring it out!

mrakitin commented 3 years ago

Can we claim it closes #96?

tacaswell commented 3 years ago

Following discussion with @jwlodek it looks like pushing erase on every trigger is redundant. The function that gets called internally allocates 0 arrays, pushes them through the AD pipeline and resets the histograms. However, the function called when we push the acquire button also resets the histograms and pushes the newly collected data down epics pipe.

https://github.com/epics-modules/xspress3/blob/39832f64ff7f246b2f6dcb91b6ff63a7e8f8cbfd/xspress3App/src/xspress3Epics.cpp#L868

https://github.com/epics-modules/xspress3/blob/39832f64ff7f246b2f6dcb91b6ff63a7e8f8cbfd/xspress3App/src/xspress3Epics.cpp#L1161

mrakitin commented 3 years ago

I think this fix should go in before the next push. @tacaswell, are there any concerns left here?

tacaswell commented 3 years ago

no, I don't think so.

tacaswell commented 3 years ago

This may break things, but if it does it is because it was already broken we just did not know it!

mrakitin commented 3 years ago

Thank you!