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

ENH: pass the count parameter along, as already documented. #1137

Merged
merged 3 commits into from
Oct 13, 2023
Merged
Changes from all commits
Commits
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
33 changes: 29 additions & 4 deletions ophyd/signal.py
Original file line number Diff line number Diff line change
Expand Up @@ -1283,7 +1283,14 @@ def limits(self):
return (self._metadata["lower_ctrl_limit"], self._metadata["upper_ctrl_limit"])

def _get_with_timeout(
self, pv, timeout, connection_timeout, as_string, form, use_monitor
self,
pv,
timeout,
connection_timeout,
count,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit torn, on one hand adding new positionals in the middle is a source of latent bugs, but on the other hand all of the positionals are required so any usage is fully broken (TypeError: missing parameter) rather than the subtle "all your inputs are shifted".

On net I think it is 👍🏻 , just set off alarms in my head.

as_string,
form,
use_monitor,
):
"""
Utility method implementing a retry loop for get and get_setpoint
Expand Down Expand Up @@ -1316,7 +1323,11 @@ def _get_with_timeout(
timeout,
)
info = pv.get_with_metadata(
as_string=as_string, form=form, timeout=timeout, use_monitor=use_monitor
as_string=as_string,
count=count,
form=form,
timeout=timeout,
use_monitor=use_monitor,
)
self.control_layer_log.debug(
"pv[%s].get_with_metadata(...) returned", pv.pvname
Expand All @@ -1332,6 +1343,7 @@ def _get_with_timeout(
def get(
self,
*,
count=None,
as_string=None,
timeout=DEFAULT_TIMEOUT,
connection_timeout=DEFAULT_CONNECTION_TIMEOUT,
Expand Down Expand Up @@ -1376,7 +1388,13 @@ def get(
use_monitor = self._auto_monitor

info = self._get_with_timeout(
self._read_pv, timeout, connection_timeout, as_string, form, use_monitor
self._read_pv,
timeout,
connection_timeout,
count,
as_string,
form,
use_monitor,
)

value = info.pop("value")
Expand Down Expand Up @@ -1866,6 +1884,7 @@ def check_value(self, value):
def get_setpoint(
self,
*,
count=None,
as_string=None,
timeout=DEFAULT_TIMEOUT,
connection_timeout=DEFAULT_CONNECTION_TIMEOUT,
Expand Down Expand Up @@ -1909,7 +1928,13 @@ def get_setpoint(
use_monitor = self._auto_monitor

info = self._get_with_timeout(
self._write_pv, timeout, connection_timeout, as_string, form, use_monitor
self._write_pv,
timeout,
connection_timeout,
count,
as_string,
form,
use_monitor,
)

value = info.pop("value")
Expand Down