fix(ble): reconnect a dropped peripheral at once, forever, and resume a Maimu train

CHANGELOG.md

@@ -373,6 +373,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **A dropped Bluetooth peripheral comes back in seconds, and never stops
+  trying.** The first retry is immediate, then half a second, a second, and
+  every two seconds after that, for as long as the device is in use; before,
+  retries started after 5 seconds, backed off to a minute and gave up after
+  twelve. A peripheral with an advertised name is looked for by name first,
+  because a peripheral using BLE privacy re-advertises at a new address after
+  every drop, and scanning for the old one cost each reconnect several seconds
+  for nothing.
+- **A Maimu resumes its pulse train after a dropout.** It used to come back
+  `off`. It now resends its last command, a train for whatever is left of its
+  duration. Commands sent while the link is down no longer block the flow and
+  then replay in a burst; the newest one is sent on reconnect. The node's
+  duty-cycle setting now goes through a `train(on_ms, off_ms, duration_s)`
+  action, which the device control panel offers as well.
+
 - **Three examples would not start** (`box_new`, `box_pi_side`, `dispense`):
   they held connections to nodes that no longer existed. Those connections are
   removed; they never loaded, so the experiments behave as before.

docs-site/reference/devices.md

@@ -55,21 +55,24 @@ A BLE peripheral that goes out of range, loses power, or is claimed by another
 central is noticed straight away, not at the next write. Its row in the
 [Hardware panel](../building/devices.md) and its own dot on the status strip
 (shown beside the board dots for any peripheral that tracks its own link) both
-move to **Disconnected**, and GLIDER starts retrying on its own: 5 seconds,
-then 10, 20, 40 and 60, up to twelve attempts, before it gives up and shows
-**Error**.
+move to **Disconnected**, and GLIDER starts retrying on its own: straight
+away, then after half a second, a second, and every two seconds from then on,
+for as long as the device is in use. It does not give up. A peripheral with an
+advertised name is looked for by that name first, because a peripheral using
+BLE privacy comes back at a new address after every drop.
 
 - The Bluetooth **board**'s own dot stays green throughout. That is the host
   adapter, and it is genuinely fine — it is the peripheral that went away,
   which is why a peripheral that owns its link gets a dot of its own.
 - A run is not paused by a dropped peripheral. You get a warning notification
-  and the retries continue underneath it; only **Error** — retries exhausted —
-  needs you to go and look.
-- **A Maimu comes back off.** The stimulator runs a pulse train in its own
-  firmware, so a link that died mid-train left it stimulating with nothing
-  attached to stop it. When the reconnect succeeds, GLIDER writes `off` before
-  anything else, so the device returns in a known state instead of resuming a
-  pattern nobody is watching. Re-issue the pulse yourself if you still want it.
+  and the retries continue underneath it.
+- **A Maimu comes back doing what it was last told.** Its firmware turns the
+  light off the moment the link drops. When the reconnect succeeds, GLIDER
+  resends the last command: a pulse train for whatever is left of its
+  duration, `on` as `on`, and `off` if nothing was asked or the train has run
+  out. Commands the flow sends while the link is down do not hold the flow up;
+  the newest one is what goes out on reconnect. The log says when a train was
+  resumed, so the gap in stimulation is on record.
 
 !!! tip "Two stimulators with the same name"
     A strip dot is labelled with the device's name, since that is usually more

plugins/glider-maimu/README.md

@@ -92,12 +92,14 @@ therefore writes `off` before it disconnects, bounded so it cannot eat GLIDER's
 emergency-stop budget. Emergency stop, End Experiment and app quit all stop the
 device through that one path.
 
-The same reasoning covers a link that drops on its own — out of range, out of
-power, claimed by another central. GLIDER reconnects it with bounded backoff
-(5, 10, 20, 40 and 60 seconds, up to twelve attempts) and, the moment it is
-back, writes `off` before anything else touches the device. A stimulator that
-reconnected mid-pattern comes back in a known state instead of resuming one
-nobody is watching — re-issue the pulse yourself if you still want it.
+A link that drops on its own — out of range, out of power, claimed by another
+central — is different. The firmware turns the light off the moment the link
+goes, and GLIDER starts reconnecting straight away, then every few seconds at
+most, without giving up. The device remembers the last command it was given and
+resends it the moment the link is back: a train for whatever is left of its
+duration, `on` as `on`, and `off` if nothing was asked or the train has run
+out. Commands that arrive while the link is down return immediately instead of
+holding up the flow, and only the newest is sent on reconnect.
 
 ## Development
 

