pcdshub / pcdsdaq

Utilities for using the DAQ's pydaq, pycdb, and pyami libraries in conjunction with Bluesky
https://pcdshub.github.io/pcdsdaq
Other
0 stars 9 forks source link

ENH: Stop begin(wait=True) on ctrl+c #25

Closed ZLLentz closed 6 years ago

ZLLentz commented 6 years ago

Description

A user can escape daq.begin(wait=True) using ctrl+c, which will stop the run.

Motivation and Context

Previously, this did not stop the run, which caused confusion because the run was still going and another run could not be started.

closes #20

How Has This Been Tested?

Added tests and tested in sxr

Where Has This Been Documented?

Edited docstrings and added to release notes.

codecov-io commented 6 years ago

Codecov Report

Merging #25 into master will increase coverage by <.01%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #25      +/-   ##
==========================================
+ Coverage   99.46%   99.47%   +<.01%     
==========================================
  Files           2        2              
  Lines         377      381       +4     
==========================================
+ Hits          375      379       +4     
  Misses          2        2
Impacted Files Coverage Δ
pcdsdaq/daq.py 99.71% <100%> (ø) :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 8845bd5...77ab550. Read the comment docs.

teddyrendahl commented 6 years ago

Why not put this in the wait method itself? Won't if a user CTRL+C there we'll have the same issue?

ZLLentz commented 6 years ago

I assumed that ctrl+c during begin wants to interrupt the daq, while ctrl+c during the wait wants to interrupt the wait.

teddyrendahl commented 6 years ago

Hmm. Have to think about this. First reaction is having differing behavior is confusing.

ZLLentz commented 6 years ago

I agree that differing behavior is confusing, but I also think something like:

daq.begin(duration=20)
daq.wait()
^C

Would be confusing if it DID stop the run. The SIGINT should stop doing whatever the current running command did.

We can sleep on it and talk later. This is how the old python handled it (which is not to say that the old python did things optimally).

teddyrendahl commented 6 years ago

Yeah. I'll buy that. Also if thats one the old one does this I think consistency might be more important than what is ultimately more intuitive for a brand new user.

ZLLentz commented 6 years ago

I agree. There is also one more acceptable option: we can make ctrl+c just cancel the wait in both cases, but then show an INFO message reminding the user that the run is still going (though I expect this will be annoying for users).

teddyrendahl commented 6 years ago

I like the original suggestion better.