diff --git a/pylabrobot/hamilton/star/design.md b/pylabrobot/hamilton/star/design.md index 9d38ab002dc..760f78ca5fd 100644 --- a/pylabrobot/hamilton/star/design.md +++ b/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, diff --git a/pylabrobot/hamilton/star/driver/features/x_arm.py b/pylabrobot/hamilton/star/driver/features/x_arm.py index 991870ce088..0fc88855d71 100644 --- a/pylabrobot/hamilton/star/driver/features/x_arm.py +++ b/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] diff --git a/pylabrobot/hamilton/star/driver/master.py b/pylabrobot/hamilton/star/driver/master.py index aa3cd92d4d7..0d16ae73639 100644 --- a/pylabrobot/hamilton/star/driver/master.py +++ b/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: diff --git a/pylabrobot/hamilton/star/driver/master_tests.py b/pylabrobot/hamilton/star/driver/master_tests.py index c121a382005..ec888561360 100644 --- a/pylabrobot/hamilton/star/driver/master_tests.py +++ b/pylabrobot/hamilton/star/driver/master_tests.py @@ -193,7 +193,7 @@ 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): @@ -201,9 +201,34 @@ def keys_no_field_reads(saved: dict, configuration: object) -> List[str]: 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.""" diff --git a/pylabrobot/hamilton/star/driver/recordings/star_legacy_2021_8ch_head384_autoload1D.json b/pylabrobot/hamilton/star/driver/recordings/star_legacy_2021_8ch_head384_autoload1D.json index 79c24549136..2afce6672cb 100644 --- a/pylabrobot/hamilton/star/driver/recordings/star_legacy_2021_8ch_head384_autoload1D.json +++ b/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, diff --git a/pylabrobot/hamilton/star/driver/recordings/star_legacy_2021_8ch_head96_autoload1D.json b/pylabrobot/hamilton/star/driver/recordings/star_legacy_2021_8ch_head96_autoload1D.json index c0335611906..e4496fee3d7 100644 --- a/pylabrobot/hamilton/star/driver/recordings/star_legacy_2021_8ch_head96_autoload1D.json +++ b/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, diff --git a/pylabrobot/hamilton/star/driver/recordings/starlet_legacy_2021_8ch_head384_autoload1D.json b/pylabrobot/hamilton/star/driver/recordings/starlet_legacy_2021_8ch_head384_autoload1D.json index c84c07c059b..de790990277 100644 --- a/pylabrobot/hamilton/star/driver/recordings/starlet_legacy_2021_8ch_head384_autoload1D.json +++ b/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, diff --git a/pylabrobot/hamilton/star/driver/recordings/starlet_legacy_2021_8ch_head96_autoload1D.json b/pylabrobot/hamilton/star/driver/recordings/starlet_legacy_2021_8ch_head96_autoload1D.json index 500e0e96f57..0b38c0e0628 100644 --- a/pylabrobot/hamilton/star/driver/recordings/starlet_legacy_2021_8ch_head96_autoload1D.json +++ b/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, diff --git a/pylabrobot/hamilton/star/driver/recordings/starplus_legacy_2021_8ch_head96.json b/pylabrobot/hamilton/star/driver/recordings/starplus_legacy_2021_8ch_head96.json index 07f32b196c6..fc416c994a6 100644 --- a/pylabrobot/hamilton/star/driver/recordings/starplus_legacy_2021_8ch_head96.json +++ b/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,