added code review changes

src/ophyd_async/core/device.py

@@ -33,16 +33,10 @@ class Device(HasName):
     _name: str = ""
     #: The parent Device if it exists
     parent: Optional[Device] = None
-    # None if connect hasn't started, an Event if it has, a set Event if it's done
-    _connect_task: Optional[asyncio.Task] = None
 
-    @property
-    def connected(self):
-        return (
-            self._connect_task is not None
-            and self._connect_task.done()
-            and not self._connect_task.exception()
-        )
+    # Previous connect was successful, False on initialization
+    # since there was no previous connect
+    _previous_connect_success: bool = False
 
     def __init__(self, name: str = "") -> None:
         self.set_name(name)
@@ -99,16 +93,18 @@ async def connect(
         timeout:
             Time to wait before failing with a TimeoutError.
         """
-        if force_reconnect or not self.connected:
+
+        if force_reconnect or not self._previous_connect_success:
             # Kick off a connection
             coros = {
                 name: child_device.connect(sim, timeout=timeout)
                 for name, child_device in self.children()
             }
-            self._connect_task = asyncio.create_task(wait_for_connection(**coros))
+            connect_task = asyncio.create_task(wait_for_connection(**coros))
 
             # Wait for it to complete
-            await self._connect_task
+            await connect_task
+            self._previous_connect_success = not connect_task.exception()
 
 
 VT = TypeVar("VT", bound=Device)

tests/core/test_device.py

@@ -18,16 +18,11 @@
 
 
 class DummyBaseDevice(Device):
-
     def __init__(self):
-        self._connected = False
-
-    @property
-    def connected(self) -> bool:
-        return self._connected
+        self.connected = False
 
     async def connect(self, sim=False, timeout=DEFAULT_TIMEOUT):
-        self._connected = True
+        self.connected = True
 
 
 class DummyDeviceGroup(Device):
@@ -99,7 +94,7 @@ def __init__(self, name) -> None:
 
         async def connect(self, sim=False, timeout=DEFAULT_TIMEOUT):
             await asyncio.sleep(0.01)
-            self._connected = True
+            self.connected = True
 
     device1, device2 = DummyDeviceWithSleep("device1"), DummyDeviceWithSleep("device2")
 
@@ -132,12 +127,12 @@ async def test_device_lazily_connects(RE):
     async with DeviceCollector(sim=True, connect=False):
         sim_motor = motor.Motor("BLxxI-MO-TABLE-01:X")
 
-    assert not sim_motor.connected
+    assert not sim_motor._previous_connect_success
 
     # When ready to connect
     RE(ensure_connected(sim_motor, sim=True))
 
-    assert sim_motor.connected
+    assert sim_motor._previous_connect_success
 
 
 class MotorBundle(Device):
@@ -159,21 +154,19 @@ async def test_device_with_children_lazily_connects(RE):
     for device in [parentMotor, parentMotor.X, parentMotor.Y] + list(
         parentMotor.V.values()
     ):
-        assert not device.connected
+        assert not device._previous_connect_success
     RE(ensure_connected(parentMotor, sim=True))
 
     for device in [parentMotor, parentMotor.X, parentMotor.Y] + list(
         parentMotor.V.values()
     ):
-        assert device.connected
+        assert device._previous_connect_success
 
 
 async def test_device_with_device_collector_lazily_connects():
-    async with DeviceCollector(sim=True, connect=False):
-        sim_motor = motor.Motor("BLxxI-MO-TABLE-01:X")
-    assert not sim_motor.connected
+    sim_motor = motor.Motor("SOME_SIGNAL_WHICH_DOESN'T_EXIST:X")
     with pytest.raises(NotConnected):
-        await sim_motor.connect(timeout=0.01)
-
+        await sim_motor.connect(sim=False, timeout=0.01)
+    assert not sim_motor._previous_connect_success
     await sim_motor.connect(sim=True, timeout=0.01)
-    assert sim_motor.connected
+    assert sim_motor._previous_connect_success