fix(controller): persist LED intensity and white balance calibration

controller/MCP4725.py

@@ -14,6 +14,11 @@
 i2c = None
 dac = None
 
+# Level `on()` restores, in raw DAC counts. `off()` drives the output to zero but leaves
+# this alone, so switching the LED back on returns it to the brightness it was last set to
+# rather than to full scale.
+_level = DAC_MAX
+
 
 def map_to_voltage(value):
     return (value / DAC_MAX) * VOLTAGE_MAX
@@ -25,7 +30,9 @@ def map_to_value(voltage):
 
 def on() -> None:
     assert dac is not None
-    dac.raw_value = DAC_MAX
+    # Deliberately not DAC_MAX: driving the LED to full scale here made every switch-on
+    # flash at maximum brightness before the caller set the requested level.
+    dac.raw_value = _level
 
 
 def off() -> None:
@@ -65,8 +72,11 @@ def get_value() -> float:
 
 
 def set_value(value: float) -> None:
+    global _level
     assert dac is not None
     dac.normalized_value = value
+    if dac.raw_value > DAC_MIN:
+        _level = dac.raw_value
 
 
 def get_raw_value() -> int:
@@ -75,5 +85,8 @@ def get_raw_value() -> int:
 
 
 def set_raw_value(value: int) -> None:
+    global _level
     assert dac is not None
     dac.raw_value = value
+    if value > DAC_MIN:
+        _level = value

controller/helpers.py

@@ -1,6 +1,8 @@
 import asyncio
 import json
+import os
 import socket
+import tempfile
 from typing import Any, cast
 
 import aiofiles
@@ -17,9 +19,33 @@ async def read_hardware_config() -> dict[str, Any]:
         return cast(dict[str, Any], json.loads(content))
 
 
+def _write_hardware_config_atomic(data: dict[str, Any]) -> None:
+    """Replace hardware.json in one step.
+
+    Several controller processes persist calibration here (pump, light, imager), and a
+    partially written hardware.json leaves the instrument unbootable, so the new contents
+    go to a temporary file in the same directory and are then renamed over the old one.
+    `os.replace` is atomic within a filesystem, so a reader sees either the old file or the
+    new one and never a truncated one.
+    """
+    directory = os.path.dirname(HARDWARE_CONFIG_PATH)
+    fd, tmp_path = tempfile.mkstemp(dir=directory, prefix=".hardware.json.")
+    try:
+        with os.fdopen(fd, "w") as f:
+            f.write(json.dumps(data, indent=2))
+            f.flush()
+            os.fsync(f.fileno())
+        os.replace(tmp_path, HARDWARE_CONFIG_PATH)
+    except BaseException:
+        try:
+            os.unlink(tmp_path)
+        except OSError:
+            pass
+        raise
+
+
 async def write_hardware_config(data: dict[str, Any]) -> None:
-    async with aiofiles.open(HARDWARE_CONFIG_PATH, "w") as f:
-        await f.write(json.dumps(data, indent=2))
+    await asyncio.to_thread(_write_hardware_config_atomic, data)
 
 
 async def update_hardware_config(updates: dict[str, Any]) -> None:
@@ -29,6 +55,25 @@ async def update_hardware_config(updates: dict[str, Any]) -> None:
         await write_hardware_config(data)
 
 
+def read_hardware_config_sync() -> dict[str, Any]:
+    """Blocking counterpart of `read_hardware_config`, for threaded services."""
+    with open(HARDWARE_CONFIG_PATH, "r") as f:
+        return cast(dict[str, Any], json.load(f))
+
+
+def update_hardware_config_sync(updates: dict[str, Any]) -> None:
+    """Blocking counterpart of `update_hardware_config`, for threaded services.
+
+    The imager runs on threads rather than asyncio, so it cannot take the asyncio lock
+    above. That lock only ever served to serialize writers inside a single process anyway
+    — pump, light and imager are separate processes — and the atomic replace is what
+    actually protects the file.
+    """
+    data = read_hardware_config_sync()
+    data.update(updates)
+    _write_hardware_config_atomic(data)
+
+
 async def get_hat_version() -> float | None:
     try:
         hardware = await read_hardware_config()

controller/imager/camera/mqtt.py

@@ -10,6 +10,7 @@
 
 import loguru
 
+import helpers
 import mqtt as messaging
 
 from . import hardware