plugins/glider-maimu/src/glider_maimu/device.py

@@ -3,15 +3,17 @@
 The Maimu is a Nordic/Zephyr Bluetooth LE peripheral with exactly one writable
 GATT characteristic that accepts three UTF-8 commands::
 
-    on                      turn on and stay on
-    off                     turn off
-    <period_ms>,<duration_s>  toggle every period_ms ms, for duration_s seconds
+    on                              turn on and stay on
+    off                             turn off
+    <period_ms>,<duration_s>        toggle every period_ms ms, for duration_s seconds
+    p<on_ms>,<off_ms>,<duration_s>  on_ms on / off_ms off, for duration_s seconds
+                                    (firmware 0.1.0+; older firmware ignores it)
 
 Running one through the generic ``BLEWrite`` / ``BLE`` device works, but means
 pasting both UUIDs into the Add Device dialog and then remembering that the
 action is called ``write`` and that a pulse is spelled ``"500,10"``. This device
 bakes the protocol in: the UUIDs are defaults and the commands are named actions
-(``on`` / ``off`` / ``pulse``).
+(``on`` / ``off`` / ``pulse`` / ``train``).
 
 It subclasses :class:`~glider.hal.devices.ble_device.BLEDevice` rather than
 ``BaseDevice`` so it inherits that class's connection plumbing unchanged --
@@ -26,17 +28,22 @@
 ``off`` before it disconnects -- which puts emergency stop, End Experiment and
 app quit all on the same path.
 
-The same reasoning covers a *dropped* link: :meth:`MaimuDevice._on_reconnected`
-writes ``off`` when the automatic reconnect succeeds, so a stimulator that came
-back mid-pattern is put in a known state instead of left running one nobody
-asked for.
+A *dropped* link is different: the firmware turns the light off the moment the
+link goes, but the experiment still wants whatever it last asked for. So the
+device remembers its last command, and :meth:`MaimuDevice._on_reconnected`
+puts it back the moment the link returns -- a train for whatever is left of its
+duration, ``off`` if nothing was asked or the train has run out. Commands that
+arrive while the link is down update that memory and return at once instead of
+holding the flow up behind a reconnect; the newest one is what gets sent.
 """
 
 import asyncio
 import logging
+import time
 from collections.abc import Callable
 from typing import TYPE_CHECKING, Any
 
+from glider.hal.base_board import ConnectionState
 from glider.hal.base_device import DeviceConfig
 from glider.hal.devices.ble_device import BLEDevice
 
@@ -132,6 +139,35 @@ class MaimuDevice(BLEDevice):
                 "help": "How long the train runs. The firmware stops on its own.",
             },
         ],
+        "train": [
+            {
+                "key": "on_ms",
+                "label": "On (ms)",
+                "type": "int",
+                "default": 500,
+                "min": 1,
+                "max": 3_600_000,
+                "help": "How long each pulse stays on. 20 Hz with 10 ms pulses: On 10, Off 40.",
+            },
+            {
+                "key": "off_ms",
+                "label": "Off (ms)",
+                "type": "int",
+                "default": 500,
+                "min": 1,
+                "max": 3_600_000,
+                "help": "Gap between pulses. Unequal On/Off needs firmware 0.1.0+.",
+            },
+            {
+                "key": "duration_s",
+                "label": "Duration (s)",
+                "type": "int",
+                "default": 10,
+                "min": 1,
+                "max": 86_400,
+                "help": "How long the train runs. The firmware stops on its own.",
+            },
+        ],
     }
 
     def __init__(self, board: "BaseBoard", config: DeviceConfig, name: str | None = None):
@@ -141,6 +177,13 @@ def __init__(self, board: "BaseBoard", config: DeviceConfig, name: str | None =
         config.settings.setdefault("write_char_uuid", DEFAULT_WRITE_CHAR_UUID)
         config.settings.setdefault("service_uuid", DEFAULT_SERVICE_UUID)
         super().__init__(board, config, name)
+        # The last command the experiment gave, so a reconnect can put it back:
+        # ("on",), ("off",) or ("train", on_ms, off_ms, duration_s, ends_at)
+        # with ends_at on the perf_counter clock. None until the first command.
+        self._desired: tuple | None = None
+        # Serializes "remember a command, send it" against the reconnect hook,
+        # so the hook can never send an older command over a newer one.
+        self._desired_lock = asyncio.Lock()
 
     # --- identity ---
 
@@ -161,18 +204,19 @@ def actions(self) -> dict[str, Callable]:
             "on": self.on,
             "off": self.off,
             "pulse": self.pulse,
+            "train": self.train,
             "write": self.write,
         }
 
     # --- actions ---
 
     async def on(self) -> None:
         """Turn the stimulator on and leave it on."""
-        await self.write("on")
+        await self._command(("on",))
 
     async def off(self) -> None:
         """Turn the stimulator off."""
-        await self.write("off")
+        await self._command(("off",))
 
     async def pulse(self, period_ms: Any, duration_s: Any) -> None:
         """Toggle every ``period_ms`` milliseconds for ``duration_s`` seconds.
