From dd535088324892d7bb4c66c8bcb0b7724f2bce4d Mon Sep 17 00:00:00 2001 From: aaron-kulkarni <107003644+aaron-kulkarni@users.noreply.github.com> Date: Thu, 18 Jul 2024 08:58:27 -0400 Subject: [PATCH 1/6] refactor(shared-data): remove outdated id fields from JSON v7-v8 fixtures (#15694) replace all instances of Command `id` with `key` in the v7 protocol fixtures(v8 doesn't have any instances of `id`) continuation of #15690 # Overview # Test Plan # Changelog # Review requests # Risk assessment --- shared-data/protocol/fixtures/7/simpleV7.json | 52 +++++++++---------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/shared-data/protocol/fixtures/7/simpleV7.json b/shared-data/protocol/fixtures/7/simpleV7.json index 5ba560f83ca..aa0b78f9915 100644 --- a/shared-data/protocol/fixtures/7/simpleV7.json +++ b/shared-data/protocol/fixtures/7/simpleV7.json @@ -1208,7 +1208,7 @@ "commands": [ { "commandType": "loadPipette", - "id": "0abc123", + "key": "0abc123", "params": { "pipetteId": "pipetteId", "pipetteName": "p1000_96", @@ -1217,7 +1217,7 @@ }, { "commandType": "loadModule", - "id": "1abc123", + "key": "1abc123", "params": { "moduleId": "magneticModuleId", "model": "magneticModuleV2", @@ -1226,7 +1226,7 @@ }, { "commandType": "loadModule", - "id": "2abc123", + "key": "2abc123", "params": { "moduleId": "temperatureModuleId", "model": "temperatureModuleV2", @@ -1235,7 +1235,7 @@ }, { "commandType": "loadLabware", - "id": "3abc123", + "key": "3abc123", "params": { "labwareId": "sourcePlateId", "loadName": "armadillo_96_wellplate_200ul_pcr_full_skirt", @@ -1249,7 +1249,7 @@ }, { "commandType": "loadLabware", - "id": "4abc123", + "key": "4abc123", "params": { "labwareId": "destPlateId", "loadName": "armadillo_96_wellplate_200ul_pcr_full_skirt", @@ -1263,7 +1263,7 @@ }, { "commandType": "loadLabware", - "id": "5abc123", + "key": "5abc123", "params": { "labwareId": "tipRackId", "location": { "slotName": "8" }, @@ -1275,7 +1275,7 @@ }, { "commandType": "loadLabware", - "id": "6abc123", + "key": "6abc123", "params": { "labwareId": "fixedTrash", "location": { @@ -1289,7 +1289,7 @@ }, { "commandType": "loadLiquid", - "id": "7abc123", + "key": "7abc123", "params": { "liquidId": "waterId", "labwareId": "sourcePlateId", @@ -1301,12 +1301,12 @@ }, { "commandType": "home", - "id": "00abc123", + "key": "00abc123", "params": {} }, { "commandType": "pickUpTip", - "id": "8abc123", + "key": "8abc123", "params": { "pipetteId": "pipetteId", "labwareId": "tipRackId", @@ -1315,7 +1315,7 @@ }, { "commandType": "aspirate", - "id": "9abc123", + "key": "9abc123", "params": { "pipetteId": "pipetteId", "labwareId": "sourcePlateId", @@ -1330,14 +1330,14 @@ }, { "commandType": "waitForDuration", - "id": "10abc123", + "key": "10abc123", "params": { "seconds": 42 } }, { "commandType": "dispense", - "id": "11abc123", + "key": "11abc123", "params": { "pipetteId": "pipetteId", "labwareId": "destPlateId", @@ -1352,7 +1352,7 @@ }, { "commandType": "touchTip", - "id": "12abc123", + "key": "12abc123", "params": { "pipetteId": "pipetteId", "labwareId": "destPlateId", @@ -1365,7 +1365,7 @@ }, { "commandType": "blowout", - "id": "13abc123", + "key": "13abc123", "params": { "pipetteId": "pipetteId", "labwareId": "destPlateId", @@ -1379,7 +1379,7 @@ }, { "commandType": "moveToCoordinates", - "id": "14abc123", + "key": "14abc123", "params": { "pipetteId": "pipetteId", "coordinates": { "x": 100, "y": 100, "z": 100 } @@ -1387,7 +1387,7 @@ }, { "commandType": "moveToWell", - "id": "15abc123", + "key": "15abc123", "params": { "pipetteId": "pipetteId", "labwareId": "destPlateId", @@ -1396,7 +1396,7 @@ }, { "commandType": "moveToWell", - "id": "16abc123", + "key": "16abc123", "params": { "pipetteId": "pipetteId", "labwareId": "destPlateId", @@ -1411,7 +1411,7 @@ }, { "commandType": "dropTip", - "id": "17abc123", + "key": "17abc123", "params": { "pipetteId": "pipetteId", "labwareId": "fixedTrash", @@ -1420,7 +1420,7 @@ }, { "commandType": "waitForResume", - "id": "18abc123", + "key": "18abc123", "params": { "message": "pause command" } @@ -1428,7 +1428,7 @@ { "commandType": "moveToCoordinates", - "id": "16abc123", + "key": "16abc123", "params": { "pipetteId": "pipetteId", "coordinates": { "x": 0, "y": 0, "z": 0 }, @@ -1438,7 +1438,7 @@ }, { "commandType": "moveRelative", - "id": "18abc123", + "key": "18abc123", "params": { "pipetteId": "pipetteId", "axis": "x", @@ -1447,7 +1447,7 @@ }, { "commandType": "moveRelative", - "id": "19abc123", + "key": "19abc123", "params": { "pipetteId": "pipetteId", "axis": "y", @@ -1456,14 +1456,14 @@ }, { "commandType": "savePosition", - "id": "21abc123", + "key": "21abc123", "params": { "pipetteId": "pipetteId" } }, { "commandType": "moveRelative", - "id": "20abc123", + "key": "20abc123", "params": { "pipetteId": "pipetteId", "axis": "z", @@ -1472,7 +1472,7 @@ }, { "commandType": "savePosition", - "id": "21abc123", + "key": "21abc123", "params": { "pipetteId": "pipetteId", "positionId": "positionId" From 5b09f720c27ab909032f524c9c5e7b7f17ee8060 Mon Sep 17 00:00:00 2001 From: Laura Cox <31892318+Laura-Danielle@users.noreply.github.com> Date: Thu, 18 Jul 2024 17:57:30 +0300 Subject: [PATCH 2/6] feat(api): Update robot deck extents to check the full pipette bounds (#15532) # Overview Previously, we were only checking for a very specific set of bounds for the robot. Now, we should be able to check the exact "absolute" value that a pipette type can move to and throw and error if we exceed those bounds. # Test Plan - Put a bunch of protocols through an app and see whether the software correctly determines if something is out of bounds # Changelog - Added two new components to the deck definition for 'extents' and 'mount offsets' - Added accessors for that in the protocol engine - Modified `_is_within_pipette_extents` function to check the absolute bound per pipette type per mount without any offsets # Review requests Pls check my math, I'm almost sure I did one or two things wrong # Risk assessment Medium. If we don't get this right it could throw erroneous deck bound errors. --- .../protocol_api/core/engine/deck_conflict.py | 81 +++++++------------ .../protocol_engine/create_protocol_engine.py | 4 +- .../state/addressable_areas.py | 24 +++++- .../protocol_engine/state/geometry.py | 26 ++++++ .../protocol_engine/state/pipettes.py | 17 ++++ .../opentrons/protocol_engine/state/state.py | 4 + .../core/engine/test_deck_conflict.py | 73 +++++++++++++++-- .../test_pipette_movement_deck_conflicts.py | 39 ++++++--- .../state/test_addressable_area_state.py | 11 +++ .../state/test_addressable_area_store.py | 33 ++++++++ .../state/test_addressable_area_view.py | 11 +++ .../state/test_geometry_view.py | 17 +++- .../state/test_module_store.py | 11 +++ .../protocol_engine/state/test_module_view.py | 11 +++ .../state/test_pipette_store.py | 2 + .../state/test_pipette_view.py | 17 +++- .../protocol_engine/state/test_state_store.py | 7 ++ .../deck/definitions/4/ot2_standard.json | 7 +- .../opentrons_shared_data/robot/dev_types.py | 12 ++- shared-data/robot/definitions/1/ot2.json | 7 +- shared-data/robot/definitions/1/ot3.json | 8 +- shared-data/robot/schemas/1.json | 32 +++++++- 22 files changed, 377 insertions(+), 77 deletions(-) diff --git a/api/src/opentrons/protocol_api/core/engine/deck_conflict.py b/api/src/opentrons/protocol_api/core/engine/deck_conflict.py index 2a50964e757..405aa2256a7 100644 --- a/api/src/opentrons/protocol_api/core/engine/deck_conflict.py +++ b/api/src/opentrons/protocol_api/core/engine/deck_conflict.py @@ -16,7 +16,6 @@ from opentrons_shared_data.errors.exceptions import MotionPlanningFailureError from opentrons_shared_data.module import FLEX_TC_LID_COLLISION_ZONE -from opentrons.hardware_control.nozzle_manager import NozzleConfigurationType from opentrons.hardware_control.modules.types import ModuleType from opentrons.motion_planning import deck_conflict as wrapped_deck_conflict from opentrons.motion_planning import adjacent_slots_getters @@ -63,21 +62,6 @@ def __init__(self, message: str) -> None: _log = logging.getLogger(__name__) -# TODO (spp, 2023-12-06): move this to a location like motion planning where we can -# derive these values from geometry definitions -# Also, verify y-axis extents values for the nozzle columns. -# Bounding box measurements -A12_column_front_left_bound = Point(x=-11.03, y=2) -A12_column_back_right_bound = Point(x=526.77, y=506.2) - -_NOZZLE_PITCH = 9 -A1_column_front_left_bound = Point( - x=A12_column_front_left_bound.x - _NOZZLE_PITCH * 11, y=2 -) -A1_column_back_right_bound = Point( - x=A12_column_back_right_bound.x - _NOZZLE_PITCH * 11, y=506.2 -) - _FLEX_TC_LID_BACK_LEFT_PT = Point( x=FLEX_TC_LID_COLLISION_ZONE["back_left"]["x"], y=FLEX_TC_LID_COLLISION_ZONE["back_left"]["y"], @@ -244,8 +228,15 @@ def check_safe_for_pipette_movement( ) primary_nozzle = engine_state.pipettes.get_primary_nozzle(pipette_id) + pipette_bounds_at_well_location = ( + engine_state.pipettes.get_pipette_bounds_at_specified_move_to_position( + pipette_id=pipette_id, destination_position=well_location_point + ) + ) if not _is_within_pipette_extents( - engine_state=engine_state, pipette_id=pipette_id, location=well_location_point + engine_state=engine_state, + pipette_id=pipette_id, + pipette_bounding_box_at_loc=pipette_bounds_at_well_location, ): raise PartialTipMovementNotAllowedError( f"Requested motion with the {primary_nozzle} nozzle partial configuration" @@ -253,11 +244,7 @@ def check_safe_for_pipette_movement( ) labware_slot = engine_state.geometry.get_ancestor_slot_name(labware_id) - pipette_bounds_at_well_location = ( - engine_state.pipettes.get_pipette_bounds_at_specified_move_to_position( - pipette_id=pipette_id, destination_position=well_location_point - ) - ) + surrounding_slots = adjacent_slots_getters.get_surrounding_slots( slot=labware_slot.as_int(), robot_type=engine_state.config.robot_type ) @@ -423,42 +410,30 @@ def check_safe_for_tip_pickup_and_return( ) -# TODO (spp, 2023-02-06): update the extents check to use all nozzle bounds instead of -# just position of primary nozzle when checking if the pipette is out-of-bounds def _is_within_pipette_extents( engine_state: StateView, pipette_id: str, - location: Point, + pipette_bounding_box_at_loc: Tuple[Point, Point, Point, Point], ) -> bool: """Whether a given point is within the extents of a configured pipette on the specified robot.""" - robot_type = engine_state.config.robot_type - pipette_channels = engine_state.pipettes.get_channels(pipette_id) - nozzle_config = engine_state.pipettes.get_nozzle_layout_type(pipette_id) - primary_nozzle = engine_state.pipettes.get_primary_nozzle(pipette_id) - if robot_type == "OT-3 Standard": - if pipette_channels == 96 and nozzle_config == NozzleConfigurationType.COLUMN: - # TODO (spp, 2023-12-18): change this eventually to use column mappings in - # the pipette geometry definitions. - if primary_nozzle == "A12": - return ( - A12_column_front_left_bound.x - <= location.x - <= A12_column_back_right_bound.x - and A12_column_front_left_bound.y - <= location.y - <= A12_column_back_right_bound.y - ) - elif primary_nozzle == "A1": - return ( - A1_column_front_left_bound.x - <= location.x - <= A1_column_back_right_bound.x - and A1_column_front_left_bound.y - <= location.y - <= A1_column_back_right_bound.y - ) - # TODO (spp, 2023-11-07): check for 8-channel nozzle A1 & H1 extents on Flex & OT2 - return True + mount = engine_state.pipettes.get_mount(pipette_id) + robot_extent_per_mount = engine_state.geometry.absolute_deck_extents + pip_back_left_bound, pip_front_right_bound, _, _ = pipette_bounding_box_at_loc + pipette_bounds_offsets = engine_state.pipettes.get_pipette_bounding_box(pipette_id) + from_back_right = ( + robot_extent_per_mount.back_right[mount] + + pipette_bounds_offsets.back_right_corner + ) + from_front_left = ( + robot_extent_per_mount.front_left[mount] + + pipette_bounds_offsets.front_left_corner + ) + return ( + from_back_right.x >= pip_back_left_bound.x >= from_front_left.x + and from_back_right.y >= pip_back_left_bound.y >= from_front_left.y + and from_back_right.x >= pip_front_right_bound.x >= from_front_left.x + and from_back_right.y >= pip_front_right_bound.y >= from_front_left.y + ) def _map_labware( diff --git a/api/src/opentrons/protocol_engine/create_protocol_engine.py b/api/src/opentrons/protocol_engine/create_protocol_engine.py index fd7b1b8bd5f..8a6a4355fd7 100644 --- a/api/src/opentrons/protocol_engine/create_protocol_engine.py +++ b/api/src/opentrons/protocol_engine/create_protocol_engine.py @@ -7,6 +7,7 @@ from opentrons.hardware_control.types import DoorState from opentrons.protocol_engine.error_recovery_policy import ErrorRecoveryPolicy from opentrons.util.async_helpers import async_context_manager_in_thread +from opentrons_shared_data.robot import load as load_robot from .protocol_engine import ProtocolEngine from .resources import DeckDataProvider, ModuleDataProvider @@ -45,11 +46,12 @@ async def create_protocol_engine( else [] ) module_calibration_offsets = ModuleDataProvider.load_module_calibrations() - + robot_definition = load_robot(config.robot_type) state_store = StateStore( config=config, deck_definition=deck_definition, deck_fixed_labware=deck_fixed_labware, + robot_definition=robot_definition, is_door_open=hardware_api.door_state is DoorState.OPEN, module_calibration_offsets=module_calibration_offsets, deck_configuration=deck_configuration, diff --git a/api/src/opentrons/protocol_engine/state/addressable_areas.py b/api/src/opentrons/protocol_engine/state/addressable_areas.py index 85c61bfa917..7e3a0325ed4 100644 --- a/api/src/opentrons/protocol_engine/state/addressable_areas.py +++ b/api/src/opentrons/protocol_engine/state/addressable_areas.py @@ -1,8 +1,9 @@ """Basic addressable area data state and store.""" from dataclasses import dataclass +from functools import cached_property from typing import Dict, List, Optional, Set, Union -from opentrons_shared_data.robot.dev_types import RobotType +from opentrons_shared_data.robot.dev_types import RobotType, RobotDefinition from opentrons_shared_data.deck.dev_types import ( DeckDefinitionV5, SlotDefV3, @@ -77,6 +78,9 @@ class AddressableAreaState: use_simulated_deck_config: bool """See `Config.use_simulated_deck_config`.""" + """Information about the current robot model.""" + robot_definition: RobotDefinition + _OT2_ORDERED_SLOTS = ["1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "12"] _FLEX_ORDERED_SLOTS = [ @@ -164,6 +168,7 @@ def __init__( deck_configuration: DeckConfigurationType, config: Config, deck_definition: DeckDefinitionV5, + robot_definition: RobotDefinition, ) -> None: """Initialize an addressable area store and its state.""" if config.use_simulated_deck_config: @@ -183,6 +188,7 @@ def __init__( deck_definition=deck_definition, robot_type=config.robot_type, use_simulated_deck_config=config.use_simulated_deck_config, + robot_definition=robot_definition, ) def handle_action(self, action: Action) -> None: @@ -330,6 +336,22 @@ def __init__(self, state: AddressableAreaState) -> None: """ self._state = state + @cached_property + def deck_extents(self) -> Point: + """The maximum space on the deck.""" + extents = self._state.robot_definition["extents"] + return Point(x=extents[0], y=extents[1], z=extents[2]) + + @cached_property + def mount_offsets(self) -> Dict[str, Point]: + """The left and right mount offsets of the robot.""" + left_offset = self.state.robot_definition["mountOffsets"]["left"] + right_offset = self.state.robot_definition["mountOffsets"]["right"] + return { + "left": Point(x=left_offset[0], y=left_offset[1], z=left_offset[2]), + "right": Point(x=right_offset[0], y=right_offset[1], z=right_offset[2]), + } + def get_addressable_area(self, addressable_area_name: str) -> AddressableArea: """Get addressable area.""" if not self._state.use_simulated_deck_config: diff --git a/api/src/opentrons/protocol_engine/state/geometry.py b/api/src/opentrons/protocol_engine/state/geometry.py index 112d7d60ef4..904e0c470b2 100644 --- a/api/src/opentrons/protocol_engine/state/geometry.py +++ b/api/src/opentrons/protocol_engine/state/geometry.py @@ -3,6 +3,8 @@ from numpy import array, dot, double as npdouble from numpy.typing import NDArray from typing import Optional, List, Tuple, Union, cast, TypeVar, Dict +from dataclasses import dataclass +from functools import cached_property from opentrons.types import Point, DeckSlotName, StagingSlotName, MountType @@ -71,6 +73,12 @@ class _GripperMoveType(enum.Enum): DROP_LABWARE = enum.auto() +@dataclass +class _AbsoluteRobotExtents: + front_left: Dict[MountType, Point] + back_right: Dict[MountType, Point] + + _LabwareLocation = TypeVar("_LabwareLocation", bound=LabwareLocation) @@ -95,6 +103,24 @@ def __init__( self._addressable_areas = addressable_area_view self._last_drop_tip_location_spot: Dict[str, _TipDropSection] = {} + @cached_property + def absolute_deck_extents(self) -> _AbsoluteRobotExtents: + """The absolute deck extents for a given robot deck.""" + left_offset = self._addressable_areas.mount_offsets["left"] + right_offset = self._addressable_areas.mount_offsets["right"] + + front_left_abs = { + MountType.LEFT: Point(left_offset.x, -1 * left_offset.y, left_offset.z), + MountType.RIGHT: Point(right_offset.x, -1 * right_offset.y, right_offset.z), + } + back_right_abs = { + MountType.LEFT: self._addressable_areas.deck_extents + left_offset, + MountType.RIGHT: self._addressable_areas.deck_extents + right_offset, + } + return _AbsoluteRobotExtents( + front_left=front_left_abs, back_right=back_right_abs + ) + def get_labware_highest_z(self, labware_id: str) -> float: """Get the highest Z-point of a labware.""" labware_data = self._labware.get(labware_id) diff --git a/api/src/opentrons/protocol_engine/state/pipettes.py b/api/src/opentrons/protocol_engine/state/pipettes.py index cab42ac7238..92344dd9600 100644 --- a/api/src/opentrons/protocol_engine/state/pipettes.py +++ b/api/src/opentrons/protocol_engine/state/pipettes.py @@ -97,6 +97,8 @@ class PipetteBoundingBoxOffsets: back_left_corner: Point front_right_corner: Point + back_right_corner: Point + front_left_corner: Point @dataclass(frozen=True) @@ -194,6 +196,16 @@ def _handle_command( # noqa: C901 pipette_bounding_box_offsets=PipetteBoundingBoxOffsets( back_left_corner=config.back_left_corner_offset, front_right_corner=config.front_right_corner_offset, + back_right_corner=Point( + config.front_right_corner_offset.x, + config.back_left_corner_offset.y, + config.back_left_corner_offset.z, + ), + front_left_corner=Point( + config.back_left_corner_offset.x, + config.front_right_corner_offset.y, + config.back_left_corner_offset.z, + ), ), bounding_nozzle_offsets=BoundingNozzlesOffsets( back_left_offset=config.nozzle_map.back_left_nozzle_offset, @@ -788,6 +800,10 @@ def get_pipette_bounding_nozzle_offsets( """Get the nozzle offsets of the pipette's bounding nozzles.""" return self.get_config(pipette_id).bounding_nozzle_offsets + def get_pipette_bounding_box(self, pipette_id: str) -> PipetteBoundingBoxOffsets: + """Get the bounding box of the pipette.""" + return self.get_config(pipette_id).pipette_bounding_box_offsets + def get_pipette_bounds_at_specified_move_to_position( self, pipette_id: str, @@ -796,6 +812,7 @@ def get_pipette_bounds_at_specified_move_to_position( """Get the pipette's bounding offsets when primary nozzle is at the given position.""" primary_nozzle_offset = self.get_primary_nozzle_offset(pipette_id) tip = self.get_attached_tip(pipette_id) + # TODO update this for pipette robot stackup # Primary nozzle position at destination, in deck coordinates primary_nozzle_position = destination_position + Point( x=0, y=0, z=tip.length if tip else 0 diff --git a/api/src/opentrons/protocol_engine/state/state.py b/api/src/opentrons/protocol_engine/state/state.py index aa54383b379..e343a4dfde1 100644 --- a/api/src/opentrons/protocol_engine/state/state.py +++ b/api/src/opentrons/protocol_engine/state/state.py @@ -6,6 +6,7 @@ from typing_extensions import ParamSpec from opentrons_shared_data.deck.dev_types import DeckDefinitionV5 +from opentrons_shared_data.robot.dev_types import RobotDefinition from opentrons.protocol_engine.types import ModuleOffsetData from opentrons.util.change_notifier import ChangeNotifier @@ -144,6 +145,7 @@ def __init__( config: Config, deck_definition: DeckDefinitionV5, deck_fixed_labware: Sequence[DeckFixedLabware], + robot_definition: RobotDefinition, is_door_open: bool, change_notifier: Optional[ChangeNotifier] = None, module_calibration_offsets: Optional[Dict[str, ModuleOffsetData]] = None, @@ -162,6 +164,7 @@ def __init__( change_notifier: Internal state change notifier. module_calibration_offsets: Module offsets to preload. deck_configuration: The initial deck configuration the addressable area store will be instantiated with. + robot_definition: Static information about the robot type being used. notify_publishers: Notifies robot server publishers of internal state change. """ self._command_store = CommandStore(config=config, is_door_open=is_door_open) @@ -172,6 +175,7 @@ def __init__( deck_configuration=deck_configuration, config=config, deck_definition=deck_definition, + robot_definition=robot_definition, ) self._labware_store = LabwareStore( deck_fixed_labware=deck_fixed_labware, diff --git a/api/tests/opentrons/protocol_api/core/engine/test_deck_conflict.py b/api/tests/opentrons/protocol_api/core/engine/test_deck_conflict.py index 82ce80695d3..c50ffe4687e 100644 --- a/api/tests/opentrons/protocol_api/core/engine/test_deck_conflict.py +++ b/api/tests/opentrons/protocol_api/core/engine/test_deck_conflict.py @@ -25,9 +25,12 @@ ModuleModel, StateView, ) +from opentrons.protocol_engine.state.geometry import _AbsoluteRobotExtents +from opentrons.protocol_engine.state.pipettes import PipetteBoundingBoxOffsets + from opentrons.protocol_engine.clients import SyncClient from opentrons.protocol_engine.errors import LabwareNotLoadedOnModuleError -from opentrons.types import DeckSlotName, Point, StagingSlotName +from opentrons.types import DeckSlotName, Point, StagingSlotName, MountType from opentrons.protocol_engine.types import ( DeckType, @@ -416,7 +419,7 @@ def test_maps_trash_bins( [("OT-3 Standard", DeckType.OT3_STANDARD)], ) @pytest.mark.parametrize( - ["pipette_bounds", "expected_raise"], + ["pipette_bounds", "expected_raise", "y_value"], [ ( # nozzles above highest Z ( @@ -426,6 +429,7 @@ def test_maps_trash_bins( Point(x=50, y=50, z=60), ), does_not_raise(), + 0, ), # X, Y, Z collisions ( @@ -439,6 +443,7 @@ def test_maps_trash_bins( deck_conflict.PartialTipMovementNotAllowedError, match="collision with items in deck slot D1", ), + 0, ), ( ( @@ -451,6 +456,7 @@ def test_maps_trash_bins( deck_conflict.PartialTipMovementNotAllowedError, match="collision with items in deck slot D2", ), + 0, ), ( # Collision with staging slot ( @@ -461,8 +467,9 @@ def test_maps_trash_bins( ), pytest.raises( deck_conflict.PartialTipMovementNotAllowedError, - match="collision with items in staging slot C4", + match="will result in collision with items in staging slot C4.", ), + 170, ), ], ) @@ -471,6 +478,7 @@ def test_deck_conflict_raises_for_bad_pipette_move( mock_state_view: StateView, pipette_bounds: Tuple[Point, Point, Point, Point], expected_raise: ContextManager[Any], + y_value: float, ) -> None: """It should raise errors when moving to locations with restrictions for partial pipette movement. @@ -485,7 +493,36 @@ def test_deck_conflict_raises_for_bad_pipette_move( in order to preserve readability of the test. That means the test does actual slot overlap checks. """ - destination_well_point = Point(x=123, y=123, z=123) + destination_well_point = Point(x=123, y=y_value, z=123) + decoy.when( + mock_state_view.pipettes.get_is_partially_configured("pipette-id") + ).then_return(True) + decoy.when(mock_state_view.pipettes.get_mount("pipette-id")).then_return( + MountType.LEFT + ) + decoy.when(mock_state_view.geometry.absolute_deck_extents).then_return( + _AbsoluteRobotExtents( + front_left={ + MountType.LEFT: Point(13.5, -60.5, 0.0), + MountType.RIGHT: Point(-40.5, -60.5, 0.0), + }, + back_right={ + MountType.LEFT: Point(463.7, 433.3, 0.0), + MountType.RIGHT: Point(517.7, 433.3), + }, + ) + ) + decoy.when( + mock_state_view.pipettes.get_pipette_bounding_box("pipette-id") + ).then_return( + # 96 chan outer bounds + PipetteBoundingBoxOffsets( + back_left_corner=Point(-36.0, -25.5, -259.15), + front_right_corner=Point(63.0, -88.5, -259.15), + front_left_corner=Point(-36.0, -88.5, -259.15), + back_right_corner=Point(63.0, -25.5, -259.15), + ) + ) decoy.when( mock_state_view.pipettes.get_is_partially_configured("pipette-id") ).then_return(True) @@ -589,7 +626,7 @@ def test_deck_conflict_raises_for_collision_with_tc_lid( destination_well_point = Point(x=123, y=123, z=123) pipette_bounds_at_destination = ( Point(x=50, y=350, z=204.5), - Point(x=150, y=450, z=204.5), + Point(x=150, y=429, z=204.5), Point(x=150, y=400, z=204.5), Point(x=50, y=300, z=204.5), ) @@ -616,6 +653,32 @@ def test_deck_conflict_raises_for_collision_with_tc_lid( pipette_id="pipette-id", destination_position=destination_well_point ) ).then_return(pipette_bounds_at_destination) + decoy.when(mock_state_view.pipettes.get_mount("pipette-id")).then_return( + MountType.LEFT + ) + decoy.when( + mock_state_view.pipettes.get_pipette_bounding_box("pipette-id") + ).then_return( + # 96 chan outer bounds + PipetteBoundingBoxOffsets( + back_left_corner=Point(-67.0, -3.5, -259.15), + front_right_corner=Point(94.0, -113.0, -259.15), + front_left_corner=Point(-67.0, -113.0, -259.15), + back_right_corner=Point(94.0, -3.5, -259.15), + ) + ) + decoy.when(mock_state_view.geometry.absolute_deck_extents).then_return( + _AbsoluteRobotExtents( + front_left={ + MountType.LEFT: Point(13.5, 60.5, 0.0), + MountType.RIGHT: Point(-40.5, 60.5, 0.0), + }, + back_right={ + MountType.LEFT: Point(463.7, 433.3, 0.0), + MountType.RIGHT: Point(517.7, 433.3), + }, + ) + ) decoy.when( adjacent_slots_getters.get_surrounding_slots(5, robot_type="OT-3 Standard") diff --git a/api/tests/opentrons/protocol_api_integration/test_pipette_movement_deck_conflicts.py b/api/tests/opentrons/protocol_api_integration/test_pipette_movement_deck_conflicts.py index 33e92086edb..4984cd4fa3d 100644 --- a/api/tests/opentrons/protocol_api_integration/test_pipette_movement_deck_conflicts.py +++ b/api/tests/opentrons/protocol_api_integration/test_pipette_movement_deck_conflicts.py @@ -3,7 +3,7 @@ import pytest from opentrons import simulate -from opentrons.protocol_api import COLUMN, ALL +from opentrons.protocol_api import COLUMN, ALL, SINGLE from opentrons.protocol_api.core.engine.deck_conflict import ( PartialTipMovementNotAllowedError, ) @@ -61,8 +61,14 @@ def test_deck_conflicts_for_96_ch_a12_column_configuration() -> None: ): instrument.pick_up_tip(badly_placed_tiprack.wells_by_name()["A1"]) - # No error since no tall item in west slot of destination slot - instrument.pick_up_tip(well_placed_tiprack.wells_by_name()["A1"]) + with pytest.raises( + PartialTipMovementNotAllowedError, match="outside of robot bounds" + ): + # Picking up from A1 in an east-most slot using a configuration with column 12 would + # result in a collision with the side of the robot. + instrument.pick_up_tip(well_placed_tiprack.wells_by_name()["A1"]) + + instrument.pick_up_tip(well_placed_tiprack.wells_by_name()["A12"]) instrument.aspirate(50, well_placed_labware.wells_by_name()["A4"]) with pytest.raises( @@ -75,14 +81,19 @@ def test_deck_conflicts_for_96_ch_a12_column_configuration() -> None: ): instrument.dispense(10, tc_adjacent_plate.wells_by_name()["A1"]) + instrument.dispense(10, tc_adjacent_plate.wells_by_name()["H2"]) + # No error cuz dispensing from high above plate, so it clears tuberack in west slot instrument.dispense(15, badly_placed_labware.wells_by_name()["A1"].top(150)) thermocycler.open_lid() # type: ignore[union-attr] - # Will NOT raise error since first column of TC labware is accessible - # (it is just a few mm away from the left bound) - instrument.dispense(25, accessible_plate.wells_by_name()["A1"]) + with pytest.raises( + PartialTipMovementNotAllowedError, match="outside of robot bounds" + ): + # Dispensing to A1 in an east-most slot using a configuration with column 12 would + # result in a collision with the side of the robot. + instrument.dispense(25, accessible_plate.wells_by_name()["A1"]) instrument.drop_tip() @@ -102,7 +113,7 @@ def test_deck_conflicts_for_96_ch_a12_column_configuration() -> None: @pytest.mark.ot3_only def test_close_shave_deck_conflicts_for_96_ch_a12_column_configuration() -> None: """Shouldn't raise errors for "almost collision"s.""" - protocol_context = simulate.get_protocol_api(version="2.16", robot_type="Flex") + protocol_context = simulate.get_protocol_api(version="2.20", robot_type="Flex") res12 = protocol_context.load_labware("nest_12_reservoir_15ml", "C3") # Mag block and tiprack adapter are very close to the destination reservoir labware @@ -118,13 +129,14 @@ def test_close_shave_deck_conflicts_for_96_ch_a12_column_configuration() -> None deepwell = hs_adapter.load_labware("nest_96_wellplate_2ml_deep") protocol_context.load_trash_bin("A3") p1000_96 = protocol_context.load_instrument("flex_96channel_1000") - p1000_96.configure_nozzle_layout(style=COLUMN, start="A12", tip_racks=[tiprack_8]) + p1000_96.configure_nozzle_layout(style=SINGLE, start="A12", tip_racks=[tiprack_8]) hs.close_labware_latch() # type: ignore[union-attr] + # Note p1000_96.distribute( 15, - res12.wells()[0], - deepwell.rows()[0], + res12["A6"], + deepwell.columns()[6], disposal_vol=0, ) @@ -180,8 +192,15 @@ def test_deck_conflicts_for_96_ch_a1_column_configuration() -> None: with pytest.raises( PartialTipMovementNotAllowedError, match="outside of robot bounds" ): + # Moving the 96 channel in column configuration with column 1 + # is incompatible with moving to a plate in B3 in the right most + # column. instrument.aspirate(25, well_placed_plate.wells_by_name()["A11"]) + # No error because we're moving to column 1 of the plate with + # column 1 of the 96 channel. + instrument.aspirate(25, well_placed_plate.wells_by_name()["A1"]) + # No error cuz no taller labware on the right instrument.aspirate(10, my_tuberack.wells_by_name()["A1"]) diff --git a/api/tests/opentrons/protocol_engine/state/test_addressable_area_state.py b/api/tests/opentrons/protocol_engine/state/test_addressable_area_state.py index 7209e78bb90..66fa692fe25 100644 --- a/api/tests/opentrons/protocol_engine/state/test_addressable_area_state.py +++ b/api/tests/opentrons/protocol_engine/state/test_addressable_area_state.py @@ -28,6 +28,17 @@ def test_deck_configuration_setting( deck_type=DeckType.OT3_STANDARD, ), deck_definition=ot3_standard_deck_def, + robot_definition={ + "displayName": "OT-3", + "robotType": "OT-3 Standard", + "models": ["OT-3 Standard"], + "extents": [477.2, 493.8, 0.0], + "mountOffsets": { + "left": [-13.5, -60.5, 255.675], + "right": [40.5, -60.5, 255.675], + "gripper": [84.55, -12.75, 93.85], + }, + }, ) subject_view = AddressableAreaView(subject.state) diff --git a/api/tests/opentrons/protocol_engine/state/test_addressable_area_store.py b/api/tests/opentrons/protocol_engine/state/test_addressable_area_store.py index c3d52028647..fcadb43940e 100644 --- a/api/tests/opentrons/protocol_engine/state/test_addressable_area_store.py +++ b/api/tests/opentrons/protocol_engine/state/test_addressable_area_store.py @@ -69,6 +69,17 @@ def simulated_subject( deck_type=DeckType.OT3_STANDARD, ), deck_definition=ot3_standard_deck_def, + robot_definition={ + "displayName": "OT-3", + "robotType": "OT-3 Standard", + "models": ["OT-3 Standard"], + "extents": [477.2, 493.8, 0.0], + "mountOffsets": { + "left": [-13.5, -60.5, 255.675], + "right": [40.5, -60.5, 255.675], + "gripper": [84.55, -12.75, 93.85], + }, + }, ) @@ -85,6 +96,17 @@ def subject( deck_type=DeckType.OT3_STANDARD, ), deck_definition=ot3_standard_deck_def, + robot_definition={ + "displayName": "OT-3", + "robotType": "OT-3 Standard", + "models": ["OT-3 Standard"], + "extents": [477.2, 493.8, 0.0], + "mountOffsets": { + "left": [-13.5, -60.5, 255.675], + "right": [40.5, -60.5, 255.675], + "gripper": [84.55, -12.75, 93.85], + }, + }, ) @@ -100,6 +122,17 @@ def test_initial_state_simulated( deck_configuration=[], robot_type="OT-3 Standard", use_simulated_deck_config=True, + robot_definition={ + "displayName": "OT-3", + "robotType": "OT-3 Standard", + "models": ["OT-3 Standard"], + "extents": [477.2, 493.8, 0.0], + "mountOffsets": { + "left": [-13.5, -60.5, 255.675], + "right": [40.5, -60.5, 255.675], + "gripper": [84.55, -12.75, 93.85], + }, + }, ) diff --git a/api/tests/opentrons/protocol_engine/state/test_addressable_area_view.py b/api/tests/opentrons/protocol_engine/state/test_addressable_area_view.py index 30ebe0d0341..3d1cbe9be1a 100644 --- a/api/tests/opentrons/protocol_engine/state/test_addressable_area_view.py +++ b/api/tests/opentrons/protocol_engine/state/test_addressable_area_view.py @@ -64,6 +64,17 @@ def get_addressable_area_view( potential_cutout_fixtures_by_cutout_id=potential_cutout_fixtures_by_cutout_id or {}, deck_definition=deck_definition or cast(DeckDefinitionV5, {"otId": "fake"}), + robot_definition={ + "displayName": "OT-3", + "robotType": "OT-3 Standard", + "models": ["OT-3 Standard"], + "extents": [477.2, 493.8, 0.0], + "mountOffsets": { + "left": [-13.5, -60.5, 255.675], + "right": [40.5, -60.5, 255.675], + "gripper": [84.55, -12.75, 93.85], + }, + }, deck_configuration=deck_configuration or [], robot_type=robot_type, use_simulated_deck_config=use_simulated_deck_config, diff --git a/api/tests/opentrons/protocol_engine/state/test_geometry_view.py b/api/tests/opentrons/protocol_engine/state/test_geometry_view.py index 58a4a49940e..9887a4ef76c 100644 --- a/api/tests/opentrons/protocol_engine/state/test_geometry_view.py +++ b/api/tests/opentrons/protocol_engine/state/test_geometry_view.py @@ -174,7 +174,20 @@ def addressable_area_store( ) -> AddressableAreaStore: """Get an addressable area store that can accept actions.""" return AddressableAreaStore( - deck_configuration=[], config=state_config, deck_definition=deck_definition + deck_configuration=[], + config=state_config, + deck_definition=deck_definition, + robot_definition={ + "displayName": "OT-3", + "robotType": "OT-3 Standard", + "models": ["OT-3 Standard"], + "extents": [477.2, 493.8, 0.0], + "mountOffsets": { + "left": [-13.5, -60.5, 255.675], + "right": [40.5, -60.5, 255.675], + "gripper": [84.55, -12.75, 93.85], + }, + }, ) @@ -2077,6 +2090,8 @@ def test_get_next_drop_tip_location( pipette_bounding_box_offsets=PipetteBoundingBoxOffsets( back_left_corner=Point(x=10, y=20, z=30), front_right_corner=Point(x=40, y=50, z=60), + front_left_corner=Point(x=10, y=50, z=60), + back_right_corner=Point(x=40, y=20, z=60), ), lld_settings={}, ) diff --git a/api/tests/opentrons/protocol_engine/state/test_module_store.py b/api/tests/opentrons/protocol_engine/state/test_module_store.py index e6de0a96ac0..0dabf508483 100644 --- a/api/tests/opentrons/protocol_engine/state/test_module_store.py +++ b/api/tests/opentrons/protocol_engine/state/test_module_store.py @@ -74,6 +74,17 @@ def get_addressable_area_view( or {}, deck_definition=deck_definition or cast(DeckDefinitionV5, {"otId": "fake"}), deck_configuration=deck_configuration or [], + robot_definition={ + "displayName": "OT-3", + "robotType": "OT-3 Standard", + "models": ["OT-3 Standard"], + "extents": [477.2, 493.8, 0.0], + "mountOffsets": { + "left": [-13.5, -60.5, 255.675], + "right": [40.5, -60.5, 255.675], + "gripper": [84.55, -12.75, 93.85], + }, + }, robot_type=robot_type, use_simulated_deck_config=use_simulated_deck_config, ) diff --git a/api/tests/opentrons/protocol_engine/state/test_module_view.py b/api/tests/opentrons/protocol_engine/state/test_module_view.py index b840673f2e8..e308c09407d 100644 --- a/api/tests/opentrons/protocol_engine/state/test_module_view.py +++ b/api/tests/opentrons/protocol_engine/state/test_module_view.py @@ -87,6 +87,17 @@ def get_addressable_area_view( or {}, deck_definition=deck_definition or cast(DeckDefinitionV5, {"otId": "fake"}), deck_configuration=deck_configuration or [], + robot_definition={ + "displayName": "OT-3", + "robotType": "OT-3 Standard", + "models": ["OT-3 Standard"], + "extents": [477.2, 493.8, 0.0], + "mountOffsets": { + "left": [-13.5, -60.5, 255.675], + "right": [40.5, -60.5, 255.675], + "gripper": [84.55, -12.75, 93.85], + }, + }, robot_type=robot_type, use_simulated_deck_config=use_simulated_deck_config, ) diff --git a/api/tests/opentrons/protocol_engine/state/test_pipette_store.py b/api/tests/opentrons/protocol_engine/state/test_pipette_store.py index a99ac90e9e2..8ccfc06fd07 100644 --- a/api/tests/opentrons/protocol_engine/state/test_pipette_store.py +++ b/api/tests/opentrons/protocol_engine/state/test_pipette_store.py @@ -775,6 +775,8 @@ def test_add_pipette_config( pipette_bounding_box_offsets=PipetteBoundingBoxOffsets( back_left_corner=Point(x=1, y=2, z=3), front_right_corner=Point(x=4, y=5, z=6), + front_left_corner=Point(x=1, y=5, z=3), + back_right_corner=Point(x=4, y=2, z=3), ), lld_settings={}, ) diff --git a/api/tests/opentrons/protocol_engine/state/test_pipette_view.py b/api/tests/opentrons/protocol_engine/state/test_pipette_view.py index e15c8401699..1942a9a04e1 100644 --- a/api/tests/opentrons/protocol_engine/state/test_pipette_view.py +++ b/api/tests/opentrons/protocol_engine/state/test_pipette_view.py @@ -46,7 +46,10 @@ back_left_offset=Point(x=10, y=20, z=30), front_right_offset=Point(x=40, y=50, z=60) ) _SAMPLE_PIPETTE_BOUNDING_BOX_OFFSETS = PipetteBoundingBoxOffsets( - back_left_corner=Point(x=10, y=20, z=30), front_right_corner=Point(x=40, y=50, z=60) + back_left_corner=Point(x=10, y=20, z=30), + front_right_corner=Point(x=40, y=50, z=60), + front_left_corner=Point(x=10, y=50, z=60), + back_right_corner=Point(x=40, y=20, z=60), ) @@ -594,6 +597,8 @@ class _PipetteSpecs(NamedTuple): bounding_box_offsets=PipetteBoundingBoxOffsets( back_left_corner=Point(0.0, 31.5, 35.52), front_right_corner=Point(0.0, -31.5, 35.52), + front_left_corner=Point(0.0, -31.5, 35.52), + back_right_corner=Point(0.0, 31.5, 35.52), ), nozzle_map=NozzleMap.build( physical_nozzles=EIGHT_CHANNEL_MAP, @@ -620,6 +625,8 @@ class _PipetteSpecs(NamedTuple): bounding_box_offsets=PipetteBoundingBoxOffsets( back_left_corner=Point(0.0, 31.5, 35.52), front_right_corner=Point(0.0, -31.5, 35.52), + front_left_corner=Point(0.0, -31.5, 35.52), + back_right_corner=Point(0.0, 31.5, 35.52), ), nozzle_map=NozzleMap.build( physical_nozzles=EIGHT_CHANNEL_MAP, @@ -646,6 +653,8 @@ class _PipetteSpecs(NamedTuple): bounding_box_offsets=PipetteBoundingBoxOffsets( back_left_corner=Point(-36.0, -25.5, -259.15), front_right_corner=Point(63.0, -88.5, -259.15), + front_left_corner=Point(-36.0, -88.5, -259.15), + back_right_corner=Point(63.0, -25.5, -259.15), ), nozzle_map=NozzleMap.build( physical_nozzles=NINETY_SIX_MAP, @@ -688,6 +697,8 @@ class _PipetteSpecs(NamedTuple): bounding_box_offsets=PipetteBoundingBoxOffsets( back_left_corner=Point(-36.0, -25.5, -259.15), front_right_corner=Point(63.0, -88.5, -259.15), + front_left_corner=Point(-36.0, -88.5, -259.15), + back_right_corner=Point(63.0, -25.5, -259.15), ), nozzle_map=NozzleMap.build( physical_nozzles=NINETY_SIX_MAP, @@ -712,6 +723,8 @@ class _PipetteSpecs(NamedTuple): bounding_box_offsets=PipetteBoundingBoxOffsets( back_left_corner=Point(-36.0, -25.5, -259.15), front_right_corner=Point(63.0, -88.5, -259.15), + front_left_corner=Point(-36.0, -88.5, -259.15), + back_right_corner=Point(63.0, -25.5, -259.15), ), nozzle_map=NozzleMap.build( physical_nozzles=NINETY_SIX_MAP, @@ -736,6 +749,8 @@ class _PipetteSpecs(NamedTuple): bounding_box_offsets=PipetteBoundingBoxOffsets( back_left_corner=Point(-36.0, -25.5, -259.15), front_right_corner=Point(63.0, -88.5, -259.15), + front_left_corner=Point(-36.0, -88.5, -259.15), + back_right_corner=Point(63.0, -25.5, -259.15), ), nozzle_map=NozzleMap.build( physical_nozzles=NINETY_SIX_MAP, diff --git a/api/tests/opentrons/protocol_engine/state/test_state_store.py b/api/tests/opentrons/protocol_engine/state/test_state_store.py index d69784c6834..26f50515317 100644 --- a/api/tests/opentrons/protocol_engine/state/test_state_store.py +++ b/api/tests/opentrons/protocol_engine/state/test_state_store.py @@ -39,6 +39,13 @@ def subject( return StateStore( config=engine_config, deck_definition=ot2_standard_deck_def, + robot_definition={ + "displayName": "OT-2", + "robotType": "OT-2 Standard", + "models": ["OT-2 Standard", "OT-2 Refresh"], + "extents": [446.75, 347.5, 0.0], + "mountOffsets": {"left": [-34.0, 0.0, 0.0], "right": [0.0, 0.0, 0.0]}, + }, deck_fixed_labware=[], change_notifier=change_notifier, is_door_open=False, diff --git a/shared-data/deck/definitions/4/ot2_standard.json b/shared-data/deck/definitions/4/ot2_standard.json index 344469c65d3..e576a393e2c 100644 --- a/shared-data/deck/definitions/4/ot2_standard.json +++ b/shared-data/deck/definitions/4/ot2_standard.json @@ -8,7 +8,12 @@ "tags": ["ot2", "12 slots", "standard"] }, "robot": { - "model": "OT-2 Standard" + "model": "OT-2 Standard", + "extents": [446.75, 347.5, 0.0], + "mountOffsets": { + "left": [-34, 0.0, 0.0], + "right": [0.0, 0.0, 0.0] + } }, "locations": { "addressableAreas": [ diff --git a/shared-data/python/opentrons_shared_data/robot/dev_types.py b/shared-data/python/opentrons_shared_data/robot/dev_types.py index 555ec6008ba..90f0f19c1c4 100644 --- a/shared-data/python/opentrons_shared_data/robot/dev_types.py +++ b/shared-data/python/opentrons_shared_data/robot/dev_types.py @@ -1,7 +1,7 @@ """opentrons_shared_data.robot.dev_types: types for robot def.""" import enum from typing import NewType, List, Dict, Any -from typing_extensions import Literal, TypedDict +from typing_extensions import Literal, TypedDict, NotRequired RobotSchemaVersion1 = Literal[1] @@ -29,9 +29,19 @@ def robot_literal_to_enum(cls, robot_type: RobotType) -> "RobotTypeEnum": # No final `else` statement, depend on mypy exhaustiveness checking +class mountOffset(TypedDict): + """The mount offsets for a given robot type based off the center of the carriage..""" + + left: List[float] + right: List[float] + gripper: NotRequired[List[float]] + + class RobotDefinition(TypedDict): """A python version of the robot definition type.""" displayName: str robotType: RobotType models: List[str] + extents: List[float] + mountOffsets: mountOffset diff --git a/shared-data/robot/definitions/1/ot2.json b/shared-data/robot/definitions/1/ot2.json index 9f6a48be16c..50c6eb4256a 100644 --- a/shared-data/robot/definitions/1/ot2.json +++ b/shared-data/robot/definitions/1/ot2.json @@ -1,5 +1,10 @@ { "displayName": "OT-2", "robotType": "OT-2 Standard", - "models": ["OT-2 Standard", "OT-2 Refresh"] + "models": ["OT-2 Standard", "OT-2 Refresh"], + "extents": [446.75, 347.5, 0.0], + "mountOffsets": { + "left": [-34.0, 0.0, 0.0], + "right": [0.0, 0.0, 0.0] + } } diff --git a/shared-data/robot/definitions/1/ot3.json b/shared-data/robot/definitions/1/ot3.json index 3916570adee..eb3a943d886 100644 --- a/shared-data/robot/definitions/1/ot3.json +++ b/shared-data/robot/definitions/1/ot3.json @@ -1,5 +1,11 @@ { "displayName": "OT-3", "robotType": "OT-3 Standard", - "models": ["OT-3 Standard"] + "models": ["OT-3 Standard"], + "extents": [477.2, 493.8, 0.0], + "mountOffsets": { + "left": [-13.5, -60.5, 255.675], + "right": [40.5, -60.5, 255.675], + "gripper": [84.55, -12.75, 93.85] + } } diff --git a/shared-data/robot/schemas/1.json b/shared-data/robot/schemas/1.json index 5f409032451..44e25e6caf5 100644 --- a/shared-data/robot/schemas/1.json +++ b/shared-data/robot/schemas/1.json @@ -5,11 +5,18 @@ "robotType": { "type": "string", "enum": ["OT-2 Standard", "OT-3 Standard"] + }, + "xyzArray": { + "type": "array", + "description": "Array of 3 numbers, [x, y, z]", + "items": { "type": "number" }, + "minItems": 3, + "maxItems": 3 } }, "description": "Describes an Opentrons liquid handling robot.", "type": "object", - "required": ["displayName", "robotType", "models"], + "required": ["displayName", "robotType", "models", "extents", "mountOffsets"], "properties": { "displayName": { "description": "A user-facing friendly name for the machine.", @@ -25,6 +32,29 @@ "items": { "type": "string" } + }, + "extents": { + "description": "The maximum addressable coordinates of the deck without instruments.", + "$ref": "#/definitions/xyzArray" + }, + "mountOffsets": { + "description": "The physical mount offsets from the center of the instrument carriage.", + "type": "object", + "required": ["left", "right"], + "properties": { + "left": { + "description": "The left mount offset from the center of the carriage to the center of the left mount", + "$ref": "#/definitions/xyzArray" + }, + "right": { + "description": "The right mount offset from the center of the carriage to the center of the right mount", + "$ref": "#/definitions/xyzArray" + }, + "gripper": { + "description": "The gripper mount offset from the center of the carriage to the center of the gripper, only on OT-3 Standard definitions", + "$ref": "#/definitions/xyzArray" + } + } } } } From fa230713d98c6e2e367844c125a1940e8c16cb33 Mon Sep 17 00:00:00 2001 From: Shlok Amin Date: Thu, 18 Jul 2024 10:58:59 -0400 Subject: [PATCH 3/6] feat(app-shell): update user license agreement for desktop app installers (#15631) --- app-shell/build/license_en.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app-shell/build/license_en.txt b/app-shell/build/license_en.txt index f16605697b0..cf847badf81 100644 --- a/app-shell/build/license_en.txt +++ b/app-shell/build/license_en.txt @@ -1,6 +1,6 @@ Opentrons End-User License Agreement -Last updated: June 27, 2024 +Last updated: July 10, 2024 THIS END-USER LICENSE AGREEMENT (“EULA”) is a legal agreement between you (“User”), either as an individual or on behalf of an entity, and Opentrons Labworks Inc. (“Opentrons”) regarding your use of Opentrons robots, modules, software, and associated documentation (“Opentrons Products”) including, but not limited to, the Opentrons OT-2 robot and associated modules, the Opentrons Flex robot and associated modules, the Opentrons App, the Opentrons API, the Opentrons Protocol Designer and Protocol Library, the Opentrons Labware Library, and the Opentrons Website. By installing or using the Opentrons Products, you agree to be bound by the terms and conditions of this EULA. If you do not agree to the terms of this EULA, you must immediately cease use of the Opentrons Products. @@ -9,7 +9,7 @@ Use of Opentrons Products. Permitted Use. User shall use the Opentrons Products strictly in accordance with the terms of the EULA and Related Agreements. User shall use Opentrons Product software only in conjunction with Opentrons Product hardware. Restrictions on Use. Unless otherwise specified in a separate agreement entered into between Opentrons and User, User may not, and may not permit others to: reverse engineer, decompile or otherwise derive source code from the Opentrons Products; -disassemble the Opentrons Products, except as instructed by Opentrons employees or Opentrons technical product manuals; +disassemble or bypass protection on Opentrons Products to exceed authorized access to Opentrons systems, or to analyze or modify components of the Opentrons Products for the purpose of gaining unauthorized access to confidential Opentrons or Opentrons Product information; copy, modify, or create derivative works of the Opentrons Products for the purpose of competing with Opentrons; remove or alter any proprietary notices or marks on the Opentrons Products; use the Opentrons Products in any manner that does not comply with the applicable laws in the jurisdiction(s) in which such use takes place; From 8eb14aed45da44bf8acb9f13cb309e997d5bb580 Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Thu, 18 Jul 2024 11:39:03 -0400 Subject: [PATCH 4/6] refactor(app): Desktop implementation for SelectRecoveryOption (#15691) This PR got away from me a little bit (and I still have to test it - the sum of these two is why it's in draft). It's probably best to view it commit by commit. Overall, the point is to implement the desktop styling for the SelectRecoveryOption component of error recovery, here: https://www.figma.com/design/8dMeu8MuPfXoORtOV6cACO/Feature%3A-Error-Recovery-August-Release?node-id=4829-78111&t=bTwOek0mZSA61ugm-4 . This is a little weird because it is exactly semantically equivalent to the ODD panel, but has a very different styling - two columns instead of one. It also uses the radio buttons that we use only in OT-2 tip length calibration method selection, which are old and use cssmodules and need a cleanup. The approach here was to add some fundamental structure components to InterventionModal that will render two columns on desktop (as the TwoColumn structure, including min-size and wrap) and one column on the ODD at full width. Then, we can build on top of that in ErrorRecovery, with the big difference being that the ErrorRecovery component also folds in the standard ER footer (or will - already had to find/replace a bunch of stuff, going to move other components over to specifying their footers through the wrapper as we update their styles). There were also some incidental refactors. By commit, - 981e82802c06c1c5890fe2cb45ce24ec704a6775 : we have these utility components for visualizing structure components; make them handle sometimes being size limited and sometimes not being - a7917e746138d314a0b76f4398fc0bcef5ec4877 : Add a wrapper around the RadioGroup component. We're going to need to update these styles further (that's a todo) but we don't want to, or I don't want to, have to mess with cssmodules. Also I think @TamarZanzouri is touching this stuff at the same time, so keep it isolated and droppable. Did I let the worms in my brain drive and make a typescript wrapper for getting the change events to take an inferred union of button values? Yes. Also note the one change to the underlying component to specify IDs so that we can use aria label linking in the passed components, which is both good practice and allows tests to work later. - e835f8604e5bcaf3430b67d21155562919a883d5 The error recovery component for viewing failed and next commands was bundled with presenting text in the left column, which is not what we want this panel to have, so split it out. - 699cb79b035aa157aca8544b9bddb9d5035eaedf The first fun one: Add a component that can responsively present one or two columns, and by "responsively" i mean abuse css mediaquery to only do it on touchscreen and maintain the two-column responsive presentation through desktop resizes. Best appreciated in storybook. - 52150cbdcd24fa87c1bb754b6d1296444b4ed691 Simultaneous refactor and feature - we had a RecoveryContentWrapper but it was really just a single column, this isn't helpful, add a single and twocolumn version (lots of find and replace since I changed the name) and then add a OneOrTwoColumn on top of that. These wrappers also encompass rendering footers because the footer needs to be in different places in one or two column - 00f38e8e4db80b1c16f2c4395b36c963a5bffa25 Use all that stuff to render SelectRecoveryOption! Note that the tests now run on both presentations and it all works because of that aria label stuff. ## Todo - [x] Visual tests, at all (this is why it's a draft) --- .../InterventionModal/OneColumn.stories.tsx | 36 +-- .../molecules/InterventionModal/OneColumn.tsx | 25 +- .../OneColumnOrTwoColumn.stories.tsx | 68 +++++ .../OneColumnOrTwoColumn.tsx | 55 ++++ .../molecules/InterventionModal/TwoColumn.tsx | 16 +- .../molecules/InterventionModal/constants.ts | 1 + app/src/molecules/InterventionModal/index.tsx | 2 + .../InterventionModal/story-utils/StandIn.tsx | 10 +- .../story-utils/VisibleContainer.tsx | 5 +- .../ErrorRecoveryFlows/RecoveryDoorOpen.tsx | 9 +- .../ErrorRecoveryFlows/RecoveryError.tsx | 6 +- .../RecoveryOptions/CancelRun.tsx | 9 +- .../RecoveryOptions/FillWellAndSkip.tsx | 6 +- .../RecoveryOptions/IgnoreErrorSkipStep.tsx | 9 +- .../RecoveryOptions/ManageTips.tsx | 13 +- .../RecoveryOptions/SelectRecoveryOption.tsx | 132 +++++++--- .../__tests__/IgnoreErrorSkipStep.test.tsx | 4 +- .../__tests__/SelectRecoveryOptions.test.tsx | 235 ++++++++++-------- .../ErrorRecoveryFlows/RunPausedSplash.tsx | 7 +- .../ErrorRecoveryFlows/__fixtures__/index.ts | 2 +- .../organisms/ErrorRecoveryFlows/constants.ts | 13 +- .../shared/FailedStepNextStep.tsx | 62 +++++ .../shared/RecoveryContentWrapper.tsx | 82 +++++- .../shared/RecoveryRadioGroup.tsx | 42 ++++ .../ErrorRecoveryFlows/shared/ReplaceTips.tsx | 6 +- .../ErrorRecoveryFlows/shared/SelectTips.tsx | 6 +- .../TwoColTextAndFailedStepNextStep.tsx | 81 ++---- .../shared/__tests__/StepInfo.test.tsx | 7 +- .../ErrorRecoveryFlows/shared/index.ts | 8 +- components/src/forms/RadioGroup.tsx | 2 +- 30 files changed, 664 insertions(+), 295 deletions(-) create mode 100644 app/src/molecules/InterventionModal/OneColumnOrTwoColumn.stories.tsx create mode 100644 app/src/molecules/InterventionModal/OneColumnOrTwoColumn.tsx create mode 100644 app/src/molecules/InterventionModal/constants.ts create mode 100644 app/src/organisms/ErrorRecoveryFlows/shared/FailedStepNextStep.tsx create mode 100644 app/src/organisms/ErrorRecoveryFlows/shared/RecoveryRadioGroup.tsx diff --git a/app/src/molecules/InterventionModal/OneColumn.stories.tsx b/app/src/molecules/InterventionModal/OneColumn.stories.tsx index 60e4efa03b8..ef4e8a6a02f 100644 --- a/app/src/molecules/InterventionModal/OneColumn.stories.tsx +++ b/app/src/molecules/InterventionModal/OneColumn.stories.tsx @@ -1,39 +1,13 @@ import * as React from 'react' -import { - LegacyStyledText, - Box, - Flex, - BORDERS, - RESPONSIVENESS, - SPACING, - ALIGN_CENTER, - JUSTIFY_CENTER, -} from '@opentrons/components' +import { Box, RESPONSIVENESS } from '@opentrons/components' import { OneColumn as OneColumnComponent } from './' +import { StandInContent } from './story-utils/StandIn' import type { Meta, StoryObj } from '@storybook/react' -function StandInContent(): JSX.Element { - return ( - - - This is a standin for some other component - - - ) -} - -const meta: Meta> = { +const meta: Meta> = { title: 'App/Molecules/InterventionModal/OneColumn', component: OneColumnComponent, render: args => ( @@ -46,7 +20,7 @@ const meta: Meta> = { `} > - + This is a standin for another component ), @@ -54,6 +28,6 @@ const meta: Meta> = { export default meta -export type Story = StoryObj +export type Story = StoryObj export const ExampleOneColumn: Story = { args: {} } diff --git a/app/src/molecules/InterventionModal/OneColumn.tsx b/app/src/molecules/InterventionModal/OneColumn.tsx index 0c36b6ecac7..e92f3ffd51e 100644 --- a/app/src/molecules/InterventionModal/OneColumn.tsx +++ b/app/src/molecules/InterventionModal/OneColumn.tsx @@ -1,11 +1,28 @@ import * as React from 'react' -import { Box } from '@opentrons/components' +import { + Flex, + DIRECTION_COLUMN, + JUSTIFY_SPACE_BETWEEN, +} from '@opentrons/components' +import type { StyleProps } from '@opentrons/components' -export interface OneColumnProps { +export interface OneColumnProps extends StyleProps { children: React.ReactNode } -export function OneColumn({ children }: OneColumnProps): JSX.Element { - return {children} +export function OneColumn({ + children, + ...styleProps +}: OneColumnProps): JSX.Element { + return ( + + {children} + + ) } diff --git a/app/src/molecules/InterventionModal/OneColumnOrTwoColumn.stories.tsx b/app/src/molecules/InterventionModal/OneColumnOrTwoColumn.stories.tsx new file mode 100644 index 00000000000..791edcbdb83 --- /dev/null +++ b/app/src/molecules/InterventionModal/OneColumnOrTwoColumn.stories.tsx @@ -0,0 +1,68 @@ +import * as React from 'react' + +import { OneColumnOrTwoColumn } from './' + +import { StandInContent } from './story-utils/StandIn' +import { VisibleContainer } from './story-utils/VisibleContainer' +import { css } from 'styled-components' +import { + RESPONSIVENESS, + Flex, + ALIGN_CENTER, + JUSTIFY_SPACE_AROUND, + DIRECTION_COLUMN, +} from '@opentrons/components' + +import type { Meta, StoryObj } from '@storybook/react' + +function Wrapper(props: {}): JSX.Element { + return ( + + + + This component is the only one shown on the ODD. + + + + + This component is shown in the right column on desktop. + + + + ) +} + +const meta: Meta> = { + title: 'App/Molecules/InterventionModal/OneColumnOrTwoColumn', + component: Wrapper, + decorators: [ + Story => ( + + + + ), + ], +} + +export default meta + +type Story = StoryObj + +export const OneOrTwoColumn: Story = {} diff --git a/app/src/molecules/InterventionModal/OneColumnOrTwoColumn.tsx b/app/src/molecules/InterventionModal/OneColumnOrTwoColumn.tsx new file mode 100644 index 00000000000..8a6455d67e3 --- /dev/null +++ b/app/src/molecules/InterventionModal/OneColumnOrTwoColumn.tsx @@ -0,0 +1,55 @@ +import * as React from 'react' + +import { css } from 'styled-components' +import { + Flex, + Box, + DIRECTION_ROW, + SPACING, + WRAP, + RESPONSIVENESS, +} from '@opentrons/components' +import type { StyleProps } from '@opentrons/components' +import { TWO_COLUMN_ELEMENT_MIN_WIDTH } from './constants' + +export interface OneColumnOrTwoColumnProps extends StyleProps { + children: [React.ReactNode, React.ReactNode] +} + +export function OneColumnOrTwoColumn({ + children: [leftOrSingleElement, optionallyDisplayedRightElement], + ...styleProps +}: OneColumnOrTwoColumnProps): JSX.Element { + return ( + + + {leftOrSingleElement} + + + {optionallyDisplayedRightElement} + + + ) +} diff --git a/app/src/molecules/InterventionModal/TwoColumn.tsx b/app/src/molecules/InterventionModal/TwoColumn.tsx index 8e87a2d62b5..f0ed10ebf2a 100644 --- a/app/src/molecules/InterventionModal/TwoColumn.tsx +++ b/app/src/molecules/InterventionModal/TwoColumn.tsx @@ -1,20 +1,28 @@ import * as React from 'react' import { Flex, Box, DIRECTION_ROW, SPACING, WRAP } from '@opentrons/components' +import type { StyleProps } from '@opentrons/components' +import { TWO_COLUMN_ELEMENT_MIN_WIDTH } from './constants' -export interface TwoColumnProps { +export interface TwoColumnProps extends StyleProps { children: [React.ReactNode, React.ReactNode] } export function TwoColumn({ children: [leftElement, rightElement], + ...styleProps }: TwoColumnProps): JSX.Element { return ( - - + + {leftElement} - + {rightElement} diff --git a/app/src/molecules/InterventionModal/constants.ts b/app/src/molecules/InterventionModal/constants.ts new file mode 100644 index 00000000000..c5f1fbea4d0 --- /dev/null +++ b/app/src/molecules/InterventionModal/constants.ts @@ -0,0 +1 @@ +export const TWO_COLUMN_ELEMENT_MIN_WIDTH = '17.1875rem' as const diff --git a/app/src/molecules/InterventionModal/index.tsx b/app/src/molecules/InterventionModal/index.tsx index 4d2de359b60..b7f0ab17be7 100644 --- a/app/src/molecules/InterventionModal/index.tsx +++ b/app/src/molecules/InterventionModal/index.tsx @@ -23,6 +23,7 @@ import type { IconName } from '@opentrons/components' import { ModalContentOneColSimpleButtons } from './ModalContentOneColSimpleButtons' import { TwoColumn } from './TwoColumn' import { OneColumn } from './OneColumn' +import { OneColumnOrTwoColumn } from './OneColumnOrTwoColumn' import { ModalContentMixed } from './ModalContentMixed' import { DescriptionContent } from './DescriptionContent' import { DeckMapContent } from './DeckMapContent' @@ -31,6 +32,7 @@ export { ModalContentOneColSimpleButtons, TwoColumn, OneColumn, + OneColumnOrTwoColumn, ModalContentMixed, DescriptionContent, DeckMapContent, diff --git a/app/src/molecules/InterventionModal/story-utils/StandIn.tsx b/app/src/molecules/InterventionModal/story-utils/StandIn.tsx index 28992aba717..0fb46f44b8c 100644 --- a/app/src/molecules/InterventionModal/story-utils/StandIn.tsx +++ b/app/src/molecules/InterventionModal/story-utils/StandIn.tsx @@ -1,13 +1,19 @@ import * as React from 'react' import { Box, BORDERS } from '@opentrons/components' -export function StandInContent(): JSX.Element { +export function StandInContent({ + children, +}: { + children?: React.ReactNode +}): JSX.Element { return ( + > + {children} + ) } diff --git a/app/src/molecules/InterventionModal/story-utils/VisibleContainer.tsx b/app/src/molecules/InterventionModal/story-utils/VisibleContainer.tsx index ac80ecdb063..b716b3335ee 100644 --- a/app/src/molecules/InterventionModal/story-utils/VisibleContainer.tsx +++ b/app/src/molecules/InterventionModal/story-utils/VisibleContainer.tsx @@ -1,13 +1,15 @@ import * as React from 'react' import { Box, BORDERS, SPACING } from '@opentrons/components' +import type { StyleProps } from '@opentrons/components' -export interface VisibleContainerProps { +export interface VisibleContainerProps extends StyleProps { children: JSX.Element | JSX.Element[] } export function VisibleContainer({ children, + ...styleProps }: VisibleContainerProps): JSX.Element { return ( {children} diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryDoorOpen.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryDoorOpen.tsx index 2683a8e3abe..17bf1cf0379 100644 --- a/app/src/organisms/ErrorRecoveryFlows/RecoveryDoorOpen.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/RecoveryDoorOpen.tsx @@ -16,7 +16,10 @@ import { } from '@opentrons/components' import { RUN_STATUS_AWAITING_RECOVERY_BLOCKED_BY_OPEN_DOOR } from '@opentrons/api-client' -import { RecoveryContentWrapper, RecoveryFooterButtons } from './shared' +import { + RecoverySingleColumnContentWrapper, + RecoveryFooterButtons, +} from './shared' import type { RecoveryContentProps } from './types' @@ -31,7 +34,7 @@ export function RecoveryDoorOpen({ const { t } = useTranslation('error_recovery') return ( - + - + ) } diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryError.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryError.tsx index 2c125f9897a..e38647927db 100644 --- a/app/src/organisms/ErrorRecoveryFlows/RecoveryError.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/RecoveryError.tsx @@ -13,7 +13,7 @@ import { } from '@opentrons/components' import { RECOVERY_MAP } from './constants' -import { RecoveryContentWrapper } from './shared' +import { RecoverySingleColumnContentWrapper } from './shared' import type { RecoveryContentProps } from './types' import { SmallButton } from '../../atoms/buttons' @@ -168,7 +168,7 @@ export function ErrorContent({ btnOnClick: () => void }): JSX.Element | null { return ( - + - + ) } diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/CancelRun.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/CancelRun.tsx index 5cf63db7c2f..b9358a10b11 100644 --- a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/CancelRun.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/CancelRun.tsx @@ -12,7 +12,10 @@ import { } from '@opentrons/components' import { RECOVERY_MAP } from '../constants' -import { RecoveryFooterButtons, RecoveryContentWrapper } from '../shared' +import { + RecoveryFooterButtons, + RecoverySingleColumnContentWrapper, +} from '../shared' import { SelectRecoveryOption } from './SelectRecoveryOption' import type { RecoveryContentProps } from '../types' @@ -56,7 +59,7 @@ function CancelRunConfirmation({ }) return ( - @@ -90,7 +93,7 @@ function CancelRunConfirmation({ primaryBtnTextOverride={t('confirm')} isLoadingPrimaryBtnAction={showBtnLoadingState} /> - + ) } diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/FillWellAndSkip.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/FillWellAndSkip.tsx index 19d51269d53..5f2f8971d0a 100644 --- a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/FillWellAndSkip.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/FillWellAndSkip.tsx @@ -12,7 +12,7 @@ import { RECOVERY_MAP } from '../constants' import { CancelRun } from './CancelRun' import { RecoveryFooterButtons, - RecoveryContentWrapper, + RecoverySingleColumnContentWrapper, LeftColumnLabwareInfo, TwoColTextAndFailedStepNextStep, } from '../shared' @@ -49,7 +49,7 @@ export function FillWell(props: RecoveryContentProps): JSX.Element | null { const { goBackPrevStep, proceedNextStep } = routeUpdateActions return ( - + - + ) } diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/IgnoreErrorSkipStep.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/IgnoreErrorSkipStep.tsx index f3f255381ca..c5ecf84a61b 100644 --- a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/IgnoreErrorSkipStep.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/IgnoreErrorSkipStep.tsx @@ -11,7 +11,10 @@ import { import { ODD_SECTION_TITLE_STYLE, RECOVERY_MAP } from '../constants' import { SelectRecoveryOption } from './SelectRecoveryOption' -import { RecoveryFooterButtons, RecoveryContentWrapper } from '../shared' +import { + RecoveryFooterButtons, + RecoverySingleColumnContentWrapper, +} from '../shared' import { RadioButton } from '../../../atoms/buttons' import type { RecoveryContentProps } from '../types' @@ -78,7 +81,7 @@ export function IgnoreErrorStepHome({ } return ( - + {t('ignore_similar_errors_later_in_run')} @@ -93,7 +96,7 @@ export function IgnoreErrorStepHome({ primaryBtnOnClick={primaryOnClick} secondaryBtnOnClick={goBackPrevStep} /> - + ) } diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/ManageTips.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/ManageTips.tsx index a5bfc5eede5..324564576c3 100644 --- a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/ManageTips.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/ManageTips.tsx @@ -12,7 +12,10 @@ import { FLEX_ROBOT_TYPE, OT2_ROBOT_TYPE } from '@opentrons/shared-data' import { RadioButton } from '../../../atoms/buttons' import { ODD_SECTION_TITLE_STYLE, RECOVERY_MAP } from '../constants' -import { RecoveryFooterButtons, RecoveryContentWrapper } from '../shared' +import { + RecoveryFooterButtons, + RecoverySingleColumnContentWrapper, +} from '../shared' import { DropTipWizardFlows } from '../../DropTipWizardFlows' import { DT_ROUTES } from '../../DropTipWizardFlows/constants' import { SelectRecoveryOption } from './SelectRecoveryOption' @@ -87,7 +90,7 @@ export function BeginRemoval({ } return ( - + {t('you_may_want_to_remove', { mount })} @@ -110,7 +113,7 @@ export function BeginRemoval({ /> - + ) } @@ -158,7 +161,7 @@ function DropTipFlowsContainer( const fixitCommandTypeUtils = useDropTipFlowUtils(props) return ( - + - + ) } diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/SelectRecoveryOption.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/SelectRecoveryOption.tsx index 17ccd2e853d..a33f2fb7abc 100644 --- a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/SelectRecoveryOption.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/SelectRecoveryOption.tsx @@ -6,16 +6,22 @@ import { DIRECTION_COLUMN, Flex, SPACING, - LegacyStyledText, + StyledText, } from '@opentrons/components' import { RECOVERY_MAP, ERROR_KINDS, ODD_SECTION_TITLE_STYLE, + ODD_ONLY, + DESKTOP_ONLY, } from '../constants' import { RadioButton } from '../../../atoms/buttons' -import { RecoveryFooterButtons, RecoveryContentWrapper } from '../shared' +import { + RecoveryODDOneDesktopTwoColumnContentWrapper, + RecoveryRadioGroup, + FailedStepNextStep, +} from '../shared' import type { ErrorKind, RecoveryContentProps, RecoveryRoute } from '../types' import type { PipetteWithTip } from '../../DropTipWizardFlows' @@ -45,6 +51,7 @@ export function SelectRecoveryOptionHome({ tipStatusUtils, currentRecoveryOptionUtils, getRecoveryOptionCopy, + ...rest }: RecoveryContentProps): JSX.Element | null { const { t } = useTranslation('error_recovery') const { proceedToRouteAndStep } = routeUpdateActions @@ -58,25 +65,41 @@ export function SelectRecoveryOptionHome({ useCurrentTipStatus(determineTipStatus) return ( - - - {t('choose_a_recovery_action')} - - - - - { + { setSelectedRecoveryOption(selectedRoute) void proceedToRouteAndStep(selectedRoute as RecoveryRoute) - }} - /> - + }, + }} + > + + + {t('choose_a_recovery_action')} + + + + + + + + + + ) } @@ -87,29 +110,66 @@ interface RecoveryOptionsProps { selectedRoute?: RecoveryRoute } // For ODD use only. -export function RecoveryOptions({ +export function ODDRecoveryOptions({ validRecoveryOptions, selectedRoute, setSelectedRoute, getRecoveryOptionCopy, -}: RecoveryOptionsProps): JSX.Element[] { - return validRecoveryOptions.map((recoveryOption: RecoveryRoute) => { - const optionName = getRecoveryOptionCopy(recoveryOption) - - return ( - { - setSelectedRoute(recoveryOption) - }} - isSelected={recoveryOption === selectedRoute} - /> - ) - }) +}: RecoveryOptionsProps): JSX.Element { + return ( + + {validRecoveryOptions.map((recoveryOption: RecoveryRoute) => { + const optionName = getRecoveryOptionCopy(recoveryOption) + return ( + { + setSelectedRoute(recoveryOption) + }} + isSelected={recoveryOption === selectedRoute} + /> + ) + })} + + ) } +export function DesktopRecoveryOptions({ + validRecoveryOptions, + selectedRoute, + setSelectedRoute, + getRecoveryOptionCopy, +}: RecoveryOptionsProps): JSX.Element { + return ( + { + setSelectedRoute(e.currentTarget.value) + }} + value={selectedRoute} + options={validRecoveryOptions.map( + (option: RecoveryRoute) => + ({ + value: option, + children: ( + + {getRecoveryOptionCopy(option)} + + ), + } as const) + )} + /> + ) +} // Pre-fetch tip attachment status. Users are not blocked from proceeding at this step. export function useCurrentTipStatus( determineTipStatus: () => Promise diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/IgnoreErrorSkipStep.test.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/IgnoreErrorSkipStep.test.tsx index b0dd4ec9f1d..d6241b7dcd9 100644 --- a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/IgnoreErrorSkipStep.test.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/IgnoreErrorSkipStep.test.tsx @@ -20,7 +20,9 @@ vi.mock('../shared', async () => { const actual = await vi.importActual('../shared') return { ...actual, - RecoveryContentWrapper: vi.fn(({ children }) =>
{children}
), + RecoverySingleColumnContentWrapper: vi.fn(({ children }) => ( +
{children}
+ )), } }) vi.mock('../SelectRecoveryOption') diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/SelectRecoveryOptions.test.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/SelectRecoveryOptions.test.tsx index 0db6521b2fc..a70e66d662e 100644 --- a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/SelectRecoveryOptions.test.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/SelectRecoveryOptions.test.tsx @@ -8,7 +8,8 @@ import { i18n } from '../../../../i18n' import { mockRecoveryContentProps } from '../../__fixtures__' import { SelectRecoveryOption, - RecoveryOptions, + ODDRecoveryOptions, + DesktopRecoveryOptions, getRecoveryOptions, GENERAL_ERROR_OPTIONS, OVERPRESSURE_WHILE_ASPIRATING_OPTIONS, @@ -24,15 +25,25 @@ import type { Mock } from 'vitest' const renderSelectRecoveryOption = ( props: React.ComponentProps ) => { - return renderWithProviders(, { + return renderWithProviders( + , + { + i18nInstance: i18n, + } + )[0] +} + +const renderODDRecoveryOptions = ( + props: React.ComponentProps +) => { + return renderWithProviders(, { i18nInstance: i18n, })[0] } - -const renderRecoveryOptions = ( - props: React.ComponentProps +const renderDesktopRecoveryOptions = ( + props: React.ComponentProps ) => { - return renderWithProviders(, { + return renderWithProviders(, { i18nInstance: i18n, })[0] } @@ -101,13 +112,13 @@ describe('SelectRecoveryOption', () => { screen.getByText('Choose a recovery action') - const retryStepOption = screen.getByRole('label', { name: 'Retry step' }) + const retryStepOption = screen.getAllByRole('label', { name: 'Retry step' }) clickButtonLabeled('Continue') expect( screen.queryByRole('button', { name: 'Go back' }) ).not.toBeInTheDocument() - fireEvent.click(retryStepOption) + fireEvent.click(retryStepOption[0]) clickButtonLabeled('Continue') expect(mockProceedToRouteAndStep).toHaveBeenCalledWith( @@ -125,14 +136,14 @@ describe('SelectRecoveryOption', () => { screen.getByText('Choose a recovery action') - const retryNewTips = screen.getByRole('label', { + const retryNewTips = screen.getAllByRole('label', { name: 'Retry with new tips', }) expect( screen.queryByRole('button', { name: 'Go back' }) ).not.toBeInTheDocument() - fireEvent.click(retryNewTips) + fireEvent.click(retryNewTips[0]) clickButtonLabeled('Continue') expect(mockProceedToRouteAndStep).toHaveBeenCalledWith(RETRY_NEW_TIPS.ROUTE) @@ -148,11 +159,11 @@ describe('SelectRecoveryOption', () => { screen.getByText('Choose a recovery action') - const fillManuallyAndSkip = screen.getByRole('label', { + const fillManuallyAndSkip = screen.getAllByRole('label', { name: 'Manually fill well and skip to next step', }) - fireEvent.click(fillManuallyAndSkip) + fireEvent.click(fillManuallyAndSkip[0]) clickButtonLabeled('Continue') expect(mockProceedToRouteAndStep).toHaveBeenCalledWith( @@ -170,11 +181,11 @@ describe('SelectRecoveryOption', () => { screen.getByText('Choose a recovery action') - const retrySameTips = screen.getByRole('label', { + const retrySameTips = screen.getAllByRole('label', { name: 'Retry with same tips', }) - fireEvent.click(retrySameTips) + fireEvent.click(retrySameTips[0]) clickButtonLabeled('Continue') expect(mockProceedToRouteAndStep).toHaveBeenCalledWith( @@ -192,11 +203,11 @@ describe('SelectRecoveryOption', () => { screen.getByText('Choose a recovery action') - const skipStepWithSameTips = screen.getByRole('label', { + const skipStepWithSameTips = screen.getAllByRole('label', { name: 'Skip to next step with same tips', }) - fireEvent.click(skipStepWithSameTips) + fireEvent.click(skipStepWithSameTips[0]) clickButtonLabeled('Continue') expect(mockProceedToRouteAndStep).toHaveBeenCalledWith( @@ -204,117 +215,123 @@ describe('SelectRecoveryOption', () => { ) }) }) +;([ + ['desktop', renderDesktopRecoveryOptions] as const, + ['odd', renderODDRecoveryOptions] as const, +] as const).forEach(([target, renderer]) => { + describe(`RecoveryOptions on ${target}`, () => { + let props: React.ComponentProps + let mockSetSelectedRoute: Mock + let mockGetRecoveryOptionCopy: Mock + + beforeEach(() => { + mockSetSelectedRoute = vi.fn() + mockGetRecoveryOptionCopy = vi.fn() + const generalRecoveryOptions = getRecoveryOptions( + ERROR_KINDS.GENERAL_ERROR + ) + + props = { + validRecoveryOptions: generalRecoveryOptions, + setSelectedRoute: mockSetSelectedRoute, + getRecoveryOptionCopy: mockGetRecoveryOptionCopy, + } + + when(mockGetRecoveryOptionCopy) + .calledWith(RECOVERY_MAP.RETRY_FAILED_COMMAND.ROUTE) + .thenReturn('Retry step') + when(mockGetRecoveryOptionCopy) + .calledWith(RECOVERY_MAP.CANCEL_RUN.ROUTE) + .thenReturn('Cancel run') + when(mockGetRecoveryOptionCopy) + .calledWith(RECOVERY_MAP.RETRY_NEW_TIPS.ROUTE) + .thenReturn('Retry with new tips') + when(mockGetRecoveryOptionCopy) + .calledWith(RECOVERY_MAP.FILL_MANUALLY_AND_SKIP.ROUTE) + .thenReturn('Manually fill well and skip to next step') + when(mockGetRecoveryOptionCopy) + .calledWith(RECOVERY_MAP.RETRY_SAME_TIPS.ROUTE) + .thenReturn('Retry with same tips') + when(mockGetRecoveryOptionCopy) + .calledWith(RECOVERY_MAP.SKIP_STEP_WITH_SAME_TIPS.ROUTE) + .thenReturn('Skip to next step with same tips') + when(mockGetRecoveryOptionCopy) + .calledWith(RECOVERY_MAP.SKIP_STEP_WITH_NEW_TIPS.ROUTE) + .thenReturn('Skip to next step with new tips') + when(mockGetRecoveryOptionCopy) + .calledWith(RECOVERY_MAP.IGNORE_AND_SKIP.ROUTE) + .thenReturn('Ignore error and skip to next step') + }) -describe('RecoveryOptions', () => { - let props: React.ComponentProps - let mockSetSelectedRoute: Mock - let mockGetRecoveryOptionCopy: Mock - - beforeEach(() => { - mockSetSelectedRoute = vi.fn() - mockGetRecoveryOptionCopy = vi.fn() - const generalRecoveryOptions = getRecoveryOptions(ERROR_KINDS.GENERAL_ERROR) - - props = { - validRecoveryOptions: generalRecoveryOptions, - setSelectedRoute: mockSetSelectedRoute, - getRecoveryOptionCopy: mockGetRecoveryOptionCopy, - } - - when(mockGetRecoveryOptionCopy) - .calledWith(RECOVERY_MAP.RETRY_FAILED_COMMAND.ROUTE) - .thenReturn('Retry step') - when(mockGetRecoveryOptionCopy) - .calledWith(RECOVERY_MAP.CANCEL_RUN.ROUTE) - .thenReturn('Cancel run') - when(mockGetRecoveryOptionCopy) - .calledWith(RECOVERY_MAP.RETRY_NEW_TIPS.ROUTE) - .thenReturn('Retry with new tips') - when(mockGetRecoveryOptionCopy) - .calledWith(RECOVERY_MAP.FILL_MANUALLY_AND_SKIP.ROUTE) - .thenReturn('Manually fill well and skip to next step') - when(mockGetRecoveryOptionCopy) - .calledWith(RECOVERY_MAP.RETRY_SAME_TIPS.ROUTE) - .thenReturn('Retry with same tips') - when(mockGetRecoveryOptionCopy) - .calledWith(RECOVERY_MAP.SKIP_STEP_WITH_SAME_TIPS.ROUTE) - .thenReturn('Skip to next step with same tips') - when(mockGetRecoveryOptionCopy) - .calledWith(RECOVERY_MAP.SKIP_STEP_WITH_NEW_TIPS.ROUTE) - .thenReturn('Skip to next step with new tips') - when(mockGetRecoveryOptionCopy) - .calledWith(RECOVERY_MAP.IGNORE_AND_SKIP.ROUTE) - .thenReturn('Ignore error and skip to next step') - }) - - it('renders valid recovery options for a general error errorKind', () => { - renderRecoveryOptions(props) + it('renders valid recovery options for a general error errorKind', () => { + renderer(props) - screen.getByRole('label', { name: 'Retry step' }) - screen.getByRole('label', { name: 'Cancel run' }) - }) + screen.getByRole('label', { name: 'Retry step' }) + screen.getByRole('label', { name: 'Cancel run' }) + }) - it(`renders valid recovery options for a ${ERROR_KINDS.OVERPRESSURE_WHILE_ASPIRATING} errorKind`, () => { - props = { - ...props, - validRecoveryOptions: OVERPRESSURE_WHILE_ASPIRATING_OPTIONS, - } + it(`renders valid recovery options for a ${ERROR_KINDS.OVERPRESSURE_WHILE_ASPIRATING} errorKind`, () => { + props = { + ...props, + validRecoveryOptions: OVERPRESSURE_WHILE_ASPIRATING_OPTIONS, + } - renderRecoveryOptions(props) + renderer(props) - screen.getByRole('label', { name: 'Retry with new tips' }) - screen.getByRole('label', { name: 'Cancel run' }) - }) + screen.getByRole('label', { name: 'Retry with new tips' }) + screen.getByRole('label', { name: 'Cancel run' }) + }) - it('updates the selectedRoute when a new option is selected', () => { - renderRecoveryOptions(props) + it('updates the selectedRoute when a new option is selected', () => { + renderer(props) - fireEvent.click(screen.getByRole('label', { name: 'Cancel run' })) + fireEvent.click(screen.getByRole('label', { name: 'Cancel run' })) - expect(mockSetSelectedRoute).toHaveBeenCalledWith( - RECOVERY_MAP.CANCEL_RUN.ROUTE - ) - }) + expect(mockSetSelectedRoute).toHaveBeenCalledWith( + RECOVERY_MAP.CANCEL_RUN.ROUTE + ) + }) - it(`renders valid recovery options for a ${ERROR_KINDS.NO_LIQUID_DETECTED} errorKind`, () => { - props = { - ...props, - validRecoveryOptions: NO_LIQUID_DETECTED_OPTIONS, - } + it(`renders valid recovery options for a ${ERROR_KINDS.NO_LIQUID_DETECTED} errorKind`, () => { + props = { + ...props, + validRecoveryOptions: NO_LIQUID_DETECTED_OPTIONS, + } - renderRecoveryOptions(props) + renderer(props) - screen.getByRole('label', { - name: 'Manually fill well and skip to next step', + screen.getByRole('label', { + name: 'Manually fill well and skip to next step', + }) + screen.getByRole('label', { name: 'Ignore error and skip to next step' }) + screen.getByRole('label', { name: 'Cancel run' }) }) - screen.getByRole('label', { name: 'Ignore error and skip to next step' }) - screen.getByRole('label', { name: 'Cancel run' }) - }) - it(`renders valid recovery options for a ${ERROR_KINDS.OVERPRESSURE_PREPARE_TO_ASPIRATE} errorKind`, () => { - props = { - ...props, - validRecoveryOptions: OVERPRESSURE_PREPARE_TO_ASPIRATE, - } + it(`renders valid recovery options for a ${ERROR_KINDS.OVERPRESSURE_PREPARE_TO_ASPIRATE} errorKind`, () => { + props = { + ...props, + validRecoveryOptions: OVERPRESSURE_PREPARE_TO_ASPIRATE, + } - renderRecoveryOptions(props) + renderer(props) - screen.getByRole('label', { name: 'Retry with new tips' }) - screen.getByRole('label', { name: 'Retry with same tips' }) - screen.getByRole('label', { name: 'Cancel run' }) - }) + screen.getByRole('label', { name: 'Retry with new tips' }) + screen.getByRole('label', { name: 'Retry with same tips' }) + screen.getByRole('label', { name: 'Cancel run' }) + }) - it(`renders valid recovery options for a ${ERROR_KINDS.OVERPRESSURE_WHILE_DISPENSING} errorKind`, () => { - props = { - ...props, - validRecoveryOptions: OVERPRESSURE_WHILE_DISPENSING_OPTIONS, - } + it(`renders valid recovery options for a ${ERROR_KINDS.OVERPRESSURE_WHILE_DISPENSING} errorKind`, () => { + props = { + ...props, + validRecoveryOptions: OVERPRESSURE_WHILE_DISPENSING_OPTIONS, + } - renderRecoveryOptions(props) + renderer(props) - screen.getByRole('label', { name: 'Skip to next step with same tips' }) - screen.getByRole('label', { name: 'Skip to next step with new tips' }) - screen.getByRole('label', { name: 'Cancel run' }) + screen.getByRole('label', { name: 'Skip to next step with same tips' }) + screen.getByRole('label', { name: 'Skip to next step with new tips' }) + screen.getByRole('label', { name: 'Cancel run' }) + }) }) }) diff --git a/app/src/organisms/ErrorRecoveryFlows/RunPausedSplash.tsx b/app/src/organisms/ErrorRecoveryFlows/RunPausedSplash.tsx index 23633a8b20b..f9d253719ed 100644 --- a/app/src/organisms/ErrorRecoveryFlows/RunPausedSplash.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/RunPausedSplash.tsx @@ -30,7 +30,7 @@ import { LargeButton } from '../../atoms/buttons' import { RECOVERY_MAP } from './constants' import { RecoveryInterventionModal, - RecoveryContentWrapper, + RecoverySingleColumnContentWrapper, StepInfo, } from './shared' @@ -151,13 +151,12 @@ export function RunPausedSplash( titleHeading={buildTitleHeadingDesktop()} isOnDevice={isOnDevice} > - + - + ) } diff --git a/app/src/organisms/ErrorRecoveryFlows/__fixtures__/index.ts b/app/src/organisms/ErrorRecoveryFlows/__fixtures__/index.ts index 51639bb3981..7666d7beebf 100644 --- a/app/src/organisms/ErrorRecoveryFlows/__fixtures__/index.ts +++ b/app/src/organisms/ErrorRecoveryFlows/__fixtures__/index.ts @@ -73,7 +73,7 @@ export const mockRecoveryContentProps: RecoveryContentProps = { failedPipetteInfo: {} as any, deckMapUtils: { setSelectedLocation: () => {} } as any, stepCounts: {} as any, - protocolAnalysis: { commands: [mockFailedCommand] } as any, + protocolAnalysis: mockRobotSideAnalysis, trackExternalMap: () => null, hasLaunchedRecovery: true, getRecoveryOptionCopy: () => 'MOCK_COPY', diff --git a/app/src/organisms/ErrorRecoveryFlows/constants.ts b/app/src/organisms/ErrorRecoveryFlows/constants.ts index 7e9a9ab2a9a..846f7e2efc0 100644 --- a/app/src/organisms/ErrorRecoveryFlows/constants.ts +++ b/app/src/organisms/ErrorRecoveryFlows/constants.ts @@ -1,6 +1,6 @@ import { css } from 'styled-components' -import { SPACING, TYPOGRAPHY } from '@opentrons/components' +import { SPACING, TYPOGRAPHY, RESPONSIVENESS } from '@opentrons/components' import type { StepOrder } from './types' @@ -211,3 +211,14 @@ export const BODY_TEXT_STYLE = css` export const ODD_SECTION_TITLE_STYLE = css` margin-bottom: ${SPACING.spacing16}; ` + +export const ODD_ONLY = css` + @media not (${RESPONSIVENESS.touchscreenMediaQuerySpecs}) { + display: none; + } +` +export const DESKTOP_ONLY = css` + @media (${RESPONSIVENESS.touchscreenMediaQuerySpecs}) { + display: none; + } +` diff --git a/app/src/organisms/ErrorRecoveryFlows/shared/FailedStepNextStep.tsx b/app/src/organisms/ErrorRecoveryFlows/shared/FailedStepNextStep.tsx new file mode 100644 index 00000000000..b29ade0d2eb --- /dev/null +++ b/app/src/organisms/ErrorRecoveryFlows/shared/FailedStepNextStep.tsx @@ -0,0 +1,62 @@ +import * as React from 'react' +import { useTranslation } from 'react-i18next' +import { CategorizedStepContent } from '../../../molecules/InterventionModal' +import type { RecoveryContentProps } from '../types' + +export function FailedStepNextStep({ + stepCounts, + failedCommand, + commandsAfterFailedCommand, + protocolAnalysis, + robotType, +}: Pick< + RecoveryContentProps, + | 'stepCounts' + | 'failedCommand' + | 'commandsAfterFailedCommand' + | 'protocolAnalysis' + | 'robotType' +>): JSX.Element { + const { t } = useTranslation('error_recovery') + + const nthStepAfter = (n: number): number | undefined => + stepCounts.currentStepNumber == null + ? undefined + : stepCounts.currentStepNumber + n + const nthCommand = (n: number): typeof failedCommand => + commandsAfterFailedCommand != null + ? n < commandsAfterFailedCommand.length + ? commandsAfterFailedCommand[n] + : null + : null + + const commandsAfter = [nthCommand(0), nthCommand(1)] as const + + const indexedCommandsAfter = [ + commandsAfter[0] != null + ? { command: commandsAfter[0], index: nthStepAfter(1) } + : null, + commandsAfter[1] != null + ? { command: commandsAfter[1], index: nthStepAfter(2) } + : null, + ] as const + return ( + + ) +} diff --git a/app/src/organisms/ErrorRecoveryFlows/shared/RecoveryContentWrapper.tsx b/app/src/organisms/ErrorRecoveryFlows/shared/RecoveryContentWrapper.tsx index 3c10279d50d..b9acdcc8cae 100644 --- a/app/src/organisms/ErrorRecoveryFlows/shared/RecoveryContentWrapper.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/shared/RecoveryContentWrapper.tsx @@ -10,19 +10,34 @@ import { Flex, RESPONSIVENESS, } from '@opentrons/components' - import type { StyleProps } from '@opentrons/components' +import { + OneColumn, + TwoColumn, + OneColumnOrTwoColumn, +} from '../../../molecules/InterventionModal' +import { RecoveryFooterButtons } from './RecoveryFooterButtons' -interface SingleColumnContentWrapperProps extends StyleProps { +interface SingleColumnContentWrapperProps { children: React.ReactNode + footerDetails?: React.ComponentProps +} + +interface TwoColumnContentWrapperProps { + children: [React.ReactNode, React.ReactNode] + footerDetails?: React.ComponentProps +} + +interface OneOrTwoColumnContentWrapperProps { + children: [React.ReactNode, React.ReactNode] + footerDetails?: React.ComponentProps } // For flex-direction: column recovery content with one column only. -// -// For ODD use only. -export function RecoveryContentWrapper({ +export function RecoverySingleColumnContentWrapper({ children, + footerDetails, ...styleProps -}: SingleColumnContentWrapperProps): JSX.Element { +}: SingleColumnContentWrapperProps & StyleProps): JSX.Element { return ( - {children} + + {children} + + {footerDetails != null ? ( + + ) : null} + + ) +} + +// For two-column recovery content +export function RecoveryTwoColumnContentWrapper({ + children, + footerDetails, +}: TwoColumnContentWrapperProps): JSX.Element { + const [leftChild, rightChild] = children + return ( + + + {leftChild} + {rightChild} + + {footerDetails != null ? ( + + ) : null} + + ) +} + +// For recovery content with one column on ODD and two columns on desktop +export function RecoveryODDOneDesktopTwoColumnContentWrapper({ + children: [leftOrSingleElement, optionallyShownRightElement], + footerDetails, +}: OneOrTwoColumnContentWrapperProps): JSX.Element { + return ( + + + {leftOrSingleElement} + {optionallyShownRightElement} + + {footerDetails != null ? ( + + ) : null} ) } @@ -38,8 +104,8 @@ export function RecoveryContentWrapper({ const STYLE = css` gap: ${SPACING.spacing24}; width: 100%; + height: 100%; @media ${RESPONSIVENESS.touchscreenMediaQuerySpecs} { gap: none; - height: 100%; } ` diff --git a/app/src/organisms/ErrorRecoveryFlows/shared/RecoveryRadioGroup.tsx b/app/src/organisms/ErrorRecoveryFlows/shared/RecoveryRadioGroup.tsx new file mode 100644 index 00000000000..571f0b0333a --- /dev/null +++ b/app/src/organisms/ErrorRecoveryFlows/shared/RecoveryRadioGroup.tsx @@ -0,0 +1,42 @@ +import * as React from 'react' + +import type { ChangeEventHandler } from 'react' +import { RadioGroup, SPACING, Flex } from '@opentrons/components' + +// note: this typescript stuff is so that e.currentTarget.value in the ChangeEventHandler +// is deduced to a union of the values of the options passed to the radiogroup rather than +// just string +export interface Target extends Omit { + value: T +} + +export type Options = Array<{ + value: T + children: React.ReactNode +}> + +export interface RecoveryRadioGroupProps + extends Omit< + React.ComponentProps, + 'labelTextClassName' | 'options' | 'onchange' + > { + options: Options + onChange: ChangeEventHandler> +} + +export function RecoveryRadioGroup( + props: RecoveryRadioGroupProps +): JSX.Element { + return ( + ({ + name: '', + value: radioOption.value, + children: ( + {radioOption.children} + ), + }))} + /> + ) +} diff --git a/app/src/organisms/ErrorRecoveryFlows/shared/ReplaceTips.tsx b/app/src/organisms/ErrorRecoveryFlows/shared/ReplaceTips.tsx index e577abb7bb1..f7513af14c8 100644 --- a/app/src/organisms/ErrorRecoveryFlows/shared/ReplaceTips.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/shared/ReplaceTips.tsx @@ -3,7 +3,7 @@ import * as React from 'react' import { Flex } from '@opentrons/components' import { useTranslation } from 'react-i18next' -import { RecoveryContentWrapper } from './RecoveryContentWrapper' +import { RecoverySingleColumnContentWrapper } from './RecoveryContentWrapper' import { TwoColumn, DeckMapContent } from '../../../molecules/InterventionModal' import { RecoveryFooterButtons } from './RecoveryFooterButtons' import { LeftColumnLabwareInfo } from './LeftColumnLabwareInfo' @@ -36,7 +36,7 @@ export function ReplaceTips(props: RecoveryContentProps): JSX.Element | null { } return ( - + - + ) } diff --git a/app/src/organisms/ErrorRecoveryFlows/shared/SelectTips.tsx b/app/src/organisms/ErrorRecoveryFlows/shared/SelectTips.tsx index f51d9d2ddd6..0b6f66aa484 100644 --- a/app/src/organisms/ErrorRecoveryFlows/shared/SelectTips.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/shared/SelectTips.tsx @@ -2,7 +2,7 @@ import * as React from 'react' import { useTranslation } from 'react-i18next' import { RECOVERY_MAP } from '../constants' -import { RecoveryContentWrapper } from './RecoveryContentWrapper' +import { RecoverySingleColumnContentWrapper } from './RecoveryContentWrapper' import { TwoColumn } from '../../../molecules/InterventionModal' import { RecoveryFooterButtons } from './RecoveryFooterButtons' import { LeftColumnLabwareInfo } from './LeftColumnLabwareInfo' @@ -42,7 +42,7 @@ export function SelectTips(props: RecoveryContentProps): JSX.Element | null { toggleModal={toggleModal} /> )} - + - + ) } diff --git a/app/src/organisms/ErrorRecoveryFlows/shared/TwoColTextAndFailedStepNextStep.tsx b/app/src/organisms/ErrorRecoveryFlows/shared/TwoColTextAndFailedStepNextStep.tsx index a9bdd50399f..4ed62e8ff8d 100644 --- a/app/src/organisms/ErrorRecoveryFlows/shared/TwoColTextAndFailedStepNextStep.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/shared/TwoColTextAndFailedStepNextStep.tsx @@ -1,5 +1,4 @@ import * as React from 'react' -import { useTranslation } from 'react-i18next' import { css } from 'styled-components' import { DIRECTION_COLUMN, @@ -9,12 +8,10 @@ import { RESPONSIVENESS, } from '@opentrons/components' -import { RecoveryContentWrapper } from './RecoveryContentWrapper' -import { - TwoColumn, - CategorizedStepContent, -} from '../../../molecules/InterventionModal' +import { RecoverySingleColumnContentWrapper } from './RecoveryContentWrapper' +import { TwoColumn } from '../../../molecules/InterventionModal' import { RecoveryFooterButtons } from './RecoveryFooterButtons' +import { FailedStepNextStep } from './FailedStepNextStep' import type { RecoveryContentProps } from '../types' @@ -24,60 +21,34 @@ type TwoColTextAndFailedStepNextStepProps = RecoveryContentProps & { primaryBtnCopy: string primaryBtnOnClick: () => void secondaryBtnOnClickOverride?: () => void - secondaryBtnOnClickCopyOverride?: string } /** * Left Column: Title + body text * Right column: FailedStepNextStep */ -export function TwoColTextAndFailedStepNextStep({ - leftColBodyText, - leftColTitle, - primaryBtnCopy, - primaryBtnOnClick, - secondaryBtnOnClickOverride, - secondaryBtnOnClickCopyOverride, - routeUpdateActions, - failedCommand, - stepCounts, - commandsAfterFailedCommand, - protocolAnalysis, - robotType, -}: TwoColTextAndFailedStepNextStepProps): JSX.Element | null { +export function TwoColTextAndFailedStepNextStep( + props: TwoColTextAndFailedStepNextStepProps +): JSX.Element | null { + const { + leftColBodyText, + leftColTitle, + primaryBtnCopy, + primaryBtnOnClick, + secondaryBtnOnClickOverride, + routeUpdateActions, + } = props const { goBackPrevStep } = routeUpdateActions - const { t } = useTranslation('error_recovery') - const nthStepAfter = (n: number): number | undefined => - stepCounts.currentStepNumber == null - ? undefined - : stepCounts.currentStepNumber + n - const nthCommand = (n: number): typeof failedCommand => - commandsAfterFailedCommand != null - ? n < commandsAfterFailedCommand.length - ? commandsAfterFailedCommand[n] - : null - : null - - const commandsAfter = [nthCommand(0), nthCommand(1)] as const - - const indexedCommandsAfter = [ - commandsAfter[0] != null - ? { command: commandsAfter[0], index: nthStepAfter(1) } - : null, - commandsAfter[1] != null - ? { command: commandsAfter[1], index: nthStepAfter(2) } - : null, - ] as const return ( - + @@ -94,29 +65,13 @@ export function TwoColTextAndFailedStepNextStep({ {leftColBodyText} - + - + ) } diff --git a/app/src/organisms/ErrorRecoveryFlows/shared/__tests__/StepInfo.test.tsx b/app/src/organisms/ErrorRecoveryFlows/shared/__tests__/StepInfo.test.tsx index 54e579daf93..4e7e8b393fa 100644 --- a/app/src/organisms/ErrorRecoveryFlows/shared/__tests__/StepInfo.test.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/shared/__tests__/StepInfo.test.tsx @@ -3,7 +3,7 @@ import { describe, it, expect, beforeEach, vi } from 'vitest' import { screen } from '@testing-library/react' import { renderWithProviders } from '../../../../__testing-utils__' -import { mockRecoveryContentProps } from '../../__fixtures__' +import { mockRecoveryContentProps, mockFailedCommand } from '../../__fixtures__' import { i18n } from '../../../../i18n' import { StepInfo } from '../StepInfo' import { CommandText } from '../../../../molecules/Command' @@ -21,7 +21,10 @@ describe('StepInfo', () => { beforeEach(() => { props = { - ...mockRecoveryContentProps, + ...{ + ...mockRecoveryContentProps, + protocolAnalysis: { commands: [mockFailedCommand] } as any, + }, textStyle: 'h4', stepCounts: { currentStepNumber: 5, diff --git a/app/src/organisms/ErrorRecoveryFlows/shared/index.ts b/app/src/organisms/ErrorRecoveryFlows/shared/index.ts index 33c9299db44..955058e5311 100644 --- a/app/src/organisms/ErrorRecoveryFlows/shared/index.ts +++ b/app/src/organisms/ErrorRecoveryFlows/shared/index.ts @@ -1,5 +1,9 @@ export { RecoveryFooterButtons } from './RecoveryFooterButtons' -export { RecoveryContentWrapper } from './RecoveryContentWrapper' +export { + RecoverySingleColumnContentWrapper, + RecoveryTwoColumnContentWrapper, + RecoveryODDOneDesktopTwoColumnContentWrapper, +} from './RecoveryContentWrapper' export { ReplaceTips } from './ReplaceTips' export { SelectTips } from './SelectTips' export { TwoColTextAndFailedStepNextStep } from './TwoColTextAndFailedStepNextStep' @@ -9,5 +13,7 @@ export { TipSelectionModal } from './TipSelectionModal' export { StepInfo } from './StepInfo' export { useErrorDetailsModal, ErrorDetailsModal } from './ErrorDetailsModal' export { RecoveryInterventionModal } from './RecoveryInterventionModal' +export { FailedStepNextStep } from './FailedStepNextStep' +export { RecoveryRadioGroup } from './RecoveryRadioGroup' export type { RecoveryInterventionModalProps } from './RecoveryInterventionModal' diff --git a/components/src/forms/RadioGroup.tsx b/components/src/forms/RadioGroup.tsx index d934616a227..5d409540032 100644 --- a/components/src/forms/RadioGroup.tsx +++ b/components/src/forms/RadioGroup.tsx @@ -50,7 +50,7 @@ export function RadioGroup(props: RadioGroupProps): JSX.Element { const useStyleUpdates = props.useBlueChecked && radio.value === props.value return ( -