From e0f4fd83682cfb03b1964d3fe6f1e8b48c54501f Mon Sep 17 00:00:00 2001
From: Giles Knap <giles.knap@diamond.ac.uk>
Date: Wed, 17 Apr 2024 13:44:06 +0000
Subject: [PATCH] remove move, make _move synchronous

---
 src/ophyd_async/sim/demo/sim_motor.py | 30 +++++++++++++--------------
 tests/sim/demo/test_sim_motor.py      | 24 +++++++++++----------
 2 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/src/ophyd_async/sim/demo/sim_motor.py b/src/ophyd_async/sim/demo/sim_motor.py
index c61665247b..ccf0bd48c0 100644
--- a/src/ophyd_async/sim/demo/sim_motor.py
+++ b/src/ophyd_async/sim/demo/sim_motor.py
@@ -32,8 +32,9 @@ def __init__(self, prefix: str, name="", instant=True) -> None:
         self.egu = soft_signal_rw(float, "egu", prefix + ".egu")
 
         # sensible defaults
-        self.velocity.set(1)
-        self.egu.set("mm")
+        # TODO this cannot be set at present - James Souter is adding an
+        # initial value to the soft signal constructors
+        # await self.velocity.set(1)
 
         # Set name and signals for read() and read_configuration()
         self.set_readable_signals(
@@ -53,16 +54,6 @@ def stop(self):
             self._move_task.cancel()
             self._move_task = None
 
-    def move(self, new_position: float, timeout: Optional[float] = None):
-        """
-        Commandline only synchronous move of a Motor
-        """
-        from bluesky.run_engine import call_in_bluesky_event_loop, in_bluesky_event_loop
-
-        if in_bluesky_event_loop():
-            raise RuntimeError("Will deadlock run engine if run in a plan")
-        call_in_bluesky_event_loop(self._move(new_position), timeout)  # type: ignore
-
     def set(self, new_position: float, timeout: Optional[float] = None) -> AsyncStatus:  # noqa: F821
         """
         Asynchronously move the motor to a new position.
@@ -75,12 +66,10 @@ async def _move(self, new_position: float, watchers: List[Callable] = []):
         """
         Start the motor moving to a new position.
 
-        If the motor is already moving, it will stop and start moving to the
-        new position.
+        If the motor is already moving, it will stop first.
         If this is an instant motor the move will be instantaneous.
         """
         self.stop()
-        self._set_success = True
         start = time.monotonic()
 
         current_position = await self.user_readback.get_value()
@@ -96,7 +85,9 @@ async def update_position():
             while True:
                 time_elapsed = round(time.monotonic() - start, 2)
                 if time_elapsed >= travel_time:
-                    current_position = new_position
+                    # successfully reached our target position
+                    await self._user_readback.put(new_position)
+                    self._set_success = True
                     break
                 else:
                     current_position = (
@@ -122,3 +113,10 @@ def update_watchers(current_position: float):
 
         # set up watchers to be called when the motor position changes
         self.user_readback.subscribe_value(update_watchers)
+
+        try:
+            await self._move_task
+        finally:
+            self.user_readback.clear_sub(update_watchers)
+        if not self._set_success:
+            raise RuntimeError("Motor was stopped")
diff --git a/tests/sim/demo/test_sim_motor.py b/tests/sim/demo/test_sim_motor.py
index ae351285c8..379cc6a76b 100644
--- a/tests/sim/demo/test_sim_motor.py
+++ b/tests/sim/demo/test_sim_motor.py
@@ -3,16 +3,19 @@
 
 from bluesky.plans import spiral_square
 from bluesky.run_engine import RunEngine
+from ophyd_async.core.device import DeviceCollector
 from ophyd_async.sim.demo.sim_motor import SimMotor
 
 
 async def test_move_sim_in_plan():
     RE = RunEngine()
 
-    m1 = SimMotor("M1", "sim_motor1")
-    m2 = SimMotor("M2", "sim_motor2")
-    await m1.connect()
-    await m2.connect()
+    async with DeviceCollector():
+        m1 = SimMotor("M1", "sim_motor1")
+        m2 = SimMotor("M2", "sim_motor2")
+
+    await m1.velocity.set(2)
+    await m2.velocity.set(2)
 
     my_plan = spiral_square([], m1, m2, 0, 0, 4, 4, 10, 10)
 
@@ -23,25 +26,24 @@ async def test_move_sim_in_plan():
 
 
 async def test_slow_move():
-    _ = RunEngine()
+    async with DeviceCollector():
+        m1 = SimMotor("M1", "sim_motor1", instant=False)
 
-    m1 = SimMotor("M1", "sim_motor1", instant=False)
-    await m1.connect()
     await m1.velocity.set(20)
 
     start = time.monotonic()
-    m1.move(10)
+    await m1.set(10)
     elapsed = time.monotonic() - start
 
+    assert await m1.user_readback.get_value() == 10
     assert elapsed >= 0.5
     assert elapsed < 1
-    assert await m1.user_readback.get_value() == 10
 
 
 async def test_stop():
-    _ = RunEngine()
+    async with DeviceCollector():
+        m1 = SimMotor("M1", "sim_motor1", instant=False)
 
-    m1 = SimMotor("M1", "sim_motor1", instant=False)
     await m1.connect()
     await m1.velocity.set(2)