@@ -185,12 +229,93 @@ async def pulse(self, period_ms: Any, duration_s: Any) -> None:
         """
         period = self._whole_number(period_ms, "period_ms")
         duration = self._whole_number(duration_s, "duration_s")
-        # Two args -> BLEDevice.write comma-joins them: "500,10".
-        await self.write(period, duration)
+        await self._command(("train", period, period, duration, self._deadline(duration)))
+
+    async def train(self, on_ms: Any, off_ms: Any, duration_s: Any) -> None:
+        """Pulse ``on_ms`` on / ``off_ms`` off for ``duration_s`` seconds.
+
+        e.g. ``train(10, 40, 10)`` writes ``"p10,40,10"`` -- 10 ms pulses at
+        20 Hz for 10 seconds. Equal on and off times go out as the original
+        ``<period_ms>,<duration_s>`` command, which every firmware understands.
+        """
+        on = self._whole_number(on_ms, "on_ms")
+        off = self._whole_number(off_ms, "off_ms")
+        duration = self._whole_number(duration_s, "duration_s")
+        await self._command(("train", on, off, duration, self._deadline(duration)))
+
+    # --- remembered command ---
+
+    @staticmethod
+    def _deadline(duration_s: int) -> float:
+        return time.perf_counter() + duration_s
+
+    @staticmethod
+    def _wire(desired: tuple, *, resume: bool = False) -> tuple:
+        """The write() arguments for a remembered command.
+
+        A first send uses the train's full duration. A resend after a reconnect
+        uses what is left of it, rounded down so the train never outlasts the
+        one that was asked for, and turns into ``off`` once under a second is
+        left.
+        """
+        if desired[0] != "train":
+            return desired
+        _, on, off, duration, ends_at = desired
+        if resume:
+            left = ends_at - time.perf_counter()
+            if left < 1:
+                return ("off",)
+            duration = int(left)
+        # Two or three args -> BLEDevice.write comma-joins them: "500,10".
+        return (on, duration) if on == off else (f"p{on}", off, duration)
+
+    def _link_is_down(self) -> bool:
+        return self._initialized and self.link_state in (
+            ConnectionState.DISCONNECTED,
+            ConnectionState.RECONNECTING,
+        )
+
+    async def _command(self, desired: tuple) -> None:
+        """Remember ``desired``, then send it -- unless the link is down.
+
+        With the link down a write would block the flow behind a reconnect of
+        several seconds, and every command queued up that way used to land in a
+        burst once it came back, oldest first. Remembering the newest and
+        returning means the reconnect sends exactly one command: the current one.
+        """
+        async with self._desired_lock:
+            self._desired = desired
+            wire = self._wire(desired)
+            if self._link_is_down():
+                logger.warning(
+                    "Maimu %s: link is down; %s will be sent as soon as it reconnects",
+                    self._name,
+                    ",".join(str(a) for a in wire),
+                )
+                # Normally already running; this covers a drop no callback reported.
+                self._start_reconnect()
+                return
+            # Once, with no reconnect inside the write: the command is
+            # remembered, so if the link died under it the reconnect sends it --
+            # and an in-band reconnect would hold this lock, and the flow behind
+            # it, for seconds.
+            try:
+                await self.write(*wire, retry=False)
+            except Exception:
+                client = self._client
+                if not self._initialized or (client is not None and client.is_connected):
+                    raise  # the link is up (or we are shutting down): a real failure
+                logger.warning(
+                    "Maimu %s: link dropped while sending %s; it will be sent as soon "
+                    "as it reconnects",
+                    self._name,
+                    ",".join(str(a) for a in wire),
+                )
+                self._start_reconnect()
 
     @staticmethod
     def _whole_number(value: Any, field: str) -> int:
-        """Coerce a pulse argument to a positive whole number.
+        """Coerce a pulse or train argument to a positive whole number.
 
         The firmware ``atoi``s both fields, so a fractional or non-numeric value
         would be silently truncated into a command that does something other
@@ -200,35 +325,47 @@ def _whole_number(value: Any, field: str) -> int:
         try:
             number = float(value)
         except (TypeError, ValueError) as e:
-            raise ValueError(f"Maimu.pulse: {field} must be a number, got {value!r}") from e
+            raise ValueError(f"Maimu: {field} must be a number, got {value!r}") from e
         if not number.is_integer():
-            raise ValueError(f"Maimu.pulse: {field} must be a whole number, got {value!r}")
+            raise ValueError(f"Maimu: {field} must be a whole number, got {value!r}")
         number = int(number)
         if number < 1:
-            raise ValueError(f"Maimu.pulse: {field} must be at least 1, got {value!r}")
+            raise ValueError(f"Maimu: {field} must be at least 1, got {value!r}")
         return number
 
     # --- lifecycle ---
 
     async def _on_reconnected(self) -> None:
-        """Come back off.
-
-        The firmware runs a pulse autonomously, so a link that dropped
-        mid-train left the stimulator running with nothing attached to stop
-        it. Whatever it is doing, it has been doing it unsupervised, and this
-        is the first moment anyone can say otherwise -- so the device is put
-        in a known state rather than resumed in an unknown one.
-
-        Same reasoning as :meth:`shutdown`, and the same best-effort
-        treatment: BLEDevice logs a failure here and leaves the link up,
-        because the link genuinely did reconnect.
-
-        This and :meth:`shutdown` are independent safety paths that can both
-        fire for the same event -- a reconnect landing just as a shutdown
-        begins -- so ``off`` may be written twice in a row. That duplication
-        is safety-neutral and deliberate; it is not a bug to collapse.
+        """Put back the last command the experiment gave.
+
+        The firmware turns the light off when the link drops, so whatever the
+        experiment asked for has been interrupted, not continued. A train that
+        still has time left is resent for that remaining time; ``on`` is resent
+        as ``on``; anything else -- ``off``, a train that ran out, or no command
+        yet -- writes ``off``, so the device is always left in a known state.
+
+        Held under the command lock so a command the flow sends at the same
+        moment is never overwritten by an older one from here. Sent once, with
+        no reconnect inside the write: if the link dies again before it lands,
+        the reconnect loop carries on and calls this again, which then works out
+        what is left of the train at that later moment rather than sending a
+        figure that went stale while it waited.
+
+        Best-effort, like :meth:`shutdown`: BLEDevice logs a failure here and
+        leaves the link up, because the link genuinely did reconnect. This and
+        :meth:`shutdown` can both fire for the same event, so ``off`` may be
+        written twice in a row; that is safety-neutral and deliberate.
         """
