Skip to content
Open
Show file tree
Hide file tree
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
3 changes: 1 addition & 2 deletions src/ophyd_async/core/_signal.py
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,6 @@ class SignalW(Signal[SignalDatatypeT], Movable):
async def set(
self,
value: SignalDatatypeT,
wait=True,
timeout: CalculatableTimeout = CALCULATE_TIMEOUT,
) -> None:
"""Set the value and return a status saying when it's done.
Expand All @@ -301,7 +300,7 @@ async def set(
):
with attempt:
await _wait_for(
self._connector.backend.put(value, wait=wait), timeout, source
self._connector.backend.put(value, wait=True), timeout, source
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
self._connector.backend.put(value, wait=True), timeout, source
self._connector.backend.put(value), timeout, source

Copy link
Collaborator

Choose a reason for hiding this comment

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

Then can you remove all trace of the wait argument from all SignalBackends

)
self.log.debug(f"Successfully put value {value} to backend at source {source}")

Expand Down
13 changes: 12 additions & 1 deletion src/ophyd_async/epics/core/_aioca.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,12 @@
wait_for_connection,
)

from ._util import EpicsSignalBackend, format_datatype, get_supported_values
from ._util import (
EpicsOptions,
EpicsSignalBackend,
format_datatype,
get_supported_values,
)

logger = logging.getLogger("ophyd_async")

Expand Down Expand Up @@ -255,10 +260,12 @@ def __init__(
datatype: type[SignalDatatypeT] | None,
read_pv: str = "",
write_pv: str = "",
options: EpicsOptions | None = None,
):
self.converter: CaConverter = DisconnectedCaConverter(float, dbr.DBR_DOUBLE)
self.initial_values: dict[str, AugmentedValue] = {}
self.subscription: Subscription | None = None
self.options = options or EpicsOptions()
self._all_updates = _all_updates()
super().__init__(datatype, read_pv, write_pv)

Expand Down Expand Up @@ -304,6 +311,10 @@ async def put(self, value: SignalDatatypeT | None, wait: bool):
write_value = self.initial_values[self.write_pv]
else:
write_value = self.converter.write_value(value)
if value in self.options.no_wait_when_setting:
wait = False
else:
wait = self.options.wait
try:
await caput(
self.write_pv,
Expand Down
13 changes: 12 additions & 1 deletion src/ophyd_async/epics/core/_p4p.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,12 @@
wait_for_connection,
)

from ._util import EpicsSignalBackend, format_datatype, get_supported_values
from ._util import (
EpicsOptions,
EpicsSignalBackend,
format_datatype,
get_supported_values,
)

logger = logging.getLogger("ophyd_async")

Expand Down Expand Up @@ -347,10 +352,12 @@ def __init__(
datatype: type[SignalDatatypeT] | None,
read_pv: str = "",
write_pv: str = "",
options: EpicsOptions | None = None,
):
self.converter: PvaConverter = DisconnectedPvaConverter(float)
self.initial_values: dict[str, Any] = {}
self.subscription: Subscription | None = None
self.options = options or EpicsOptions()
super().__init__(datatype, read_pv, write_pv)

def source(self, name: str, read: bool):
Expand Down Expand Up @@ -385,6 +392,10 @@ async def put(self, value: SignalDatatypeT | None, wait: bool):
write_value = self.initial_values[self.write_pv]["value"]
else:
write_value = self.converter.write_value(value)
if value in self.options.no_wait_when_setting:
wait = False
else:
wait = self.options.wait
await context().put(self.write_pv, {"value": write_value}, wait=wait)

async def get_datakey(self, source: str) -> DataKey:
Expand Down
34 changes: 27 additions & 7 deletions src/ophyd_async/epics/core/_signal.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
get_unique,
)

from ._util import EpicsSignalBackend, get_pv_basename_and_field
from ._util import EpicsOptions, EpicsSignalBackend, get_pv_basename_and_field


class EpicsProtocol(Enum):
Expand Down Expand Up @@ -78,14 +78,22 @@


def _epics_signal_backend(
datatype: type[SignalDatatypeT] | None, read_pv: str, write_pv: str
datatype: type[SignalDatatypeT] | None,
read_pv: str,
write_pv: str,
options: EpicsOptions | None,
) -> SignalBackend[SignalDatatypeT]:
"""Create an epics signal backend."""
r_protocol, r_pv = split_protocol_from_pv(read_pv)
w_protocol, w_pv = split_protocol_from_pv(write_pv)
protocol = get_unique({read_pv: r_protocol, write_pv: w_protocol}, "protocols")
options = options

Check failure

Code scanning / CodeQL

Redundant assignment Error

This assignment assigns a variable to itself.

Copilot Autofix

AI 3 days ago

To fix the redundant assignment, simply remove the line options = options from within the _epics_signal_backend function in src/ophyd_async/epics/core/_signal.py (line 90). This line serves no functional purpose and its removal will not alter the program's behavior. No additional changes or imports are required.


Suggested changeset 1
src/ophyd_async/epics/core/_signal.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/ophyd_async/epics/core/_signal.py b/src/ophyd_async/epics/core/_signal.py
--- a/src/ophyd_async/epics/core/_signal.py
+++ b/src/ophyd_async/epics/core/_signal.py
@@ -87,7 +87,6 @@
     r_protocol, r_pv = split_protocol_from_pv(read_pv)
     w_protocol, w_pv = split_protocol_from_pv(write_pv)
     protocol = get_unique({read_pv: r_protocol, write_pv: w_protocol}, "protocols")
-    options = options
     signal_backend_type = get_signal_backend_type(protocol)
     return signal_backend_type(
         datatype,
EOF
@@ -87,7 +87,6 @@
r_protocol, r_pv = split_protocol_from_pv(read_pv)
w_protocol, w_pv = split_protocol_from_pv(write_pv)
protocol = get_unique({read_pv: r_protocol, write_pv: w_protocol}, "protocols")
options = options
signal_backend_type = get_signal_backend_type(protocol)
return signal_backend_type(
datatype,
Copilot is powered by AI and may make mistakes. Always verify output.
Copy link
Collaborator

Choose a reason for hiding this comment

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

The AI is right

Check notice

Code scanning / CodeQL

Unused local variable Note

Variable options is not used.

Copilot Autofix

AI 3 days ago

To fix the detected problem, remove the line options = options at line 90. This assignment is redundant because options is already a function parameter and is not modified or used before being passed as-is to signal_backend_type. Removing it avoids confusion about potential usage, removes clutter, and complies with best practices regarding unused local variables. No additional imports, method definitions, or further changes are necessary for this fix. Only remove this assignment in the function _epics_signal_backend around lines 80-97 in src/ophyd_async/epics/core/_signal.py.

Suggested changeset 1
src/ophyd_async/epics/core/_signal.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/ophyd_async/epics/core/_signal.py b/src/ophyd_async/epics/core/_signal.py
--- a/src/ophyd_async/epics/core/_signal.py
+++ b/src/ophyd_async/epics/core/_signal.py
@@ -87,12 +87,12 @@
     r_protocol, r_pv = split_protocol_from_pv(read_pv)
     w_protocol, w_pv = split_protocol_from_pv(write_pv)
     protocol = get_unique({read_pv: r_protocol, write_pv: w_protocol}, "protocols")
-    options = options
     signal_backend_type = get_signal_backend_type(protocol)
     return signal_backend_type(
         datatype,
         r_pv,
         w_pv,
+        options,
     )
 
 
EOF
@@ -87,12 +87,12 @@
r_protocol, r_pv = split_protocol_from_pv(read_pv)
w_protocol, w_pv = split_protocol_from_pv(write_pv)
protocol = get_unique({read_pv: r_protocol, write_pv: w_protocol}, "protocols")
options = options
signal_backend_type = get_signal_backend_type(protocol)
return signal_backend_type(
datatype,
r_pv,
w_pv,
options,
)


Copilot is powered by AI and may make mistakes. Always verify output.
signal_backend_type = get_signal_backend_type(protocol)
return signal_backend_type(datatype, r_pv, w_pv)
return signal_backend_type(
datatype,
r_pv,
w_pv,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
w_pv,
w_pv,
options

)


def epics_signal_rw(
Expand All @@ -95,6 +103,8 @@
name: str = "",
timeout: float = DEFAULT_TIMEOUT,
attempts: int = 1,
wait: bool = True,
no_wait_when_setting: set[SignalDatatypeT] | None = None,
) -> SignalRW[SignalDatatypeT]:
"""Create a `SignalRW` backed by 1 or 2 EPICS PVs.
Expand All @@ -104,7 +114,12 @@
:param name: The name of the signal (defaults to empty string)
:param timeout: A timeout to be used when reading (not connecting) this signal
"""
backend = _epics_signal_backend(datatype, read_pv, write_pv or read_pv)
backend = _epics_signal_backend(
datatype,
read_pv,
write_pv or read_pv,
EpicsOptions(wait, no_wait_when_setting or set()),
)
return SignalRW(backend, name=name, timeout=timeout, attempts=attempts)


Expand Down Expand Up @@ -148,7 +163,7 @@
:param name: The name of the signal (defaults to empty string)
:param timeout: A timeout to be used when reading (not connecting) this signal
"""
backend = _epics_signal_backend(datatype, read_pv, read_pv)
backend = _epics_signal_backend(datatype, read_pv, read_pv, None)
return SignalR(backend, name=name, timeout=timeout)


