STAR save_configuration: firmware_variable values written beside the fields

pylabrobot/hamilton/star/design.md

@@ -111,6 +111,8 @@ them in step. A driver given no deck models nothing and still drives.
 file. Against a simulated device it stands in for the device; against a physical one it is
 cross-checked against what answers, on what is fitted and how much of it, and setup refuses if they
 disagree. Values that come from a drive's documentation rather than from a device stay in the code.
+The ones the firmware version decides are listed in the class's `firmware_variable` and written in
+a `firmware_variable` block beside the fields: shown, never read back, and checked against the code.
 
 **P14. The simulator overrides only what would reach the wire.** One subclass per feature, each
 implementing `answer` from the model; everything above it - discovery, the initialization order,

pylabrobot/hamilton/star/driver/features/x_arm.py

@@ -4,7 +4,7 @@
 import datetime
 import logging
 from dataclasses import dataclass
-from typing import TYPE_CHECKING, Any, Dict, Literal, Optional, Tuple, cast
+from typing import TYPE_CHECKING, Any, ClassVar, Dict, Literal, Optional, Tuple, cast
 
 from pylabrobot.hamilton.protocol.text.framing import parse_firmware_version_date
 from pylabrobot.resources.coordinate import Coordinate
@@ -72,6 +72,8 @@ class XArmConfiguration:
   acceleration_level_default: int = 4
   current_limit_default: int = 7
 
+  firmware_variable: ClassVar[Tuple[str, ...]] = ("current_limit_range", "current_limit_digits")
+
   @property
   def current_limit_digits(self) -> int:
     major = (self.firmware_version or "").split(".", 1)[0]

pylabrobot/hamilton/star/driver/master.py

@@ -86,6 +86,19 @@ def _range(values: Optional[Tuple[float, float]]) -> str:
   return "unresolved" if values is None else f"{values[0]} to {values[1]} mm"
 
 
+def _serialize_configuration(configuration: Any) -> Dict[str, Any]:
+  """A configuration's fields, and the values its firmware version decides."""
+  saved = cast(Dict[str, Any], serialize(dataclasses.asdict(configuration)))
+  for field in dataclasses.fields(configuration):
+    value = getattr(configuration, field.name)
+    if dataclasses.is_dataclass(value) and not isinstance(value, type):
+      saved[field.name] = _serialize_configuration(value)
+  names = getattr(configuration, "firmware_variable", ())
+  if names:
+    saved["firmware_variable"] = serialize({name: getattr(configuration, name) for name in names})
+  return saved
+
+
 class STARDriver:
   """Interface for the Hamilton STARDriver."""
 
