diff --git a/pylabrobot/hamilton/star/design.md b/pylabrobot/hamilton/star/design.md index 9d38ab002dc..2017f26062f 100644 --- a/pylabrobot/hamilton/star/design.md +++ b/pylabrobot/hamilton/star/design.md @@ -37,9 +37,9 @@ hand one its configuration before setup and re-running setup does not throw it a **P3. Every feature has a `.configuration` dataclass, and it holds facts only.** Three kinds, in this order: what the device answered about itself, per-machine calibration, and the device facts of -that generation of drive - encoder resolutions, the ranges each drive accepts, and the firmware's -own defaults. What the driver chooses to send is not a fact about the device and does not go here -(P18). `DeviceConfiguration` in `driver/configuration.py` is the same thing for the device. +that generation of drive - encoder resolutions and the ranges each drive accepts. A default is not a +fact and does not go here, not even the firmware's own (P18). `DeviceConfiguration` in +`driver/configuration.py` is the same thing for the device. **P4. The wire counts in increments; this driver speaks mm, degrees, uL and seconds.** Every conversion is a method on the configuration - `x_increments_to_mm` / `x_mm_to_increments` - so the @@ -130,7 +130,8 @@ that a constant already holds are referenced by name rather than repeated. **P18. The driver's defaults are public `default_*` attributes on the feature.** Declared on the class with a type, in standard units, round, and slightly below the firmware's own default. A caller tunes one device by assigning on the instance, or every device by assigning on the class. -Named `default_[_]`: `default_y_speed`, `default_z_speed`. +Named `default_[_]`: `default_y_speed`, `default_z_speed`. Never on the +configuration. **P19. A probe names its speeds for what they do.** `search_speed` is the speed it searches at; `approach_speed` is the speed to where the search starts, where the command has one. Never a bare @@ -185,7 +186,7 @@ the Prep's. the oldest of the configurations and reads like the document it was transcribed from rather than like the rest of them. -8. **The driver's defaults still live in the configurations.** `Head`, `Head384`, `iSWAP`, - `Autoload` and `XArm` keep them as `*_default` / `*_default_increments` fields, some in - increments, and `Pipettes` keeps a few there. `Head96` and `CoreGrippers` follow P18; the rest - move over one feature at a time. +8. **The driver's defaults still live in the configurations.** `Head`, `Head384`, `iSWAP` and + `Autoload` keep them as `*_default` / `*_default_increments` fields, some in increments, and + `Pipettes` keeps a few there. `Head96`, `CoreGrippers` and `XArm` follow P18; the rest move over + one feature at a time. diff --git a/pylabrobot/hamilton/star/driver/features/head.py b/pylabrobot/hamilton/star/driver/features/head.py index 1550a81bc56..f5e4cd2f194 100644 --- a/pylabrobot/hamilton/star/driver/features/head.py +++ b/pylabrobot/hamilton/star/driver/features/head.py @@ -974,8 +974,8 @@ async def request_x_position(self) -> float: async def move_to_x_position( self, x: float, - acceleration_level: int = 3, - current_limit: int = 7, + acceleration_level: Optional[int] = None, + current_limit: Optional[int] = None, settle_reads: int = 20, ): """Move channel A1 along X. The whole arm travels, with everything else it carries. @@ -985,8 +985,8 @@ async def move_to_x_position( Args: x: where to put channel A1, in mm. - acceleration_level: how hard to accelerate, 1 to 4. - current_limit: the motor current limit, 1 to 7. + acceleration_level: how hard to accelerate. Defaults to the X-arm that holds the head. + current_limit: the motor current limit. Defaults to the X-arm that holds the head. settle_reads: how many reads to take before calling the arm stopped. Raises: diff --git a/pylabrobot/hamilton/star/driver/features/iswap.py b/pylabrobot/hamilton/star/driver/features/iswap.py index 47299cdc712..50b72475c27 100644 --- a/pylabrobot/hamilton/star/driver/features/iswap.py +++ b/pylabrobot/hamilton/star/driver/features/iswap.py @@ -1161,8 +1161,8 @@ async def elbow_request_x_position(self) -> float: async def elbow_move_to_x_position( self, x: float, - acceleration_level: int = 3, - current_limit: int = 7, + acceleration_level: Optional[int] = None, + current_limit: Optional[int] = None, settle_reads: int = 20, ): """Move the elbow along X. The whole arm travels, with everything else it carries. @@ -1172,8 +1172,8 @@ async def elbow_move_to_x_position( Args: x: where to put the elbow, in deck mm. - acceleration_level: how hard to accelerate, 1 to 4. - current_limit: the motor current limit, 1 to 7. + acceleration_level: how hard to accelerate. Defaults to the X-arm that holds the iSWAP. + current_limit: the motor current limit. Defaults to the X-arm that holds the iSWAP. settle_reads: how many reads to take before calling the arm stopped. Raises: diff --git a/pylabrobot/hamilton/star/driver/features/pipettes.py b/pylabrobot/hamilton/star/driver/features/pipettes.py index 931221ef832..7c2eaca156c 100644 --- a/pylabrobot/hamilton/star/driver/features/pipettes.py +++ b/pylabrobot/hamilton/star/driver/features/pipettes.py @@ -1303,16 +1303,16 @@ async def request_x_position(self) -> float: async def move_to_x_position( self, x: float, - acceleration_level: int = 3, - current_limit: int = 7, + acceleration_level: Optional[int] = None, + current_limit: Optional[int] = None, settle_reads: int = 20, ): """Move the channels along X. The whole arm travels, with everything else it carries. Args: x: where to go, in mm. - acceleration_level: how hard to accelerate, 1 to 4. - current_limit: the motor current limit, 1 to 7. + acceleration_level: how hard to accelerate. Defaults to the X-arm that holds the pipettes. + current_limit: the motor current limit. Defaults to the X-arm that holds the pipettes. settle_reads: how many reads to take before calling the arm stopped. Raises: diff --git a/pylabrobot/hamilton/star/driver/features/x_arm.py b/pylabrobot/hamilton/star/driver/features/x_arm.py index 991870ce088..6913f92cd0c 100644 --- a/pylabrobot/hamilton/star/driver/features/x_arm.py +++ b/pylabrobot/hamilton/star/driver/features/x_arm.py @@ -69,8 +69,6 @@ class XArmConfiguration: x_mm_per_increment: float = 0.1 x_range_increments: Tuple[int, int] = (0, 30_000) # what the move accepts; x_range is narrower acceleration_level_range: Tuple[int, int] = (1, 5) # index into five curves, not a rate - acceleration_level_default: int = 4 - current_limit_default: int = 7 @property def current_limit_digits(self) -> int: @@ -98,8 +96,6 @@ def with_device_facts_of(self, other: "XArmConfiguration") -> "XArmConfiguration x_mm_per_increment=other.x_mm_per_increment, x_range_increments=other.x_range_increments, acceleration_level_range=other.acceleration_level_range, - acceleration_level_default=other.acceleration_level_default, - current_limit_default=other.current_limit_default, ) # -- conversions: the wire counts in steps, the driver speaks mm --------------------------- @@ -212,6 +208,9 @@ class XArm: is, how far it travels, and how far along X what it carries reaches. """ + default_acceleration_level: int = 3 + default_current_limit: int = 7 + def __init__( self, driver: "STARDriver", @@ -354,15 +353,14 @@ async def initialize(self, current_limit: Optional[int] = None): """Initialize this arm's drive. This moves it. Args: - current_limit: the motor current limit. Defaults to - `configuration.current_limit_default`. + current_limit: the motor current limit. Defaults to `default_current_limit`. Raises: ValueError: If the current limit is outside what the drive accepts. """ c = self.configuration # The parameter is sent, so what the drive does is written here rather than left to the drive's # own default, which nothing would record. - current_limit = c.current_limit_default if current_limit is None else current_limit + current_limit = self.default_current_limit if current_limit is None else current_limit low, high = c.current_limit_range if not low <= current_limit <= high: raise ValueError(f"current_limit must be between {low} and {high}, is {current_limit}") @@ -478,8 +476,8 @@ async def _record_where_it_stopped(self) -> None: async def move_to_x_position( self, x: float, - acceleration_level: int = 3, - current_limit: int = 7, + acceleration_level: Optional[int] = None, + current_limit: Optional[int] = None, settle_reads: int = 20, ): """Move the arm to an absolute X position. @@ -490,12 +488,12 @@ async def move_to_x_position( Args: x: target X position in mm, at the arm's reference point. Must lie within the arm's travel range (`configuration.x_range`). - acceleration_level: which acceleration curve to use. The drive's own default is - `configuration.acceleration_level_default`; this is the gentler one legacy sends. The - hardest curve leaves the arm oscillating about its target rather than approaching it, so it - takes longer to come to rest and further still with a 96-head parked forward - it arrives - either way, but the settling read below has more to wait for. - current_limit: the motor current limit. + acceleration_level: which acceleration curve to use. Defaults to + `default_acceleration_level`, gentler than the drive's own 4. The hardest curve leaves the + arm oscillating about its target rather than approaching it, so it takes longer to come to + rest and further still with a 96-head parked forward - it arrives either way, but the + settling read below has more to wait for. + current_limit: the motor current limit. Defaults to `default_current_limit`. settle_reads: how many reads to spend waiting for the arm to come to rest. Each is a command round trip, about 10 ms, against a settle of 27 to 90 ms where this was measured. Raises: @@ -504,6 +502,10 @@ async def move_to_x_position( """ c = self.configuration self._check_reachable(x) + if acceleration_level is None: + acceleration_level = self.default_acceleration_level + if current_limit is None: + current_limit = self.default_current_limit low, high = c.acceleration_level_range if not low <= acceleration_level <= high: raise ValueError( @@ -565,8 +567,8 @@ async def _unchecked_fw_move_x_with_attached_components_at_z_safety(self, x: flo async def move_x_relative( self, distance: float, - acceleration_level: int = 3, - current_limit: int = 7, + acceleration_level: Optional[int] = None, + current_limit: Optional[int] = None, ): """Move the arm by a distance from where it is now. diff --git a/pylabrobot/hamilton/star/driver/features/x_arm_tests.py b/pylabrobot/hamilton/star/driver/features/x_arm_tests.py index f46ca01893b..98f5ac3b722 100644 --- a/pylabrobot/hamilton/star/driver/features/x_arm_tests.py +++ b/pylabrobot/hamilton/star/driver/features/x_arm_tests.py @@ -103,6 +103,16 @@ async def test_left_drive(self): ["X0XIlw7", "X0XPla05000lr3lw7", "X0XPla04875lr3lw7", "X0XO"], ) + async def test_the_defaults_are_the_arms(self): + driver = await _both_arms() + arm = cast(XArm, driver.left_x_arm) + sent = record(arm) + arm.default_acceleration_level = 2 + arm.default_current_limit = 5 + await XArm.initialize(arm) + await XArm.move_to_x_position(arm, 500.0) + self.assertEqual(sent, ["X0XIlw5", "X0XPla05000lr2lw5"]) + async def test_a_firmware_5_drive_takes_a_two_digit_current_limit(self): """From X0 firmware 5.0 the limiter is `lw##`, 00..15.""" left = dataclasses.replace( @@ -326,23 +336,23 @@ class TestConfiguringAnArm(unittest.IsolatedAsyncioTestCase): async def test_what_is_written_is_where_the_device_holds_it(self): driver = await _both_arms() arm = cast(XArm, driver.left_x_arm) - corrected = dataclasses.replace(arm.configuration, current_limit_default=3) + corrected = dataclasses.replace(arm.configuration, acceleration_level_range=(1, 3)) arm.configuration = corrected - self.assertEqual(arm.configuration.current_limit_default, 3) + self.assertEqual(arm.configuration.acceleration_level_range, (1, 3)) self.assertIs(cast(DeviceConfiguration, driver.configuration).left_arm, corrected) async def test_the_constructor_takes_one_too(self): """The same write, done where every other feature takes its configuration.""" driver = await _both_arms() corrected = dataclasses.replace( - cast(XArm, driver.left_x_arm).configuration, current_limit_default=3 + cast(XArm, driver.left_x_arm).configuration, acceleration_level_range=(1, 3) ) arm = XArm(driver, side="left", configuration=corrected) - self.assertEqual(arm.configuration.current_limit_default, 3) + self.assertEqual(arm.configuration.acceleration_level_range, (1, 3)) self.assertIs(cast(DeviceConfiguration, driver.configuration).left_arm, corrected) async def test_it_refuses_before_the_device_has_been_read(self): @@ -357,27 +367,27 @@ async def test_a_simulated_device_then_answers_what_was_written(self): arm's device facts across a re-read as a physical device's does - so what was written stays.""" driver = await _both_arms() arm = cast(XArm, driver.left_x_arm) - arm.configuration = dataclasses.replace(arm.configuration, current_limit_default=3) + arm.configuration = dataclasses.replace(arm.configuration, acceleration_level_range=(1, 3)) await driver.discover() - self.assertEqual(arm.configuration.current_limit_default, 3) + self.assertEqual(arm.configuration.acceleration_level_range, (1, 3)) def test_device_facts_carry_over_and_readings_do_not(self): """What a physical device's discovery does with a configured arm. It rebuilds one from the reply, then takes the device facts off the arm as it was configured: those are what no device answers, so a re-read must not put them back to what this generation documents.""" - configured = dataclasses.replace(BARE_X_ARM, current_limit_default=15) + configured = dataclasses.replace(BARE_X_ARM, acceleration_level_range=(1, 3)) answered = dataclasses.replace(BARE_X_ARM, width=354.0, x_range=(95.0, 1340.2)) kept = answered.with_device_facts_of(configured) - self.assertEqual(kept.current_limit_default, 15) + self.assertEqual(kept.acceleration_level_range, (1, 3)) self.assertEqual(kept.width, 354.0) self.assertEqual(kept.x_range, (95.0, 1340.2)) # Neither of the two it was worked out from is changed. self.assertEqual(configured.width, BARE_X_ARM.width) - self.assertEqual(answered.current_limit_default, BARE_X_ARM.current_limit_default) + self.assertEqual(answered.acceleration_level_range, BARE_X_ARM.acceleration_level_range) class TestGeometryDoesNotFollowTheReportedWidth(unittest.TestCase): 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..2b3d0091d68 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 @@ -65,9 +65,7 @@ "acceleration_level_range": [ 1, 5 - ], - "acceleration_level_default": 4, - "current_limit_default": 7 + ] }, "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..6de66b3c0e0 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 @@ -65,9 +65,7 @@ "acceleration_level_range": [ 1, 5 - ], - "acceleration_level_default": 4, - "current_limit_default": 7 + ] }, "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..87f961e42cb 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 @@ -68,9 +68,7 @@ "acceleration_level_range": [ 1, 5 - ], - "acceleration_level_default": 4, - "current_limit_default": 7 + ] }, "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..ccea270e321 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 @@ -68,9 +68,7 @@ "acceleration_level_range": [ 1, 5 - ], - "acceleration_level_default": 4, - "current_limit_default": 7 + ] }, "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..aa4b8fa6431 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 @@ -68,9 +68,7 @@ "acceleration_level_range": [ 1, 5 - ], - "acceleration_level_default": 4, - "current_limit_default": 7 + ] }, "right_arm": null, "min_iswap_collision_free_position": 350.0,