search

NSLS-II / nslsii

NSLS-II related devices
BSD 3-Clause "New" or "Revised" License
10 stars 21 forks source link

updates for new xspress3 IOC #113

Closed jklynch closed 2 years ago

jklynch commented 3 years ago

This PR adds new classes for the latest xspress3 IOC. The existing xspress3 classes remain unchanged.

codecov-io commented 3 years ago

Codecov Report

Merging #113 (bd1c794) into master (0e4e463) will decrease coverage by 31.63%. The diff coverage is 12.50%.

:exclamation: Current head bd1c794 differs from pull request most recent head 015c9a7. Consider uploading reports for the commit 015c9a7 to get more accurate results Impacted file tree graph

@@             Coverage Diff             @@
##           master     #113       +/-   ##
===========================================
- Coverage   58.63%   26.99%   -31.64%     
===========================================
  Files          13       15        +2     
  Lines         938      963       +25     
===========================================
- Hits          550      260      -290     
- Misses        388      703      +315
Impacted Files Coverage Δ
nslsii/areadetector/__init__.py 100.00% <ø> (ø)
nslsii/tests/test_xspress3.py 12.50% <12.50%> (ø)
nslsii/tests/test_ipynb.py 12.50% <0.00%> (-87.50%) :arrow_down:
nslsii/tests/test_logutils.py 16.16% <0.00%> (-82.64%) :arrow_down:
nslsii/tests/test_kafka_publisher.py 21.73% <0.00%> (-71.02%) :arrow_down:
nslsii/common/ipynb/logutils.py 35.71% <0.00%> (-57.15%) :arrow_down:
nslsii/common/ipynb/info.py 15.00% <0.00%> (-40.00%) :arrow_down:
nslsii/temperature_controllers.py 26.53% <0.00%> (-36.74%) :arrow_down:
nslsii/tests/temperature_controllers_test.py 37.50% <0.00%> (-31.25%) :arrow_down:
nslsii/__init__.py 11.62% <0.00%> (-25.59%) :arrow_down:
... and 2 more

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 0e4e463...015c9a7. Read the comment docs.

codecov-commenter commented 3 years ago

Codecov Report

Merging #113 (bd1c794) into master (9cd77e0) will decrease coverage by 30.67%. The diff coverage is 12.50%.

:exclamation: Current head bd1c794 differs from pull request most recent head f38e6b2. Consider uploading reports for the commit f38e6b2 to get more accurate results Impacted file tree graph

@@             Coverage Diff             @@
##           master     #113       +/-   ##
===========================================
- Coverage   57.67%   26.99%   -30.68%     
===========================================
  Files          13       15        +2     
  Lines         938      963       +25     
===========================================
- Hits          541      260      -281     
- Misses        397      703      +306
Impacted Files Coverage Δ
nslsii/areadetector/__init__.py 100.00% <ø> (ø)
nslsii/tests/test_xspress3.py 12.50% <12.50%> (ø)
nslsii/tests/test_ipynb.py 12.50% <0.00%> (-87.50%) :arrow_down:
nslsii/tests/test_logutils.py 16.16% <0.00%> (-82.64%) :arrow_down:
nslsii/tests/test_kafka_publisher.py 21.73% <0.00%> (-65.22%) :arrow_down:
nslsii/common/ipynb/logutils.py 35.71% <0.00%> (-57.15%) :arrow_down:
nslsii/common/ipynb/info.py 15.00% <0.00%> (-40.00%) :arrow_down:
nslsii/temperature_controllers.py 26.53% <0.00%> (-36.74%) :arrow_down:
nslsii/tests/temperature_controllers_test.py 37.50% <0.00%> (-31.25%) :arrow_down:
nslsii/__init__.py 11.62% <0.00%> (-25.59%) :arrow_down:
... and 3 more

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 e3db7da...f38e6b2. Read the comment docs.

jklynch commented 2 years ago

@tacaswell @jwlodek this code supports "step scans" but is still missing a functional flyer. BMM is using this code in production so I really want to release it, which is why I'm asking for a review now. Under different circumstances I would try to get an example flyer in to the first release but we are having trouble getting time with the hardware, so I would like to get this much released soon.

jklynch commented 2 years ago

@mrakitin On the subject of CI this PR was started so long ago I don't think we had even begun to move to GitHub Actions. That will have to wait for another PR. If you can point to example tests using a simulated IOC that would be helpful.

mrakitin commented 2 years ago

@mrakitin On the subject of CI this PR was started so long ago I don't think we had even begun to move to GitHub Actions. That will have to wait for another PR. If you can point to example tests using a simulated IOC that would be helpful.

I see. I don't know a good example of the simulated IOC for xspress3, I thought you were using your caproto IOC code for that...

jklynch commented 2 years ago

@mrakitin thank you for the thorough review!

prjemian commented 2 years ago

On 8/5/2021 3:26 PM, jklynch wrote:

After consulting the oracle I believe |put| is the correct method here since it is part of the ophyd EPICS interface, while |set| is intended for a higher level client such as bluesky.

Thanks for this clarity. To this day, I've been confusing and conflating the two.

method use
.put() ophyd
.set() bluesky plans