@@ -1543,12 +1556,12 @@ def _saved_configuration(self) -> Dict[str, Any]:
       raise RuntimeError("nothing has been read off this device; call `setup` first")
 
     saved: Dict[str, Any] = {
-      "device": serialize(dataclasses.asdict(self.configuration)),
+      "device": _serialize_configuration(self.configuration),
       "arms": {},
     }
     for arm in self.arms:
       carried = {
-        name: serialize(dataclasses.asdict(feature.configuration))
+        name: _serialize_configuration(feature.configuration)
         for name, feature in (
           ("pipettes", arm.pipettes),
           ("head96", arm.head96),
@@ -1560,7 +1573,7 @@ def _saved_configuration(self) -> Dict[str, Any]:
       if carried:
         saved["arms"][arm.side] = carried
     if self.autoload is not None:
-      saved["autoload"] = serialize(dataclasses.asdict(self.autoload.configuration))
+      saved["autoload"] = _serialize_configuration(self.autoload.configuration)
     return saved
 
   def save_configuration(self, path: str, indent: Optional[int] = 2) -> None:

pylabrobot/hamilton/star/driver/master_tests.py

@@ -193,17 +193,42 @@ def keys_no_field_reads(saved: dict, configuration: object) -> List[str]:
     Every key with no field to read it into, as a dotted path from `saved`.
   """
   fields = {field.name: field for field in dataclasses.fields(configuration)}  # type: ignore[arg-type]
-  dropped = [key for key in saved if key not in fields]
+  dropped = [key for key in saved if key not in fields and key != "firmware_variable"]
   for key, value in saved.items():
     nested = getattr(configuration, key, None)
     if key in fields and isinstance(value, dict) and dataclasses.is_dataclass(nested):
       dropped += [f"{key}.{inner}" for inner in keys_no_field_reads(value, nested)]
   return dropped
 
 
+def firmware_variable_mismatches(saved: dict, configuration: object) -> List[str]:
+  """Every `firmware_variable` value a saved configuration holds that the code does not give.
+
+  Args:
+    saved: the configuration as JSON holds it.
+    configuration: what reading it back built.
+
+  Returns:
+    Every mismatched or missing name, as a dotted path from `saved`.
+  """
+  names = getattr(configuration, "firmware_variable", ())
+  block = saved.get("firmware_variable", {})
+  wrong = [
+    f"firmware_variable.{name}"
+    for name in sorted(set(names) | set(block))
+    if name not in names or block.get(name) != serialize(getattr(configuration, name))
+  ]
+  for key, value in saved.items():
+    nested = getattr(configuration, key, None)
+    if isinstance(value, dict) and dataclasses.is_dataclass(nested):
+      wrong += [f"{key}.{inner}" for inner in firmware_variable_mismatches(value, nested)]
+  return wrong
+
+
 class TestRecordings(unittest.TestCase):
-  """What ships under recordings/ is read back whole: no key a configuration no longer has, and no
-  window left empty for a head to be built on."""
+  """What ships under recordings/ is read back whole: no key a configuration no longer has, every
+  `firmware_variable` value what the code gives, and no window left empty for a head to be built
+  on."""
 
   def test_every_key_is_a_field(self):
     for path in sorted(pathlib.Path(RECORDING_STAR).parent.glob("*.json")):
@@ -220,6 +245,7 @@ def test_every_key_is_a_field(self):
       for where, value, configuration in sections:
         with self.subTest(recording=path.name, section=where):
           self.assertEqual(keys_no_field_reads(value, configuration), [])
+          self.assertEqual(firmware_variable_mismatches(value, configuration), [])
 
   def test_every_head_has_a_z_range(self):
     for path in sorted(pathlib.Path(RECORDING_STAR).parent.glob("*.json")):
@@ -252,6 +278,14 @@ async def test_a_device_that_is_not_what_was_declared_is_refused(self):
     with self.assertRaisesRegex(ValueError, "autoload_installed"):
       await star.setup()
 
+  async def test_a_saved_arm_carries_its_firmware_variable_values(self):
+    star = STARSimulationDriver(deck=STARDeck(), declared_configuration_json=RECORDING_STAR)
+    await star.setup()
+    self.assertEqual(
+      star._saved_configuration()["device"]["left_arm"]["firmware_variable"],
+      {"current_limit_range": [0, 7], "current_limit_digits": 1},
+    )
+
 
 class TestRepeatedSetup(unittest.IsolatedAsyncioTestCase):
   """Setup is repeatable: a second one re-reads the device over the link the first one opened."""

pylabrobot/hamilton/star/driver/recordings/star_legacy_2021_8ch_head384_autoload1D.json

@@ -67,7 +67,14 @@
         5
       ],
       "acceleration_level_default": 4,
-      "current_limit_default": 7
+      "current_limit_default": 7,
+      "firmware_variable": {
+        "current_limit_range": [
+          0,
+          7
+        ],
+        "current_limit_digits": 1
+      }
     },
     "right_arm": null,
     "min_iswap_collision_free_position": 350.0,

pylabrobot/hamilton/star/driver/recordings/star_legacy_2021_8ch_head96_autoload1D.json

@@ -67,7 +67,14 @@
         5
       ],
       "acceleration_level_default": 4,
-      "current_limit_default": 7
+      "current_limit_default": 7,
+      "firmware_variable": {
+        "current_limit_range": [
+          0,
+          7
+        ],
+        "current_limit_digits": 1
+      }
     },
     "right_arm": null,
     "min_iswap_collision_free_position": 350.0,

pylabrobot/hamilton/star/driver/recordings/starlet_legacy_2021_8ch_head384_autoload1D.json

@@ -70,7 +70,14 @@
         5
       ],
       "acceleration_level_default": 4,
-      "current_limit_default": 7
+      "current_limit_default": 7,
+      "firmware_variable": {
+        "current_limit_range": [
+          0,
+          7
+        ],
+        "current_limit_digits": 1
+      }
     },
     "right_arm": null,
     "min_iswap_collision_free_position": 350.0,

pylabrobot/hamilton/star/driver/recordings/starlet_legacy_2021_8ch_head96_autoload1D.json

@@ -70,7 +70,14 @@
         5
       ],
       "acceleration_level_default": 4,
-      "current_limit_default": 7
+      "current_limit_default": 7,
+      "firmware_variable": {
+        "current_limit_range": [
+          0,
+          7
+        ],
+        "current_limit_digits": 1
+      }
     },
     "right_arm": null,
     "min_iswap_collision_free_position": 350.0,

pylabrobot/hamilton/star/driver/recordings/starplus_legacy_2021_8ch_head96.json

@@ -70,7 +70,14 @@
         5
       ],
       "acceleration_level_default": 4,
-      "current_limit_default": 7
+      "current_limit_default": 7,
+      "firmware_variable": {
+        "current_limit_range": [
+          0,
+          7
+        ],
+        "current_limit_digits": 1
+      }
     },
     "right_arm": null,
     "min_iswap_collision_free_position": 350.0,