Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add settle time as property of Signal to be referenced in set method #1074

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

maffettone
Copy link

Currently settle time only exists as an attribute in Positioners and not Signals. However, Signal.set() uses this as a keyword argument, which obscures it from bps.mv or other plans. This adds settle_time as a property of Signal, that can be overridden in the call to set(). It keeps the defaults as None, and should maintain backward compatibility with existing devices.

ophyd/signal.py Outdated Show resolved Hide resolved
@tacaswell
Copy link
Contributor

We also need a test of this. We have a couple example of using time.monotonic() to run timing tests to make sure things are really sleeping when we expect.

@maffettone
Copy link
Author

Is there a described protocol for developing and running pytest? I'm running the bash scripts that get run in GHA and hoping those will create the containers needed to pass pytest on master.

@prjemian
Copy link
Contributor

Tu run tests locally, consider using https://github.com/nektos/act which runs the tests in a local VM (docker?) as they would run in GHA.

@coretl
Copy link
Collaborator

coretl commented Sep 21, 2022

Note to self: this needs to be included in #1063

@maffettone
Copy link
Author

Coming back to this, even using act, I end up with a few failed tests on the master branch.

I'll work from this basis as I build the tests for this, and ensure coverage doesn't decrease.

| ---------- coverage: platform linux, python 3.8.14-final-0 -----------
| Name                                           Stmts   Miss  Cover
| ------------------------------------------------------------------
| ophyd/__init__.py                                 64     22    66%
| ophyd/__main__.py                                  9      0   100%
| ophyd/_caproto_shim.py                            81     81     0%
| ophyd/_dispatch.py                               116      6    95%
| ophyd/_dummy_shim.py                              36     36     0%
| ophyd/_pyepics_shim.py                            96      7    93%
| ophyd/_version.py                                  2      2     0%
| ophyd/areadetector/__init__.py                     9      0   100%
| ophyd/areadetector/base.py                       228      1    99%
| ophyd/areadetector/cam.py                       1102      5    99%
| ophyd/areadetector/common_plugins.py              89      0   100%
| ophyd/areadetector/detectors.py                  110      9    92%
| ophyd/areadetector/docs.py                         1      0   100%
| ophyd/areadetector/filestore_mixins.py           255    103    60%
| ophyd/areadetector/paths.py                       49      7    86%
| ophyd/areadetector/plugins.py                   1126     56    95%
| ophyd/areadetector/trigger_mixins.py             116     52    55%
| ophyd/areadetector/util.py                        87     72    17%
| ophyd/callbacks.py                                37     37     0%
| ophyd/device.py                                  680     66    90%
| ophyd/docs/__init__.py                             3      0   100%
| ophyd/docs/templates/__init__.py                   3      0   100%
| ophyd/docs/templates/autosummary/__init__.py       0      0   100%
| ophyd/docs/utils.py                               68     14    79%
| ophyd/epics_motor.py                             153     21    86%
| ophyd/flyers.py                                  210    109    48%
| ophyd/log.py                                      97     24    75%
| ophyd/mca.py                                     248     16    94%
| ophyd/mixins.py                                   67      7    90%
| ophyd/ophydobj.py                                230     10    96%
| ophyd/positioner.py                              146      6    96%
| ophyd/pseudopos.py                               381     47    88%
| ophyd/pv_positioner.py                           193     17    91%
| ophyd/quadem.py                                   82      0   100%
| ophyd/scaler.py                                   88     26    70%
| ophyd/signal.py                                  774     45    94%
| ophyd/sim.py                                     743    146    80%
| ophyd/status.py                                  287     23    92%
| ophyd/tests/__init__.py                            1      0   100%
| ophyd/tests/config.py                              7      0   100%
| ophyd/tests/conftest.py                           93      5    95%
| ophyd/tests/fake_motor_ioc.py                     33      0   100%
| ophyd/tests/mca_ioc.py                           118      0   100%
| ophyd/tests/scaler_ioc.py                        154      0   100%
| ophyd/tests/signal_ioc.py                         27      0   100%
| ophyd/tests/test_areadetector.py                 421     91    78%
| ophyd/tests/test_bluesky_protocols.py             66     10    85%
| ophyd/tests/test_device.py                       475      6    99%
| ophyd/tests/test_docs.py                          14      0   100%
| ophyd/tests/test_epicsmotor.py                   206      5    98%
| ophyd/tests/test_flyers.py                       123     66    46%
| ophyd/tests/test_hints.py                         35      0   100%
| ophyd/tests/test_issue944.py                      29      0   100%
| ophyd/tests/test_kind.py                         292      0   100%
| ophyd/tests/test_log.py                           44      0   100%
| ophyd/tests/test_main.py                           6      0   100%
| ophyd/tests/test_mca.py                          104      0   100%
| ophyd/tests/test_ophydobj.py                     131      0   100%
| ophyd/tests/test_positioner.py                    69      0   100%
| ophyd/tests/test_pseudopos.py                    262     78    70%
| ophyd/tests/test_pvpositioner.py                 216      2    99%
| ophyd/tests/test_quadem.py                        60      1    98%
| ophyd/tests/test_scaler.py                        84      7    92%
| ophyd/tests/test_signal.py                       442      0   100%
| ophyd/tests/test_signalpositioner.py              54      0   100%
| ophyd/tests/test_sim.py                          225      1    99%
| ophyd/tests/test_status.py                       307      1    99%
| ophyd/tests/test_timeout.py                       26      0   100%
| ophyd/tests/test_timestamps.py                    61      0   100%
| ophyd/tests/test_units.py                         46      0   100%
| ophyd/tests/test_utils.py                         76      0   100%
| ophyd/tests/test_versioning.py                    23      0   100%
| ophyd/units.py                                    73     24    67%
| ophyd/utils/__init__.py                           82     23    72%
| ophyd/utils/epics_pvs.py                         171      4    98%
| ophyd/utils/errors.py                             28      0   100%
| ophyd/utils/paths.py                              26      1    96%
| ophyd/utils/startup.py                             8      3    62%
| ophyd/v2/__init__.py                               0      0   100%
| ------------------------------------------------------------------
| TOTAL                                          12784   1401    89%
| Coverage XML written to file cov.xml
| 
| =========================== short test summary info ============================
| FAILED ophyd/tests/test_areadetector.py::test_fstiff_plugin[pyepics-None-/tmp/ophyd_AD_test/data1/%Y/%m/%d-None-False0]
| FAILED ophyd/tests/test_areadetector.py::test_fstiff_plugin[pyepics-None-/tmp/ophyd_AD_test/data1/%Y/%m/%d-None-False1]
| FAILED ophyd/tests/test_areadetector.py::test_fstiff_plugin[pyepics-/tmp/ophyd_AD_test/data1-%Y/%m/%d-None-False]
| FAILED ophyd/tests/test_areadetector.py::test_fstiff_plugin[pyepics-/tmp/ophyd_AD_test/data1-/tmp/ophyd_AD_test/data1/%Y/%m/%d-%Y/%m/%d-False]
| FAILED ophyd/tests/test_areadetector.py::test_fstiff_plugin[pyepics-/-/tmp/ophyd_AD_test/data1/%Y/%m/%d-None-False]
| FAILED ophyd/tests/test_areadetector.py::test_fshdf_plugin[pyepics-None-/tmp/ophyd_AD_test/data1/%Y/%m/%d-None-False0]
| FAILED ophyd/tests/test_areadetector.py::test_fshdf_plugin[pyepics-None-/tmp/ophyd_AD_test/data1/%Y/%m/%d-None-False1]
| FAILED ophyd/tests/test_areadetector.py::test_fshdf_plugin[pyepics-/tmp/ophyd_AD_test/data1-%Y/%m/%d-None-False]
| FAILED ophyd/tests/test_areadetector.py::test_fshdf_plugin[pyepics-/tmp/ophyd_AD_test/data1-/tmp/ophyd_AD_test/data1/%Y/%m/%d-%Y/%m/%d-False]
| FAILED ophyd/tests/test_areadetector.py::test_fshdf_plugin[pyepics-/-/tmp/ophyd_AD_test/data1/%Y/%m/%d-None-False]
| FAILED ophyd/tests/test_flyers.py::test_monitor_flyer[pyepics-True] - Failed:...
| FAILED ophyd/tests/test_flyers.py::test_monitor_flyer[pyepics-False] - Failed...
| FAILED ophyd/tests/test_pseudopos.py::test_multi_sequential[pyepics] - Failed...
| FAILED ophyd/tests/test_pseudopos.py::test_multi_concurrent[pyepics] - Failed...
| FAILED ophyd/tests/test_pseudopos.py::test_single_pseudo[pyepics] - Failed: T...
| = 15 failed, 1745 passed, 124 skipped, 1885 deselected, 1 xfailed in 468.75s (0:07:48) =
[Code CI/test-1]   ❌  Failure - Main Test with pytest
[Code CI/test-1] exitcode '1': failure
[Code CI/test-1] 🏁  Job failed

@tacaswell
Copy link
Contributor

It looks like something on CI got faster and we are no longer actually going through re-try loop 🤦🏻 .

@tacaswell
Copy link
Contributor

Can you rebase to get rid of the merge commit?

@tacaswell
Copy link
Contributor

The py39 failure is failure to start/connect to the AD IOC

Co-authored-by: Thomas A Caswell <[email protected]>
@prjemian
Copy link
Contributor

This approved PR languishes. Can it be merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants