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.

Sign up for GitHub

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

Jump to bottom

EpicsSignal.set() timeout should default to EpicsSignal.timeout #926

Merged
merged 11 commits into from
Nov 3, 2020
Merged

EpicsSignal.set() timeout should default to EpicsSignal.timeout #926

Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
5d4372e
fixes #925
prjemian Oct 27, 2020
9e061c2
MNT #926 add signal name to log
prjemian Oct 27, 2020
26012f9
MNT #926 per review
prjemian Oct 27, 2020
c94f3ae
MNT #926 per discussion
prjemian Oct 27, 2020
b37b3bb
MNT #926 per review
prjemian Oct 27, 2020
41b9a86
MNT #926 add set_default_timeout(write_timeout)
prjemian Oct 28, 2020
ca6b49d
MNT #926 per discussion
prjemian Oct 28, 2020
766ead5
MNT #926 typo
prjemian Oct 28, 2020
a72669f
ENH #927 per #926
prjemian Oct 30, 2020
30289e8
MNT #927 deprecate old interface
prjemian Oct 30, 2020
79adcc1
MNT #926 per review
prjemian Nov 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 71 additions & 43 deletions ophyd/signal.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@
)


# Sentinels used for default values; see set_default_timeout below for details.
# Sentinels used for default values; see set_defaults() below for details.
DEFAULT_AUTO_MONITOR = object()
DEFAULT_CONNECTION_TIMEOUT = object()
DEFAULT_TIMEOUT = object()
DEFAULT_WRITE_TIMEOUT = object()
Expand Down Expand Up @@ -287,20 +288,20 @@ def set_thread():
except TimeoutError:
success = False
self.log.warning(
'set_and_wait(value=%s, timeout=%s, atol=%s, rtol=%s)',
value, timeout, self.tolerance, self.rtolerance
'%s: set_and_wait(value=%s, timeout=%s, atol=%s, rtol=%s)',
self.name, value, timeout, self.tolerance, self.rtolerance
)
except Exception:
success = False
self.log.exception(
'set_and_wait(value=%s, timeout=%s, atol=%s, rtol=%s)',
value, timeout, self.tolerance, self.rtolerance
'%s: set_and_wait(value=%s, timeout=%s, atol=%s, rtol=%s)',
self.name, value, timeout, self.tolerance, self.rtolerance
)
else:
success = True
self.log.info(
'set_and_wait(value=%s, timeout=%s, atol=%s, rtol=%s) succeeded => %s',
value, timeout, self.tolerance, self.rtolerance, self._readback)
'%s: set_and_wait(value=%s, timeout=%s, atol=%s, rtol=%s) succeeded => %s',
self.name, value, timeout, self.tolerance, self.rtolerance, self._readback)

if settle_time is not None:
self.log.info('settling for %d seconds', settle_time)
Expand Down Expand Up @@ -681,7 +682,7 @@ class EpicsSignalBase(Signal):
only applied if the PV is connected within connection_timeout (below).

The default value DEFAULT_TIMEOUT means, "Fall back to class-wide
default." See EpicsSignalBase.set_default_timeout to configure class
default." See EpicsSignalBase.set_defaults to configure class
defaults.

Explicitly passing None means, "Wait forever."
Expand All @@ -702,7 +703,7 @@ class EpicsSignalBase(Signal):
a channel.

The default value DEFAULT_CONNECTION_TIMEOUT means, "Fall back to
class-wide default." See EpicsSignalBase.set_default_timeout to
class-wide default." See EpicsSignalBase.set_defaults to
configure class defaults.

Explicitly passing None means, "Wait forever."
Expand All @@ -712,9 +713,11 @@ class EpicsSignalBase(Signal):
# instantiation.
__any_instantiated = False

# See set_default_timeout for more on these.
# See set_defaults() for more on these.
__default_connection_timeout = 1.0
__default_timeout = 2.0
__default_timeout = 2.0 # *read* timeout
__default_write_timeout = None # Wait forever.
__default_auto_monitor = False

_read_pv_metadata_key_map = dict(
status=('status', AlarmStatus),
Expand All @@ -734,7 +737,9 @@ class EpicsSignalBase(Signal):
)
)

def __init__(self, read_pv, *, string=False, auto_monitor=False, name=None,
def __init__(self, read_pv, *, string=False,
auto_monitor=DEFAULT_AUTO_MONITOR,
name=None,
metadata=None, all_pvs=None,
timeout=DEFAULT_TIMEOUT,
write_timeout=DEFAULT_WRITE_TIMEOUT,
Expand All @@ -744,25 +749,21 @@ def __init__(self, read_pv, *, string=False, auto_monitor=False, name=None,
self._read_pv = None
self._read_pvname = read_pv
self._string = bool(string)
self._auto_monitor = auto_monitor

self._signal_is_ready = threading.Event()
self._first_connection = True

if auto_monitor is DEFAULT_AUTO_MONITOR:
auto_monitor = self.__default_auto_monitor
self._auto_monitor = auto_monitor
if connection_timeout is DEFAULT_CONNECTION_TIMEOUT:
connection_timeout = self.__default_connection_timeout
self._connection_timeout = connection_timeout
if timeout is DEFAULT_TIMEOUT:
timeout = self.__default_timeout
self._timeout = timeout
if write_timeout is DEFAULT_WRITE_TIMEOUT:
# This is very different than the connection and read timeouts
# above. It relates to how long an action takes to complete. Any
# default value we choose here is likely to cause problems---either
# by being too short and giving up too early on a lengthy action or
# being too long and delaying the report of a failure.
# The important thing it is that it is configurable at per-Signal
# level via the write_timeout parameter.
write_timeout = None # Wait forever.
write_timeout = self.__default_write_timeout
self._write_timeout = write_timeout

if name is None:
Expand Down Expand Up @@ -821,34 +822,44 @@ def _mark_as_instantiated(cls):
cls.__any_instantiated = True

@classmethod
def set_default_timeout(cls, *, timeout=2.0, connection_timeout=1.0):
def set_defaults(cls,
*,
timeout=2.0,
connection_timeout=1.0,
write_timeout=2,
auto_monitor=False):
"""
Set the class-wide defaults for timeouts
Set class-wide defaults for EPICS CA communications

This may only be called before any instances of EpicsSignalBase are
This may be called only before any instances of EpicsSignalBase are
made.

This setting applies to the class it is called on and all its
subclasses. For example,

>>> EpicsSignalBase.set_default_timeout(...)
>>> EpicsSignalBase.set_defaults(...)

will apply to ``EpicsSignalRO`` and ``EpicsSignal``, which are both
subclasses of ``EpicsSignalBase``.

but

>>> EpicsSignal.set_default_timeout(...)
>>> EpicsSignal.set_defaults(...)

will not apply to ``EpicsSignalRO``.

Parameters
----------
timeout: float, optional
Total time budget (seconds) for reading, not including connection time.
auto_monitor: bool, optional
If ``True``, update cached value from EPICS CA monitor callbacks.
If ``False``, request new value from EPICS each time get() is called.
connection_timeout: float, optional
Time (seconds) allocated for establishing a connection with the
IOC.
timeout: float, optional
Total time budget (seconds) for reading, not including connection time.
write_timeout: float, optional
Time (seconds) allocated for writing, not including connection time.

Raises
------
Expand All @@ -858,15 +869,31 @@ def set_default_timeout(cls, *, timeout=2.0, connection_timeout=1.0):
"""
if EpicsSignalBase.__any_instantiated:
raise RuntimeError(
"The method EpicsSignalBase.set_default_timeout may only "
"The method EpicsSignalBase.set_defaults may only "
"be called before the first instance of EpicsSignalBase is "
"created. This is to ensure that all instances are created "
"with the same default retry setting in place.")
"with the same default settings in place.")

cls.__default_auto_monitor = auto_monitor
cls.__default_connection_timeout = connection_timeout
cls.__default_timeout = timeout
# The write_timeout is very different than the connection and read timeouts
# above. It relates to how long an action takes to complete. Any
# default value we choose here is likely to cause problems---either
# by being too short and giving up too early on a lengthy action or
# being too long and delaying the report of a failure.
cls.__default_write_timeout = write_timeout

# TODO Is there a good reason to prohibit setting these three timeout
# properties?
# TODO Is there a good reason to prohibit setting these three timeout
# properties?

@classmethod
def set_default_timeout(cls, **kwargs):
warnings.warn(
"set_default_timeout() will be removed "
"in a future release. Use set_defaults() instead."
)
cls.set_defaults(**kwargs)

@property
def connection_timeout(self):
Expand Down Expand Up @@ -1229,7 +1256,7 @@ class EpicsSignalRO(EpicsSignalBase):
only applied if the PV is connected within connection_timeout (below).

The default value DEFAULT_TIMEOUT means, "Fall back to class-wide
default." See EpicsSignalBase.set_default_timeout to configure class
default." See EpicsSignalBase.set_defaults to configure class
defaults.

Explicitly passing None means, "Wait forever."
Expand All @@ -1250,16 +1277,14 @@ class EpicsSignalRO(EpicsSignalBase):
a channel.

The default value DEFAULT_CONNECTION_TIMEOUT means, "Fall back to
class-wide default." See EpicsSignalBase.set_default_timeout to
class-wide default." See EpicsSignalBase.set_defaults to
configure class defaults.

Explicitly passing None means, "Wait forever."
'''

def __init__(self, read_pv, *, string=False, auto_monitor=False, name=None,
**kwargs):
super().__init__(read_pv, string=string, auto_monitor=auto_monitor,
name=name, **kwargs)
def __init__(self, read_pv, *, string=False, name=None, **kwargs):
super().__init__(read_pv, string=string, name=name, **kwargs)
self._metadata['write_access'] = False

def put(self, *args, **kwargs):
Expand Down Expand Up @@ -1320,7 +1345,7 @@ class EpicsSignal(EpicsSignalBase):
only applied if the PV is connected within connection_timeout (below).

The default value DEFAULT_TIMEOUT means, "Fall back to class-wide
default." See EpicsSignalBase.set_default_timeout to configure class
default." See EpicsSignalBase.set_defaults to configure class
defaults.

Explicitly passing None means, "Wait forever."
Expand All @@ -1341,7 +1366,7 @@ class EpicsSignal(EpicsSignalBase):
a channel.

The default value DEFAULT_CONNECTION_TIMEOUT means, "Fall back to
class-wide default." See EpicsSignalBase.set_default_timeout to
class-wide default." See EpicsSignalBase.set_defaults to
configure class defaults.

Explicitly passing None means, "Wait forever."
Expand All @@ -1365,7 +1390,7 @@ class EpicsSignal(EpicsSignalBase):
)

def __init__(self, read_pv, write_pv=None, *, put_complete=False,
string=False, limits=False, auto_monitor=False, name=None,
string=False, limits=False, name=None,
**kwargs):

self._write_pv = None
Expand All @@ -1386,7 +1411,7 @@ def __init__(self, read_pv, write_pv=None, *, put_complete=False,

self._setpoint_pvname = write_pv

super().__init__(read_pv, string=string, auto_monitor=auto_monitor,
super().__init__(read_pv, string=string,
name=name, metadata=metadata,
all_pvs={read_pv, write_pv}, **kwargs)

Expand Down Expand Up @@ -1685,7 +1710,7 @@ def put(self, value, force=False,
self._run_subs(sub_type=self.SUB_SETPOINT, old_value=old_value,
value=value, timestamp=timestamp)

def set(self, value, *, timeout=None, settle_time=None):
def set(self, value, *, timeout=DEFAULT_WRITE_TIMEOUT, settle_time=None):
'''Set is like `EpicsSignal.put`, but is here for bluesky compatibility

If put completion is used for this EpicsSignal, the status object will
Expand All @@ -1711,6 +1736,9 @@ def set(self, value, *, timeout=None, settle_time=None):
--------
Signal.set
'''
if timeout is DEFAULT_WRITE_TIMEOUT:
timeout = self.write_timeout

if not self._put_complete:
return super().set(value, timeout=timeout, settle_time=settle_time)

Expand Down