Expand All @@ -158,6 +173,9 @@
name: str = "",
timeout: float = DEFAULT_TIMEOUT,
attempts: int = 1,
# no_wait_on: set[SignalDatatypeT] | bool = False
wait: bool = True,
no_wait_when_setting: set[SignalDatatypeT] | None = None,
) -> SignalW[SignalDatatypeT]:
"""Create a `SignalW` backed by 1 EPICS PVs.
Expand All @@ -166,7 +184,9 @@
:param name: The name of the signal (defaults to empty string)
:param timeout: A timeout to be used when reading (not connecting) this signal
"""
backend = _epics_signal_backend(datatype, write_pv, write_pv)
backend = _epics_signal_backend(
datatype, write_pv, write_pv, EpicsOptions(wait, no_wait_when_setting or set())
)
return SignalW(backend, name=name, timeout=timeout, attempts=attempts)


Expand All @@ -179,5 +199,5 @@
:param name: The name of the signal
:param timeout: A timeout to be used when reading (not connecting) this signal
"""
backend = _epics_signal_backend(None, write_pv, write_pv)
backend = _epics_signal_backend(None, write_pv, write_pv, None)
return SignalX(backend, name=name, timeout=timeout)
11 changes: 9 additions & 2 deletions src/ophyd_async/epics/core/_util.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from collections.abc import Mapping, Sequence
from typing import Any, TypeVar, get_args, get_origin
from dataclasses import dataclass, field
from typing import Any, Generic, TypeVar, get_args, get_origin

import numpy as np

Expand All @@ -19,6 +20,12 @@
T = TypeVar("T")


@dataclass
class EpicsOptions(Generic[SignalDatatypeT]):
wait: bool = True
no_wait_when_setting: set[SignalDatatypeT] = field(default_factory=set)


def get_pv_basename_and_field(pv: str) -> tuple[str, str | None]:
"""Split PV into record name and field."""
if "." in pv:
Expand Down Expand Up @@ -86,5 +93,5 @@ async def stop_busy_record(
value: SignalDatatypeT,
timeout: float = DEFAULT_TIMEOUT,
) -> None:
await signal.set(value, wait=False)
await signal.set(value)
await wait_for_value(signal, value, timeout=timeout)
2 changes: 1 addition & 1 deletion src/ophyd_async/epics/demo/_motor.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ async def set( # type: ignore
if timeout == CALCULATE_TIMEOUT:
timeout = abs(new_position - old_position) / velocity + DEFAULT_TIMEOUT
# Wait for the value to set, but don't wait for put completion callback
await self.setpoint.set(new_position, wait=False)
await self.setpoint.set(new_position)
# Observe the readback Signal, and on each new position...
async for current_position in observe_value(
self.readback, done_timeout=timeout
Expand Down
4 changes: 2 additions & 2 deletions src/ophyd_async/epics/motor.py
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ async def set( # type: ignore

await self.check_motor_limit(old_position, new_position)

move_status = self.user_setpoint.set(new_position, wait=True, timeout=timeout)
move_status = self.user_setpoint.set(new_position, timeout=timeout)
async for current_position in observe_value(
self.user_readback, done_status=move_status
):
Expand All @@ -267,7 +267,7 @@ async def stop(self, success=False):
self._set_success = success
# Put with completion will never complete as we are waiting for completion on
# the move above, so need to pass wait=False
await self.motor_stop.set(1, wait=False)
await self.motor_stop.set(1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

motor_stop needs constructing with wait=False


async def locate(self) -> Location[float]:
"""Return the current setpoint and readback of the motor."""
Expand Down
2 changes: 1 addition & 1 deletion src/ophyd_async/epics/odin/_odin_io.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ def collect_stream_docs(

async def close(self) -> None:
await stop_busy_record(self._drv.capture, Writing.DONE, timeout=DEFAULT_TIMEOUT)
await self._drv.meta_stop.set(True, wait=True)
await self._drv.meta_stop.set(True)
if self._capture_status and not self._capture_status.done:
await self._capture_status
self._capture_status = None
4 changes: 1 addition & 3 deletions src/ophyd_async/epics/pmac/_pmac_trajectory.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,7 @@ async def stop(self):

@AsyncStatus.wrap
async def _execute_trajectory(self, path: Path, motor_info: _PmacMotorInfo):
execute_status = self.pmac.trajectory.execute_profile.set(
True, wait=True, timeout=None
)
execute_status = self.pmac.trajectory.execute_profile.set(True, timeout=None)
# We consume SLICE_SIZE from self.path and parse a trajectory
# containing at least 2 * SLICE_SIZE, as a gapless trajectory
# will contain 2 points per slice frame. If gaps are present,
Expand Down
4 changes: 1 addition & 3 deletions src/ophyd_async/fastcs/panda/_writer.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,4 @@ async def collect_stream_docs(

# Could put this function as default for StandardDetector
async def close(self):
await self.panda_data_block.capture.set(
False, wait=True, timeout=DEFAULT_TIMEOUT
)
await self.panda_data_block.capture.set(False, timeout=DEFAULT_TIMEOUT)
22 changes: 11 additions & 11 deletions tests/unit_tests/core/test_mock_signal_backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ async def test_set_mock_put_proceeds_timeout():
set_mock_put_proceeds(mock_signal, False)

with pytest.raises(asyncio.TimeoutError):
await mock_signal.set("test", wait=True, timeout=0.1)
await mock_signal.set("test", timeout=0.1)


async def test_put_proceeds_timeout():
Expand Down Expand Up @@ -145,7 +145,7 @@ async def test_mock_utils_throw_error_if_backend_isnt_mock_signal_backend():
async def test_get_mock_put():
mock_signal = epics_signal_rw(str, "READ_PV", "WRITE_PV", name="mock_name")
await mock_signal.connect(mock=True)
await mock_signal.set("test_value", wait=True)
await mock_signal.set("test_value")

mock = get_mock_put(mock_signal)
mock.assert_called_once_with("test_value", wait=True)
Expand Down Expand Up @@ -173,8 +173,8 @@ async def mock_signals():
signal1 = epics_signal_rw(str, "READ_PV1", "WRITE_PV1", name="mock_name1")
signal2 = epics_signal_rw(str, "READ_PV2", "WRITE_PV2", name="mock_name2")

await signal1.set("first_value", wait=True, timeout=1)
await signal2.set("first_value", wait=True, timeout=1)
await signal1.set("first_value", timeout=1)
await signal2.set("first_value", timeout=1)
assert await signal1.get_value() == "first_value"
assert await signal2.get_value() == "first_value"
return signal1, signal2
Expand All @@ -184,8 +184,8 @@ async def test_blocks_during_put(mock_signals):
signal1, signal2 = mock_signals

with mock_puts_blocked(signal1, signal2):
status1 = signal1.set("second_value", wait=True, timeout=None)
status2 = signal2.set("second_value", wait=True, timeout=None)
status1 = signal1.set("second_value", timeout=None)
status2 = signal2.set("second_value", timeout=None)
await asyncio.sleep(0.1)
assert await signal1.get_value() == "second_value"
assert await signal2.get_value() == "second_value"
Expand All @@ -205,9 +205,9 @@ async def test_callback_on_mock_put_as_context_manager(mock_signals):
signal2_callbacks = MagicMock()
signal1, signal2 = mock_signals
with callback_on_mock_put(signal1, signal1_callbacks):
await signal1.set("second_value", wait=True)
await signal1.set("second_value")
with callback_on_mock_put(signal2, signal2_callbacks):
await signal2.set("second_value", wait=True)
await signal2.set("second_value")

signal1_callbacks.assert_called_once_with("second_value", wait=True)
signal2_callbacks.assert_called_once_with("second_value", wait=True)
Expand All @@ -234,9 +234,9 @@ async def test_async_callback_on_mock_put(mock_signals):
signal2_callbacks = AsyncMock()
signal1, signal2 = mock_signals
with callback_on_mock_put(signal1, signal1_callbacks):
await signal1.set("second_value", wait=True)
await signal1.set("second_value")
with callback_on_mock_put(signal2, signal2_callbacks):
await signal2.set("second_value", wait=True)
await signal2.set("second_value")

signal1_callbacks.assert_awaited_once_with("second_value", wait=True)
signal2_callbacks.assert_awaited_once_with("second_value", wait=True)
Expand Down Expand Up @@ -311,7 +311,7 @@ async def test_set_mock_values_exhausted_fails(mock_signals):

async def test_reset_mock_put_calls(mock_signals):
signal1, _ = mock_signals
await signal1.set("test_value", wait=True, timeout=1)
await signal1.set("test_value", timeout=1)
get_mock_put(signal1).assert_called_with("test_value", wait=ANY)
get_mock_put(signal1).reset_mock()
with pytest.raises(AssertionError) as exc:
Expand Down
4 changes: 2 additions & 2 deletions tests/unit_tests/core/test_multi_derived_signal.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,9 @@ async def test_derived_signal_backend_put_wait_fails(
derived_signal: SignalRW,
) -> None:
with pytest.raises(RuntimeError):
await derived_signal.set(value=None, wait=False)
await derived_signal.set(value=None)
with pytest.raises(RuntimeError):
await derived_signal.set(value=None, wait=True)
await derived_signal.set(value=None)


def test_make_rw_signal_type_mismatch():
Expand Down
4 changes: 2 additions & 2 deletions tests/unit_tests/core/test_observe.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,12 +155,12 @@ async def test_observe_signals_value_timeout_message():

async def tick1():
for i in range(n_updates):
sig1.set(i + 10.0, False)
sig1.set(i + 10.0)
await asyncio.sleep(time_delay_sec1)

async def tick2():
for i in range(n_updates):
sig2.set(i + 100.0, False)
sig2.set(i + 100.0)
await asyncio.sleep(time_delay_sec2)

async def watch(timeout, done_timeout):
Expand Down
2 changes: 1 addition & 1 deletion tests/unit_tests/epics/test_motor.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ async def test_motor_moving_stopped(sim_motor: motor.Motor):

# Note: needs to explicitly be called with 1, not just processed.
# See https://epics.anl.gov/bcda/synApps/motor/motorRecord.html#Fields_command
get_mock_put(sim_motor.motor_stop).assert_called_once_with(1, wait=False)
get_mock_put(sim_motor.motor_stop).assert_called_once_with(1, wait=True)

set_mock_put_proceeds(sim_motor.user_setpoint, True)
await wait_for_pending_wakeups()
Expand Down
Loading