@@ -199,6 +200,9 @@ def _receive_message(self, message: dict[str, typing.Any]) -> typing.Optional[st
             self.capture(message["payload"])
             return None
 
+        if message["payload"].get("action", "") == "save_settings":
+            return self._save_settings()
+
         if message["payload"].get("action", "") != "settings":
             return None
 
@@ -223,6 +227,30 @@ def _receive_message(self, message: dict[str, typing.Any]) -> typing.Optional[st
         loguru.logger.success("Updated camera settings!")
         return '{"status":"Camera settings updated"}'
 
+    def _save_settings(self) -> str:
+        """Persist the live white-balance gains as the instrument's calibration.
+
+        The camera reads `red_gain`/`blue_gain` out of hardware.json when it starts, so
+        until they are written back there a white-balance calibration only lasts until the
+        next reboot. Only an explicit save persists, which keeps a preview tweak from
+        quietly replacing a good calibration.
+        """
+        assert self._camera is not None
+
+        gains = self._camera.settings.white_balance_gains
+        if gains is None:
+            loguru.logger.error("No white balance gains to save")
+            return '{"status":"Camera settings error"}'
+
+        try:
+            helpers.update_hardware_config_sync({"red_gain": gains.red, "blue_gain": gains.blue})
+        except (OSError, ValueError):
+            loguru.logger.exception("Couldn't persist white balance gains to hardware config")
+            return '{"status":"Camera settings error"}'
+
+        loguru.logger.success(f"Saved white balance gains: red={gains.red}, blue={gains.blue}")
+        return '{"status":"Camera settings saved"}'
+
     @property
     def camera(self) -> typing.Optional[hardware.PiCamera]:
         """Return the camera wrapper managed by this worker.

controller/light/main.py

@@ -14,6 +14,42 @@
 led = None
 chronometer = None
 
+# Key under which the calibrated brightness is persisted in hardware.json.
+INTENSITY_CONFIG_KEY = "led_intensity"
+DEFAULT_INTENSITY = 1.0
+
+# Calibrated brightness, in the range [0, 1]. Restored from hardware.json at startup and
+# used whenever the LED is switched on without an explicit value, so a reboot or an
+# off/on cycle no longer costs the operator their light calibration.
+intensity = DEFAULT_INTENSITY
+
+
+def _clamp(value: float) -> float:
+    return max(0.0, min(float(value), 1.0))
+
+
+async def load_intensity() -> None:
+    """Restore the calibrated brightness from hardware.json.
+
+    A missing or malformed value is not fatal — the LED simply falls back to full
+    brightness, which is the behaviour this instrument had before the value was persisted.
+    """
+    global intensity
+    try:
+        config = await helpers.read_hardware_config()
+    except (OSError, ValueError) as e:
+        print(f"Could not read hardware config, using default LED intensity: {e}")
+        return
+
+    stored = config.get(INTENSITY_CONFIG_KEY)
+    if stored is None:
+        return
+
+    try:
+        intensity = _clamp(stored)
+    except (TypeError, ValueError):
+        print(f"Ignoring invalid {INTENSITY_CONFIG_KEY} in hardware config: {stored!r}")
+
 
 async def start() -> None:
     global led
@@ -32,6 +68,9 @@ async def start() -> None:
     else:
         raise Exception("Unknown hat_version", hat_version)
 
+    # The LED stays off until asked to turn on; this only restores the level it will use.
+    await load_intensity()
+
     global client
     client = aiomqtt.Client(hostname="localhost", port=1883, protocol=aiomqtt.ProtocolVersion.V5)
     task_group = asyncio.TaskGroup()
@@ -77,19 +116,27 @@ async def handle_action(action: str, payload) -> None:
 
 async def on(payload) -> None:
     assert led is not None
-    value = payload.get("value", 1)
-    value = max(0.0, min(value, 1))
+    global intensity
+
+    # No explicit value means "switch on as calibrated", which is what restores the
+    # operator's setting after a reboot or an off/on cycle.
+    requested = payload.get("value")
+    value = intensity if requested is None else _clamp(requested)
 
     if value == 0:
         await off()
         return
 
+    intensity = value
+
     global chronometer
     if chronometer is None:
         chronometer = int(time.time())
 
-    led.on()
+    # Set the level before enabling the output, otherwise the LED briefly lights at
+    # whatever level the driver defaults to.
     led.set_value(value)
+    led.on()
 
     await publish_status()
 
@@ -107,19 +154,37 @@ async def off() -> None:
 
 
 async def save() -> None:
+    """Persist the current brightness as the calibrated one.
+
+    hardware.json is the source of truth restored at startup. On the v3 HAT the DAC can
+    also latch the level in its own EEPROM, which is kept because it makes the LED come up
+    correctly even before this service starts; the v2.6 LM36011 has no EEPROM and its
+    `save()` is a no-op, which is exactly why the level cannot live in hardware alone.
+    """
     assert led is not None
 
+    try:
+        await helpers.update_hardware_config({INTENSITY_CONFIG_KEY: intensity})
+    except (OSError, ValueError) as e:
+        # A failed save must not take down the MQTT handler; the LED keeps working at the
+        # requested level, it just will not survive a reboot.
+        print(f"Could not persist LED intensity: {e}")
+        return
+
     if hasattr(led, "save"):
         led.save()
 
+    await publish_status()
+
 
 async def publish_status() -> None:
     assert client is not None
     assert led is not None
 
-    value = led.get_value()
-
-    payload = {"status": "Off" if led.is_off() else "On", "value": value}
+    # Report the calibrated level rather than reading the hardware back. The DAC reads 0
+    # whenever the LED is off, and the LM36011 rounds its torch current to whole units on
+    # readback, so neither can tell the dashboard which brightness to show at startup.
+    payload = {"status": "Off" if led.is_off() else "On", "value": intensity}
     await client.publish(topic="status/light", payload=json.dumps(payload), retain=True)