-        await self.write("off")
+        async with self._desired_lock:
+            desired = self._desired
+            wire = self._wire(desired, resume=True) if desired is not None else ("off",)
+            if wire != ("off",):
+                logger.warning(
+                    "Maimu %s: link restored; resuming %s",
+                    self._name,
+                    ",".join(str(a) for a in wire),
+                )
+            await self.write(*wire, retry=False)
 
     async def shutdown(self) -> None:
         """Stop the stimulator, then disconnect.
@@ -253,6 +390,9 @@ async def shutdown(self) -> None:
         state. That is the same news as a failed write, so it gets the same
         warning.
         """
+        # Forget the last command first, so a reconnect landing during the
+        # shutdown can only ever put the device back to off.
+        self._desired = None
         try:
             if self._initialized and self._client is not None:
                 try:

plugins/glider-maimu/src/glider_maimu/node.py

@@ -167,12 +167,10 @@ async def hardware_operation(self) -> None:
         """Send the configured command, then trigger the exec output."""
         off_ms = self._off_ms or self._period_ms
         if self._mode == MODE_PULSE and off_ms != self._period_ms:
-            # "p<on_ms>,<off_ms>,<duration_s>" (firmware 0.1.0+). The 'p' makes
-            # older firmware ignore it rather than misread "10,40,10" as a
-            # 10 ms toggle for 40 s.
-            await self._device.execute_action(
-                "write", f"p{self._period_ms}", off_ms, self._duration_s
-            )
+            # Sent as "p<on_ms>,<off_ms>,<duration_s>" (firmware 0.1.0+). The
+            # 'p' makes older firmware ignore it rather than misread "10,40,10"
+            # as a 10 ms toggle for 40 s.
+            await self._device.execute_action("train", self._period_ms, off_ms, self._duration_s)
         elif self._mode == MODE_PULSE:
             await self._device.execute_action(MODE_PULSE, self._period_ms, self._duration_s)
         else: