Derived signal timeout bug

[jacob720]

I wrote a device for the cobra on i15-1 which uses a derived signal to move it between a safe and beam position, which takes ~30s. I found when I do bps.abs_set(derived_signal, value) which sets the underlying motor, I get a timeout error (see https://graylog.diamond.ac.uk/messages/graylog-prod_19/99066f52-2934-11f1-a66b-22d3eb5a3de6). This seemed to happen after 10s of the move.

This happened even if I gave long timeouts to the motor set call inside the set_derived function. If I just set the motor directly, I don't get this error so it seems like something to do with the derived signal. This is how I wrote the device:

class Cobra(StandardReadable):
    def __init__(self, prefix: str):
        self.motor = Motor(prefix=prefix)
        self.position = derived_signal_rw(
            self._get_position,
            self._set_position,
            current_position=self.motor,
        )
        self._safe_position = 2.0
        self._beam_position = 100.0
        super().__init__(prefix)

    def _get_position(self, current_position: float) -> CobraPosition:
        if current_position == self._safe_position:
            return CobraPosition.SAFE
        elif current_position == self._beam_position:
            return CobraPosition.BEAM
        raise ValueError(
            f"Device's position {current_position} is not {CobraPosition.SAFE}: "
            f"{self._safe_position} or {CobraPosition.BEAM}: {self._beam_position}"
        )

    async def _set_position(self, position: CobraPosition):
        if position == CobraPosition.SAFE:
            await self.motor.set(self._safe_position, 60)
        elif position == CobraPosition.BEAM:
            await self.motor.set(self._beam_position, 60)

yield from bps.abs_set(cobra.position, CobraPosition.SAFE, group=group)  # TimeoutError
yield from bps.abs_set(cobra.motor, 2.0, group=group)  # works

Steps To Reproduce

Steps to reproduce the behaviour:

1. Create a derived signal who's set method takes longer than 10s
2. Call bps.abs_set on the derived signal.

Acceptance Criteria

• You can have a derived signal who's set method takes longer than 10s

[oliwenmandiamond]

The issue here is that is that when trying to move your derived signal, it still has a default timeout of 10 seconds. You would need to provide the timeout argument to your plan

bps.abs_set(cobra.position, my_value, timeout=60)

I believe the above should work.

For epics_signal_rw, you can specify the default timeout to use when creating the signal

(function) def epics_signal_rw(
    datatype: type[SignalDatatypeT@epics_signal_rw],
    read_pv: str,
    write_pv: str | None = None,
    name: str = "",
    timeout: float = DEFAULT_TIMEOUT,
    attempts: int = 1
) -> SignalRW[SignalDatatypeT@epics_signal_rw]

So the issue is that derived signals should also be able to specify the default timeout and currently you can't

(function) def derived_signal_rw(
    raw_to_derived: (...) -> SignalDatatypeT@derived_signal_rw,
    set_derived: (SignalDatatypeT@derived_signal_rw) -> Awaitable[None],
    derived_units: str | None = None,
    derived_precision: int | None = None,
    **raw_devices_and_constants: Primitive | Device
) -> SignalRW[SignalDatatypeT@derived_signal_rw]

[Relm-Arrowny]

i am 90% sure derived_signal is not to blame here. I have the timeout set to the interval between electron fills, and it never times out on the 10s default.

[coretl]

I think the issue here is that the timeout from the SignalW.set call is never passed down to the _set_position function, because it's never passed to SignalBackend.put, it's applied in the SignalW.set method directly:

ophyd-async/src/ophyd_async/core/_signal.py

Line 301 in 20adc85

await _wait_for(self._connector.backend.put(value), timeout, source)

Options are:

1. Leave it like it is and document as a limitation of derived signals
2. Change SignalBackend.put to always take timeout as an argument, then handle the timeout in every backend
3. Make the SignalW that derived_signal_rw creates special, so it always passes the timeout down to the DerivedSignalBackend, which passes it to the derived setter function
4. As 3, but make it opt-in via a separate derived_signal_rw_with_timeout or similar

Everything except 1 is quite a lot of work, how important is this feature?