diff --git a/api/docs/v2/versioning.rst b/api/docs/v2/versioning.rst index 081edca651a..9c4ccc62a0d 100644 --- a/api/docs/v2/versioning.rst +++ b/api/docs/v2/versioning.rst @@ -132,6 +132,11 @@ This table lists the correspondence between Protocol API versions and robot soft Changes in API Versions ======================= +Version 2.20 +------------ + +- You can now call :py:obj:`.ProtocolContext.define_liquid()` without supplying a ``description`` or ``display_color``. + Version 2.19 ------------ diff --git a/api/src/opentrons/execute.py b/api/src/opentrons/execute.py index e4109d5d390..2e6a5870f7d 100644 --- a/api/src/opentrons/execute.py +++ b/api/src/opentrons/execute.py @@ -142,23 +142,23 @@ def get_protocol_api( When this function is called, modules and instruments will be recached. :param version: The API version to use. This must be lower than - ``opentrons.protocol_api.MAX_SUPPORTED_VERSION``. - It may be specified either as a string (``'2.0'``) or - as a ``protocols.types.APIVersion`` - (``APIVersion(2, 0)``). + ``opentrons.protocol_api.MAX_SUPPORTED_VERSION``. + It may be specified either as a string (``'2.0'``) or + as a ``protocols.types.APIVersion`` + (``APIVersion(2, 0)``). :param bundled_labware: If specified, a mapping from labware names to - labware definitions for labware to consider in the - protocol. Note that if you specify this, _only_ - labware in this argument will be allowed in the - protocol. This is preparation for a beta feature - and is best not used. + labware definitions for labware to consider in the + protocol. Note that if you specify this, *only* + labware in this argument will be allowed in the + protocol. This is preparation for a beta feature + and is best not used. :param bundled_data: If specified, a mapping from filenames to contents - for data to be available in the protocol from - :py:obj:`opentrons.protocol_api.ProtocolContext.bundled_data`. + for data to be available in the protocol from + :py:obj:`opentrons.protocol_api.ProtocolContext.bundled_data`. :param extra_labware: A mapping from labware load names to custom labware definitions. - If this is ``None`` (the default), and this function is called on a robot, - it will look for labware in the ``labware`` subdirectory of the Jupyter - data directory. + If this is ``None`` (the default), and this function is called on a robot, + it will look for labware in the ``labware`` subdirectory of the Jupyter + data directory. :return: The protocol context. """ if isinstance(version, str): @@ -313,18 +313,18 @@ def execute( :param protocol_file: The protocol file to execute :param protocol_name: The name of the protocol file. This is required - internally, but it may not be a thing we can get - from the protocol_file argument. + internally, but it may not be a thing we can get + from the ``protocol_file`` argument. :param propagate_logs: Whether this function should allow logs from the - Opentrons stack to propagate up to the root handler. - This can be useful if you're integrating this - function in a larger application, but most logs that - occur during protocol simulation are best associated - with the actions in the protocol that cause them. - Default: ``False`` + Opentrons stack to propagate up to the root handler. + This can be useful if you're integrating this + function in a larger application, but most logs that + occur during protocol simulation are best associated + with the actions in the protocol that cause them. + Default: ``False`` :param log_level: The level of logs to emit on the command line: - ``"debug"``, ``"info"``, ``"warning"``, or ``"error"``. - Defaults to ``"warning"``. + ``"debug"``, ``"info"``, ``"warning"``, or ``"error"``. + Defaults to ``"warning"``. :param emit_runlog: A callback for printing the run log. If specified, this will be called whenever a command adds an entry to the run log, which can be used for display and progress @@ -353,17 +353,17 @@ def execute( ``KeyError``. :param custom_labware_paths: A list of directories to search for custom labware. - Loads valid labware from these paths and makes them available - to the protocol context. If this is ``None`` (the default), and - this function is called on a robot, it will look in the ``labware`` - subdirectory of the Jupyter data directory. + Loads valid labware from these paths and makes them available + to the protocol context. If this is ``None`` (the default), and + this function is called on a robot, it will look in the ``labware`` + subdirectory of the Jupyter data directory. :param custom_data_paths: A list of directories or files to load custom - data files from. Ignored if the apiv2 feature - flag if not set. Entries may be either files or - directories. Specified files and the - non-recursive contents of specified directories - are presented by the protocol context in - ``ProtocolContext.bundled_data``. + data files from. Ignored if the apiv2 feature + flag if not set. Entries may be either files or + directories. Specified files and the + non-recursive contents of specified directories + are presented by the protocol context in + ``ProtocolContext.bundled_data``. """ stack_logger = logging.getLogger("opentrons") stack_logger.propagate = propagate_logs @@ -457,10 +457,10 @@ def main() -> int: """Handler for command line invocation to run a protocol. :param argv: The arguments the program was invoked with; this is usually - :py:obj:`sys.argv` but if you want to override that you can. + :py:obj:`sys.argv` but if you want to override that you can. :returns int: A success or failure value suitable for use as a shell - return code passed to :py:obj:`sys.exit` (0 means success, - anything else is a kind of failure). + return code passed to :py:obj:`sys.exit` (0 means success, + anything else is a kind of failure). """ parser = argparse.ArgumentParser( prog="opentrons_execute", description="Run an OT-2 protocol" diff --git a/api/src/opentrons/hardware_control/ot3api.py b/api/src/opentrons/hardware_control/ot3api.py index 4f0cf262775..5f9c9840834 100644 --- a/api/src/opentrons/hardware_control/ot3api.py +++ b/api/src/opentrons/hardware_control/ot3api.py @@ -2714,7 +2714,10 @@ async def liquid_probe( error: Optional[PipetteLiquidNotFoundError] = None pos = await self.gantry_position(checked_mount, refresh=True) - while (probe_start_pos.z - pos.z) < max_z_dist: + # probe_start_pos.z + z_distance of pass - pos.z should be < max_z_dist + # due to rounding errors this can get caught in an infinite loop when the distance is almost equal + # so we check to see if they're within 0.01 which is 1/5th the minimum movement distance from move_utils.py + while (probe_start_pos.z - pos.z) < (max_z_dist + 0.01): # safe distance so we don't accidentally aspirate liquid if we're already close to liquid safe_plunger_pos = top_types.Point( pos.x, pos.y, pos.z + probe_safe_reset_mm @@ -2724,7 +2727,7 @@ async def liquid_probe( pos.x, pos.y, pos.z + probe_pass_z_offset_mm ) max_z_time = ( - max_z_dist - (probe_start_pos.z - safe_plunger_pos.z) + max_z_dist - probe_start_pos.z + pass_start_pos.z ) / probe_settings.mount_speed p_travel_required_for_z = max_z_time * probe_settings.plunger_speed p_pass_travel = min(p_travel_required_for_z, p_working_mm) diff --git a/api/src/opentrons/protocol_api/protocol_context.py b/api/src/opentrons/protocol_api/protocol_context.py index 59b7d1d8aee..054af703fe7 100644 --- a/api/src/opentrons/protocol_api/protocol_context.py +++ b/api/src/opentrons/protocol_api/protocol_context.py @@ -87,6 +87,16 @@ ] +class _Unset: + """A sentinel value for when no value has been supplied for an argument, + when `None` is already taken for some other meaning. + + User code should never use this explicitly. + """ + + pass + + class ProtocolContext(CommandPublisher): """A context for the state of a protocol. @@ -1197,17 +1207,54 @@ def set_rail_lights(self, on: bool) -> None: @requires_version(2, 14) def define_liquid( - self, name: str, description: Optional[str], display_color: Optional[str] + self, + name: str, + description: Union[str, None, _Unset] = _Unset(), + display_color: Union[str, None, _Unset] = _Unset(), ) -> Liquid: + # This first line of the docstring overrides the method signature in our public + # docs, which would otherwise have the `_Unset()`s expanded to a bunch of junk. """ + define_liquid(self, name: str, description: Optional[str] = None, display_color: Optional[str] = None) + Define a liquid within a protocol. :param str name: A human-readable name for the liquid. - :param str description: An optional description of the liquid. - :param str display_color: An optional hex color code, with hash included, to represent the specified liquid. Standard three-value, four-value, six-value, and eight-value syntax are all acceptable. + :param Optional[str] description: An optional description of the liquid. + :param Optional[str] display_color: An optional hex color code, with hash included, + to represent the specified liquid. For example, ``"#48B1FA"``. + Standard three-value, four-value, six-value, and eight-value syntax are all + acceptable. :return: A :py:class:`~opentrons.protocol_api.Liquid` object representing the specified liquid. + + .. versionchanged:: 2.20 + You can now omit the ``description`` and ``display_color`` arguments. + Formerly, when you didn't want to provide values, you had to supply + ``description=None`` and ``display_color=None`` explicitly. """ + desc_and_display_color_omittable_since = APIVersion(2, 20) + if isinstance(description, _Unset): + if self._api_version < desc_and_display_color_omittable_since: + raise APIVersionError( + api_element="Calling `define_liquid()` without a `description`", + current_version=str(self._api_version), + until_version=str(desc_and_display_color_omittable_since), + message="Use a newer API version or explicitly supply `description=None`.", + ) + else: + description = None + if isinstance(display_color, _Unset): + if self._api_version < desc_and_display_color_omittable_since: + raise APIVersionError( + api_element="Calling `define_liquid()` without a `display_color`", + current_version=str(self._api_version), + until_version=str(desc_and_display_color_omittable_since), + message="Use a newer API version or explicitly supply `display_color=None`.", + ) + else: + display_color = None + return self._core.define_liquid( name=name, description=description, diff --git a/api/src/opentrons/protocol_api/validation.py b/api/src/opentrons/protocol_api/validation.py index 207c417cf5e..1ad6628ae24 100644 --- a/api/src/opentrons/protocol_api/validation.py +++ b/api/src/opentrons/protocol_api/validation.py @@ -208,7 +208,7 @@ def ensure_and_convert_deck_slot( api_element=f"Specifying a deck slot like '{deck_slot}'", until_version=f"{_COORDINATE_DECK_LABEL_VERSION_GATE}", current_version=f"{api_version}", - message=f" Increase your protocol's apiLevel, or use slot '{alternative}' instead.", + message=f"Increase your protocol's apiLevel, or use slot '{alternative}' instead.", ) return parsed_slot.to_equivalent_for_robot_type(robot_type) diff --git a/api/src/opentrons/protocol_runner/create_simulating_orchestrator.py b/api/src/opentrons/protocol_runner/create_simulating_orchestrator.py index 0aa5114b5a5..dd826eeade3 100644 --- a/api/src/opentrons/protocol_runner/create_simulating_orchestrator.py +++ b/api/src/opentrons/protocol_runner/create_simulating_orchestrator.py @@ -47,7 +47,11 @@ async def create_simulating_orchestrator( robot_type=robot_type ) - # TODO(mc, 2021-08-25): move initial home to protocol engine + # TODO(mm, 2024-08-06): This home has theoretically been replaced by Protocol Engine + # `home` commands within the `RunOrchestrator` or `ProtocolRunner`. However, it turns + # out that this `HardwareControlAPI`-level home is accidentally load-bearing, + # working around Protocol Engine bugs where *both* layers need to be homed for + # certain commands to work. https://opentrons.atlassian.net/browse/EXEC-646 await simulating_hardware_api.home() protocol_engine = await create_protocol_engine( diff --git a/api/src/opentrons/simulate.py b/api/src/opentrons/simulate.py index 01a1484c6b5..1c6a4c3a745 100644 --- a/api/src/opentrons/simulate.py +++ b/api/src/opentrons/simulate.py @@ -228,10 +228,9 @@ def get_protocol_api( use_virtual_hardware: bool = True, ) -> protocol_api.ProtocolContext: """ - Build and return a ``protocol_api.ProtocolContext`` - connected to Virtual Smoothie. + Build and return a ``protocol_api.ProtocolContext`` that simulates robot control. - This can be used to run protocols from interactive Python sessions + This can be used to simulate protocols from interactive Python sessions such as Jupyter or an interpreter on the command line: .. code-block:: python @@ -242,28 +241,31 @@ def get_protocol_api( >>> instr.home() :param version: The API version to use. This must be lower than - ``opentrons.protocol_api.MAX_SUPPORTED_VERSION``. - It may be specified either as a string (``'2.0'``) or - as a ``protocols.types.APIVersion`` - (``APIVersion(2, 0)``). + ``opentrons.protocol_api.MAX_SUPPORTED_VERSION``. + It may be specified either as a string (``'2.0'``) or + as a ``protocols.types.APIVersion`` + (``APIVersion(2, 0)``). :param bundled_labware: If specified, a mapping from labware names to - labware definitions for labware to consider in the - protocol. Note that if you specify this, _only_ - labware in this argument will be allowed in the - protocol. This is preparation for a beta feature - and is best not used. + labware definitions for labware to consider in the + protocol. Note that if you specify this, *only* + labware in this argument will be allowed in the + protocol. This is preparation for a beta feature + and is best not used. :param bundled_data: If specified, a mapping from filenames to contents - for data to be available in the protocol from - :py:obj:`opentrons.protocol_api.ProtocolContext.bundled_data`. + for data to be available in the protocol from + :py:obj:`opentrons.protocol_api.ProtocolContext.bundled_data`. :param extra_labware: A mapping from labware load names to custom labware definitions. - If this is ``None`` (the default), and this function is called on a robot, - it will look for labware in the ``labware`` subdirectory of the Jupyter - data directory. - :param hardware_simulator: If specified, a hardware simulator instance. + If this is ``None`` (the default), and this function is called on a robot, + it will look for labware in the ``labware`` subdirectory of the Jupyter + data directory. + :param hardware_simulator: This is only for internal use by Opentrons. If specified, + it's a hardware simulator instance to reuse instead of creating a fresh one. :param robot_type: The type of robot to simulate: either ``"Flex"`` or ``"OT-2"``. - If you're running this function on a robot, the default is the type of that - robot. Otherwise, the default is ``"OT-2"``, for backwards compatibility. - :param use_virtual_hardware: If true, use the protocol engines virtual hardware, if false use the lower level hardware simulator. + If you're running this function on a robot, the default is the type of that + robot. Otherwise, the default is ``"OT-2"``, for backwards compatibility. + :param use_virtual_hardware: This is only for internal use by Opentrons. + If ``True``, use the Protocol Engine's virtual hardware. If ``False``, use the + lower level hardware simulator. :return: The protocol context. """ if isinstance(version, str): @@ -321,12 +323,18 @@ def get_protocol_api( hardware_api=checked_hardware, bundled_data=bundled_data, extra_labware=extra_labware, - use_virtual_hardware=use_virtual_hardware, + use_pe_virtual_hardware=use_virtual_hardware, ) # Intentional difference from execute.get_protocol_api(): # For the caller's convenience, we home the virtual hardware so they don't get MustHomeErrors. # Since this hardware is virtual, there's no harm in commanding this "movement" implicitly. + # + # Calling `checked_hardware_sync.home()` is a hack. It ought to be redundant with + # `context.home()`. We need it here to work around a Protocol Engine simulation bug + # where both the `HardwareControlAPI` level and the `ProtocolEngine` level need to + # be homed for certain commands to work. https://opentrons.atlassian.net/browse/EXEC-646 + checked_hardware.sync.home() context.home() return context @@ -435,15 +443,15 @@ def simulate( """ Simulate the protocol itself. - This is a one-stop function to simulate a protocol, whether python or json, - no matter the api version, from external (i.e. not bound up in other + This is a one-stop function to simulate a protocol, whether Python or JSON, + no matter the API version, from external (i.e. not bound up in other internal server infrastructure) sources. - To simulate an opentrons protocol from other places, pass in a file like - object as protocol_file; this function either returns (if the simulation + To simulate an opentrons protocol from other places, pass in a file-like + object as ``protocol_file``; this function either returns (if the simulation has no problems) or raises an exception. - To call from the command line use either the autogenerated entrypoint + To call from the command line, use either the autogenerated entrypoint ``opentrons_simulate`` (``opentrons_simulate.exe``, on windows) or ``python -m opentrons.simulate``. @@ -474,36 +482,37 @@ def simulate( :param protocol_file: The protocol file to simulate. :param file_name: The name of the file :param custom_labware_paths: A list of directories to search for custom labware. - Loads valid labware from these paths and makes them available - to the protocol context. If this is ``None`` (the default), and - this function is called on a robot, it will look in the ``labware`` - subdirectory of the Jupyter data directory. + Loads valid labware from these paths and makes them available + to the protocol context. If this is ``None`` (the default), and + this function is called on a robot, it will look in the ``labware`` + subdirectory of the Jupyter data directory. :param custom_data_paths: A list of directories or files to load custom - data files from. Ignored if the apiv2 feature - flag if not set. Entries may be either files or - directories. Specified files and the - non-recursive contents of specified directories - are presented by the protocol context in - ``protocol_api.ProtocolContext.bundled_data``. - :param hardware_simulator_file_path: A path to a JSON file defining a - hardware simulator. + data files from. Ignored if the apiv2 feature + flag if not set. Entries may be either files or + directories. Specified files and the + non-recursive contents of specified directories + are presented by the protocol context in + ``protocol_api.ProtocolContext.bundled_data``. + :param hardware_simulator_file_path: A path to a JSON file defining the simulated + hardware. This is mainly for internal use by Opentrons, and is not necessary + to simulate protocols. :param duration_estimator: For internal use only. - Optional duration estimator object. + Optional duration estimator object. :param propagate_logs: Whether this function should allow logs from the - Opentrons stack to propagate up to the root handler. - This can be useful if you're integrating this - function in a larger application, but most logs that - occur during protocol simulation are best associated - with the actions in the protocol that cause them. - Default: ``False`` + Opentrons stack to propagate up to the root handler. + This can be useful if you're integrating this + function in a larger application, but most logs that + occur during protocol simulation are best associated + with the actions in the protocol that cause them. + Default: ``False`` :param log_level: The level of logs to capture in the run log: - ``"debug"``, ``"info"``, ``"warning"``, or ``"error"``. - Defaults to ``"warning"``. + ``"debug"``, ``"info"``, ``"warning"``, or ``"error"``. + Defaults to ``"warning"``. :returns: A tuple of a run log for user output, and possibly the required - data to write to a bundle to bundle this protocol. The bundle is - only emitted if bundling is allowed - and this is an unbundled Protocol API - v2 python protocol. In other cases it is None. + data to write to a bundle to bundle this protocol. The bundle is + only emitted if bundling is allowed + and this is an unbundled Protocol API + v2 python protocol. In other cases it is None. """ stack_logger = logging.getLogger("opentrons") stack_logger.propagate = propagate_logs @@ -636,8 +645,7 @@ def get_arguments(parser: argparse.ArgumentParser) -> argparse.ArgumentParser: Useful if you want to use this module as a component of another CLI program and want to add its arguments. - :param parser: A parser to add arguments to. If not specified, one will be - created. + :param parser: A parser to add arguments to. If not specified, one will be created. :returns argparse.ArgumentParser: The parser with arguments added. """ parser.add_argument( @@ -794,7 +802,7 @@ def _create_live_context_pe( deck_type: str, extra_labware: Dict[str, "LabwareDefinitionDict"], bundled_data: Optional[Dict[str, bytes]], - use_virtual_hardware: bool = True, + use_pe_virtual_hardware: bool = True, ) -> ProtocolContext: """Return a live ProtocolContext that controls the robot through ProtocolEngine.""" assert api_version >= ENGINE_CORE_API_VERSION @@ -804,7 +812,7 @@ def _create_live_context_pe( create_protocol_engine_in_thread( hardware_api=hardware_api_wrapped, config=_get_protocol_engine_config( - robot_type, virtual=use_virtual_hardware + robot_type, use_pe_virtual_hardware=use_pe_virtual_hardware ), error_recovery_policy=error_recovery_policy.never_recover, drop_tips_after_run=False, @@ -910,7 +918,9 @@ async def run(protocol_source: ProtocolSource) -> _SimulateResult: hardware_api_wrapped = hardware_api.wrapped() protocol_engine = await create_protocol_engine( hardware_api=hardware_api_wrapped, - config=_get_protocol_engine_config(robot_type, virtual=True), + config=_get_protocol_engine_config( + robot_type, use_pe_virtual_hardware=True + ), error_recovery_policy=error_recovery_policy.never_recover, load_fixed_trash=should_load_fixed_trash(protocol_source.config), ) @@ -936,6 +946,13 @@ async def run(protocol_source: ProtocolSource) -> _SimulateResult: ), ) + # TODO(mm, 2024-08-06): This home is theoretically redundant with Protocol + # Engine `home` commands within the `RunOrchestrator`. However, we need this to + # work around Protocol Engine bugs where both the `HardwareControlAPI` level + # and the `ProtocolEngine` level need to be homed for certain commands to work. + # https://opentrons.atlassian.net/browse/EXEC-646 + await hardware_api_wrapped.home() + scraper = _CommandScraper(stack_logger, log_level, protocol_runner.broker) with scraper.scrape(): result = await orchestrator.run( @@ -961,15 +978,17 @@ async def run(protocol_source: ProtocolSource) -> _SimulateResult: return asyncio.run(run(protocol_source)) -def _get_protocol_engine_config(robot_type: RobotType, virtual: bool) -> Config: +def _get_protocol_engine_config( + robot_type: RobotType, use_pe_virtual_hardware: bool +) -> Config: """Return a Protocol Engine config to execute protocols on this device.""" return Config( robot_type=robot_type, deck_type=DeckType(deck_type_for_simulation(robot_type)), ignore_pause=True, - use_virtual_pipettes=virtual, - use_virtual_modules=virtual, - use_virtual_gripper=virtual, + use_virtual_pipettes=use_pe_virtual_hardware, + use_virtual_modules=use_pe_virtual_hardware, + use_virtual_gripper=use_pe_virtual_hardware, use_simulated_deck_config=True, ) @@ -992,7 +1011,6 @@ def main() -> int: parser = get_arguments(parser) args = parser.parse_args() - # Try to migrate api v1 containers if needed # TODO(mm, 2022-12-01): Configure the DurationEstimator with the correct deck type. duration_estimator = DurationEstimator() if args.estimate_duration else None # type: ignore[no-untyped-call] diff --git a/api/tests/opentrons/hardware_control/test_ot3_api.py b/api/tests/opentrons/hardware_control/test_ot3_api.py index 21ab1ad8ef9..a6ae8e870d1 100644 --- a/api/tests/opentrons/hardware_control/test_ot3_api.py +++ b/api/tests/opentrons/hardware_control/test_ot3_api.py @@ -14,11 +14,11 @@ TypedDict, ) from typing_extensions import Literal -from math import copysign +from math import copysign, isclose import pytest import types from decoy import Decoy -from mock import AsyncMock, patch, Mock, PropertyMock, MagicMock +from mock import AsyncMock, patch, Mock, PropertyMock, MagicMock, call from hypothesis import given, strategies, settings, HealthCheck, assume, example from opentrons.calibration_storage.types import CalibrationStatus, SourceType @@ -838,7 +838,7 @@ async def test_liquid_probe( mock_move_to_plunger_bottom.call_count == 2 mock_liquid_probe.assert_called_once_with( mount, - 52, + 46, fake_settings_aspirate.mount_speed, (fake_settings_aspirate.plunger_speed * -1), fake_settings_aspirate.sensor_threshold_pascals, @@ -856,6 +856,183 @@ async def test_liquid_probe( ) # should raise no exceptions +@pytest.mark.parametrize( + "mount, head_node, pipette_node", + [ + (OT3Mount.LEFT, NodeId.head_l, NodeId.pipette_left), + (OT3Mount.RIGHT, NodeId.head_r, NodeId.pipette_right), + ], +) +async def test_liquid_probe_plunger_moves( + mock_move_to: AsyncMock, + ot3_hardware: ThreadManager[OT3API], + hardware_backend: OT3Simulator, + head_node: NodeId, + pipette_node: Axis, + mount: OT3Mount, + fake_liquid_settings: LiquidProbeSettings, + mock_current_position_ot3: AsyncMock, + mock_move_to_plunger_bottom: AsyncMock, + mock_gantry_position: AsyncMock, +) -> None: + """Verify the plunger moves in liquid_probe.""" + # This test verifies that both: + # - the plunger movements in each liquid probe pass are what we expect + # - liquid probe successfully chooses the correct distance to move + # when approaching its max z distance + instr_data = AttachedPipette( + config=load_pipette_data.load_definition( + PipetteModelType("p1000"), PipetteChannelType(1), PipetteVersionType(3, 4) + ), + id="fakepip", + ) + await ot3_hardware.cache_pipette(mount, instr_data, None) + pipette = ot3_hardware.hardware_pipettes[mount.to_mount()] + + assert pipette + await ot3_hardware.add_tip(mount, 100) + await ot3_hardware.home() + mock_move_to.return_value = None + + with patch.object( + hardware_backend, "liquid_probe", AsyncMock(spec=hardware_backend.liquid_probe) + ) as mock_liquid_probe: + + mock_liquid_probe.side_effect = [ + PipetteLiquidNotFoundError, + PipetteLiquidNotFoundError, + PipetteLiquidNotFoundError, + PipetteLiquidNotFoundError, + None, + ] + + fake_max_z_dist = 75.0 + config = ot3_hardware.config.liquid_sense + mount_speed = config.mount_speed + non_responsive_z_mm = ot3_hardware.liquid_probe_non_responsive_z_distance( + mount_speed + ) + + probe_pass_overlap = 0.1 + probe_pass_z_offset_mm = non_responsive_z_mm + probe_pass_overlap + probe_safe_reset_mm = max(2.0, probe_pass_z_offset_mm) + + # simulate multiple passes of liquid probe + mock_gantry_position.side_effect = [ + Point(x=0, y=0, z=100), + Point(x=0, y=0, z=100), + Point(x=0, y=0, z=100), + Point(x=0, y=0, z=82.15), + Point(x=0, y=0, z=64.3), + Point(x=0, y=0, z=46.45), + Point(x=0, y=0, z=28.6), + Point(x=0, y=0, z=25), + ] + probe_start_pos = await ot3_hardware.gantry_position(mount) + safe_plunger_pos = Point( + probe_start_pos.x, + probe_start_pos.y, + probe_start_pos.z + probe_safe_reset_mm, + ) + + p_impulse_mm = config.plunger_impulse_time * config.plunger_speed + p_total_mm = pipette.plunger_positions.bottom - pipette.plunger_positions.top + p_working_mm = p_total_mm - (pipette.backlash_distance + p_impulse_mm) + + max_z_time = ( + fake_max_z_dist - (probe_start_pos.z - safe_plunger_pos.z) + ) / config.mount_speed + p_travel_required_for_z = max_z_time * config.plunger_speed + await ot3_hardware.liquid_probe(mount, fake_max_z_dist) + + max_z_distance = fake_max_z_dist + # simulate multiple passes of liquid_probe plunger moves + for _pass in mock_liquid_probe.call_args_list: + plunger_move = _pass[0][1] + expected_plunger_move = ( + min(p_travel_required_for_z, p_working_mm) + p_impulse_mm + ) + assert isclose(plunger_move, expected_plunger_move) + + mount_travel_time = plunger_move / config.plunger_speed + mount_travel_distance = mount_speed * mount_travel_time + max_z_distance -= mount_travel_distance + + move_mount_z_time = (max_z_distance + probe_safe_reset_mm) / mount_speed + p_travel_required_for_z = move_mount_z_time * config.plunger_speed + + +@pytest.mark.parametrize( + "mount, head_node, pipette_node", + [ + (OT3Mount.LEFT, NodeId.head_l, NodeId.pipette_left), + (OT3Mount.RIGHT, NodeId.head_r, NodeId.pipette_right), + ], +) +async def test_liquid_probe_mount_moves( + mock_move_to: AsyncMock, + ot3_hardware: ThreadManager[OT3API], + hardware_backend: OT3Simulator, + head_node: NodeId, + pipette_node: Axis, + mount: OT3Mount, + fake_liquid_settings: LiquidProbeSettings, + mock_current_position_ot3: AsyncMock, + mock_move_to_plunger_bottom: AsyncMock, + mock_gantry_position: AsyncMock, +) -> None: + """Verify move targets for one singular liquid pass probe.""" + instr_data = AttachedPipette( + config=load_pipette_data.load_definition( + PipetteModelType("p1000"), PipetteChannelType(1), PipetteVersionType(3, 4) + ), + id="fakepip", + ) + await ot3_hardware.cache_pipette(mount, instr_data, None) + pipette = ot3_hardware.hardware_pipettes[mount.to_mount()] + + assert pipette + await ot3_hardware.add_tip(mount, 100) + await ot3_hardware.home() + mock_move_to.return_value = None + + with patch.object( + hardware_backend, "liquid_probe", AsyncMock(spec=hardware_backend.liquid_probe) + ): + + fake_max_z_dist = 10.0 + config = ot3_hardware.config.liquid_sense + mount_speed = config.mount_speed + non_responsive_z_mm = ot3_hardware.liquid_probe_non_responsive_z_distance( + mount_speed + ) + + probe_pass_overlap = 0.1 + probe_pass_z_offset_mm = non_responsive_z_mm + probe_pass_overlap + probe_safe_reset_mm = max(2.0, probe_pass_z_offset_mm) + + mock_gantry_position.return_value = Point(x=0, y=0, z=100) + probe_start_pos = await ot3_hardware.gantry_position(mount) + safe_plunger_pos = Point( + probe_start_pos.x, + probe_start_pos.y, + probe_start_pos.z + probe_safe_reset_mm, + ) + pass_start_pos = Point( + probe_start_pos.x, + probe_start_pos.y, + probe_start_pos.z + probe_pass_z_offset_mm, + ) + await ot3_hardware.liquid_probe(mount, fake_max_z_dist) + expected_moves = [ + call(mount, safe_plunger_pos), + call(mount, pass_start_pos), + call(mount, Point(z=probe_start_pos.z + 2)), + call(mount, probe_start_pos), + ] + assert mock_move_to.call_args_list == expected_moves + + async def test_multi_liquid_probe( mock_move_to: AsyncMock, ot3_hardware: ThreadManager[OT3API], @@ -990,7 +1167,7 @@ async def _fake_pos_update_and_raise( OT3Mount.LEFT, fake_max_z_dist, fake_settings_aspirate ) # assert that it went through 4 passes and then prepared to aspirate - assert mock_move_to_plunger_bottom.call_count == 5 + assert mock_move_to_plunger_bottom.call_count == 4 @pytest.mark.parametrize( diff --git a/api/tests/opentrons/protocol_api/test_protocol_context.py b/api/tests/opentrons/protocol_api/test_protocol_context.py index 6674e228b2d..1e1dda706c6 100644 --- a/api/tests/opentrons/protocol_api/test_protocol_context.py +++ b/api/tests/opentrons/protocol_api/test_protocol_context.py @@ -1153,7 +1153,7 @@ def test_home( decoy.verify(mock_core.home(), times=1) -def test_add_liquid( +def test_define_liquid( decoy: Decoy, mock_core: ProtocolCore, subject: ProtocolContext ) -> None: """It should add a liquid to the state.""" @@ -1177,6 +1177,43 @@ def test_add_liquid( assert result == expected_result +@pytest.mark.parametrize( + ("api_version", "expect_success"), + [ + (APIVersion(2, 19), False), + (APIVersion(2, 20), True), + ], +) +def test_define_liquid_arg_defaulting( + expect_success: bool, + decoy: Decoy, + mock_core: ProtocolCore, + subject: ProtocolContext, +) -> None: + """Test API version dependent behavior for missing description and display_color.""" + success_result = Liquid( + _id="water-id", name="water", description=None, display_color=None + ) + decoy.when( + mock_core.define_liquid(name="water", description=None, display_color=None) + ).then_return(success_result) + + if expect_success: + assert ( + subject.define_liquid( + name="water" + # description and display_color omitted. + ) + == success_result + ) + else: + with pytest.raises(APIVersionError): + subject.define_liquid( + name="water" + # description and display_color omitted. + ) + + def test_bundled_data( decoy: Decoy, mock_core_map: LoadedCoreMap, mock_deck: Deck, mock_core: ProtocolCore ) -> None: diff --git a/api/tests/opentrons/test_simulate.py b/api/tests/opentrons/test_simulate.py index 6750bf850b0..6d5c96fc49c 100644 --- a/api/tests/opentrons/test_simulate.py +++ b/api/tests/opentrons/test_simulate.py @@ -296,6 +296,45 @@ def test_get_protocol_api_usable_without_homing(api_version: APIVersion) -> None pipette.pick_up_tip(tip_rack["A1"]) # Should not raise. +def test_liquid_probe_get_protocol_api() -> None: + """Covers `simulate.get_protocol_api()`-specific issues with liquid probes. + + See https://opentrons.atlassian.net/browse/EXEC-646. + """ + protocol = simulate.get_protocol_api(version="2.20", robot_type="Flex") + pipette = protocol.load_instrument("flex_1channel_1000", mount="left") + tip_rack = protocol.load_labware("opentrons_flex_96_tiprack_1000ul", "A1") + well_plate = protocol.load_labware( + "opentrons_96_wellplate_200ul_pcr_full_skirt", "A2" + ) + pipette.pick_up_tip(tip_rack["A1"]) + pipette.require_liquid_presence(well_plate["A1"]) # Should not raise MustHomeError. + + +def test_liquid_probe_simulate_file() -> None: + """Covers `opentrons_simulate`-specific issues with liquid probes. + + See https://opentrons.atlassian.net/browse/EXEC-646. + """ + protocol_contents = textwrap.dedent( + """\ + requirements = {"robotType": "Flex", "apiLevel": "2.20"} + def run(protocol): + pipette = protocol.load_instrument("flex_1channel_1000", mount="left") + tip_rack = protocol.load_labware("opentrons_flex_96_tiprack_1000ul", "A1") + well_plate = protocol.load_labware( + "opentrons_96_wellplate_200ul_pcr_full_skirt", "A2" + ) + pipette.pick_up_tip(tip_rack["A1"]) + pipette.require_liquid_presence(well_plate["A1"]) + """ + ) + protocol_contents_stream = io.StringIO(protocol_contents) + simulate.simulate( + protocol_file=protocol_contents_stream + ) # Should not raise MustHomeError. + + class TestGetProtocolAPILabware: """Tests for making sure get_protocol_api() handles extra labware correctly.""" diff --git a/app/src/assets/images/labware/opentrons_flex_96_tiprack_adapter.png b/app/src/assets/images/labware/opentrons_flex_96_tiprack_adapter.png new file mode 100644 index 00000000000..28a65ff766b Binary files /dev/null and b/app/src/assets/images/labware/opentrons_flex_96_tiprack_adapter.png differ diff --git a/app/src/assets/localization/en/app_settings.json b/app/src/assets/localization/en/app_settings.json index 41a6923112c..adbc00d3181 100644 --- a/app/src/assets/localization/en/app_settings.json +++ b/app/src/assets/localization/en/app_settings.json @@ -3,7 +3,6 @@ "__dev_internal__protocolStats": "Protocol Stats", "__dev_internal__protocolTimeline": "Protocol Timeline", "__dev_internal__enableRunNotes": "Display Notes During a Protocol Run", - "__dev_internal__enableQuickTransfer": "Enable Quick Transfer", "__dev_internal__enableLabwareCreator": "Enable App Labware Creator", "add_folder_button": "Add labware source folder", "add_ip_button": "Add", diff --git a/app/src/assets/localization/en/protocol_setup.json b/app/src/assets/localization/en/protocol_setup.json index 5217a95dc4c..2a21ab64da1 100644 --- a/app/src/assets/localization/en/protocol_setup.json +++ b/app/src/assets/localization/en/protocol_setup.json @@ -8,6 +8,8 @@ "add_to_slot": "Add to slot {{slotName}}", "additional_labware": "{{count}} additional labware", "additional_off_deck_labware": "Additional Off-Deck Labware", + "applied_labware_offsets": "applied labware offsets", + "are_you_sure_you_want_to_proceed": "Are you sure you want to proceed to run?", "attach_gripper_failure_reason": "Attach the required gripper to continue", "attach_gripper": "attach gripper", "attach_module": "Attach module before calibrating", @@ -47,6 +49,9 @@ "configured": "configured", "confirm_heater_shaker_module_modal_description": "Before the run begins, module should have both anchors fully extended for a firm attachment. The thermal adapter should be attached to the module. ", "confirm_heater_shaker_module_modal_title": "Confirm Heater-Shaker Module is attached", + "confirm_offsets": "Confirm offsets", + "confirm_liquids": "Confirm liquids", + "confirm_placements": "Confirm placements", "confirm_selection": "Confirm selection", "confirm_values": "Confirm values", "connect_all_hardware": "Connect and calibrate all hardware first", @@ -101,6 +106,7 @@ "labware_latch": "Labware Latch", "labware_location": "Labware Location", "labware_name": "Labware name", + "labware_placement": "labware placement", "labware_position_check_not_available_analyzing_on_robot": "Labware Position Check is not available while protocol is analyzing on robot", "labware_position_check_not_available_empty_protocol": "Labware Position Check requires that the protocol loads labware and pipettes", "labware_position_check_not_available": "Labware Position Check is not available after run has started", @@ -118,11 +124,13 @@ "learn_more": "Learn more", "liquid_information": "Liquid information", "liquid_name": "Liquid name", + "liquids": "liquids", "liquid_setup_step_description": "View liquid starting locations and volumes", "liquid_setup_step_title": "Liquids", "liquids_not_in_setup": "No liquids used in this protocol", "liquids_not_in_the_protocol": "no liquids are specified for this protocol.", - "liquids": "Liquids", + "liquids_ready": "Liquids ready", + "liquids_confirmed": "Liquids confirmed", "list_view": "List View", "loading_data": "Loading data...", "loading_labware_offsets": "Loading labware offsets", @@ -149,6 +157,7 @@ "module_name": "Module", "module_not_connected": "Not connected", "module_setup_step_title": "Deck hardware", + "module_setup_step_ready": "Calibration ready", "module_slot_location": "Slot {{slotName}}, {{moduleName}}", "module": "Module", "modules_connected_plural": "{{count}} modules attached", @@ -191,6 +200,7 @@ "offset_data": "Offset Data", "offsets_applied_plural": "{{count}} offsets applied", "offsets_applied": "{{count}} offset applied", + "offsets_ready": "Offsets ready", "on_adapter_in_mod": "on {{adapterName}} in {{moduleName}}", "on_adapter": "on {{adapterName}}", "on_deck": "On deck", @@ -206,6 +216,8 @@ "pipette_offset_cal_description": "This measures a pipette’s X, Y and Z values in relation to the pipette mount and the deck. Pipette Offset Calibration relies on Deck Calibration and Tip Length Calibration. ", "pipette_offset_cal": "Pipette Offset Calibration", "placement": "Placement", + "placements_ready": "Placements ready", + "placements_confirmed": "Placements confirmed", "plug_in_module_to_configure": "Plug in a {{module}} to add it to the slot", "plug_in_required_module_plural": "Plug in and power up the required modules to continue", "plug_in_required_module": "Plug in and power up the required module to continue", @@ -246,6 +258,7 @@ "robot_calibration_step_description_pipettes_only": "Review required instruments and calibrations for this protocol.", "robot_calibration_step_description": "Review required pipettes and tip length calibrations for this protocol.", "robot_calibration_step_title": "Instruments", + "robot_calibration_step_ready": "Calibration ready", "run_disabled_calibration_not_complete": "Make sure robot calibration is complete before proceeding to run", "run_disabled_modules_and_calibration_not_complete": "Make sure robot calibration is complete and all modules are connected before proceeding to run", "run_disabled_modules_not_connected": "Make sure all modules are connected before proceeding to run", @@ -260,6 +273,8 @@ "setup_is_view_only": "Setup is view-only once run has started", "slot_location": "Slot {{slotName}}", "slot_number": "Slot Number", + "stacked_slot": "Stacked slot", + "start_run": "Start run", "status": "Status", "step": "STEP {{index}}", "there_are_no_unconfigured_modules": "No {{module}} is connected. Attach one and place it in {{slot}}.", @@ -271,6 +286,7 @@ "total_liquid_volume": "Total volume", "update_deck_config": "Update deck configuration", "update_deck": "Update deck", + "update_offsets": "Update offsets", "updated": "Updated", "usb_connected_no_port_info": "USB Port Connected", "usb_drive_notification": "Leave USB drive attached until run starts", @@ -286,5 +302,6 @@ "view_setup_instructions": "View setup instructions", "volume": "Volume", "what_labware_offset_is": "A Labware Offset is a type of positional adjustment that accounts for small, real-world variances in the overall position of the labware on a robot’s deck. Labware Offset data is unique to a specific combination of labware definition, deck slot, and robot.", - "with_the_chosen_value": "With the chosen values, the following error occurred:" + "with_the_chosen_value": "With the chosen values, the following error occurred:", + "you_havent_confirmed": "You haven't confirmed the {{missingSteps}} yet. Ensure these are correct before proceeding to run the protocol." } diff --git a/app/src/assets/localization/en/quick_transfer.json b/app/src/assets/localization/en/quick_transfer.json index f40298d6ae1..b754376c81a 100644 --- a/app/src/assets/localization/en/quick_transfer.json +++ b/app/src/assets/localization/en/quick_transfer.json @@ -47,6 +47,7 @@ "deleted_transfer": "Deleted quick transfer", "destination": "Destination", "destination_labware": "Destination labware", + "disabled": "Disabled", "dispense_flow_rate": "Dispense flow rate", "dispense_flow_rate_µL": "Dispense flow rate (µL/s)", "dispense_settings": "Dispense Settings", @@ -90,7 +91,8 @@ "pipette_currently_attached": "Quick transfer options depend on the pipettes currently attached to your robot.", "pipette_path": "Pipette path", "pipette_path_multi_aspirate": "Multi-aspirate", - "pipette_path_multi_dispense": "Multi-dispense, {{volume}} disposal volume, blowout into {{blowOutLocation}}", + "pipette_path_multi_dispense": "Multi-dispense", + "pipette_path_multi_dispense_volume_blowout": "Multi-dispense, {{volume}} disposal volume, blowout into {{blowOutLocation}}", "pipette_path_single": "Single transfers", "pre_wet_tip": "Pre-wet tip", "quick_transfer": "Quick transfer", diff --git a/app/src/molecules/LegacyModal/LegacyModalHeader.tsx b/app/src/molecules/LegacyModal/LegacyModalHeader.tsx index 725df7c0c4b..8c884deef80 100644 --- a/app/src/molecules/LegacyModal/LegacyModalHeader.tsx +++ b/app/src/molecules/LegacyModal/LegacyModalHeader.tsx @@ -8,8 +8,8 @@ import { Icon, JUSTIFY_CENTER, JUSTIFY_SPACE_BETWEEN, - SPACING, LegacyStyledText, + SPACING, TYPOGRAPHY, } from '@opentrons/components' @@ -19,6 +19,8 @@ import type { IconProps } from '@opentrons/components' export interface LegacyModalHeaderProps { onClose?: React.MouseEventHandler title: React.ReactNode + titleElement1?: JSX.Element + titleElement2?: JSX.Element backgroundColor?: string color?: string icon?: IconProps @@ -44,7 +46,16 @@ const closeIconStyles = css` export const LegacyModalHeader = ( props: LegacyModalHeaderProps ): JSX.Element => { - const { icon, onClose, title, backgroundColor, color, closeButton } = props + const { + icon, + onClose, + title, + titleElement1, + titleElement2, + backgroundColor, + color, + closeButton, + } = props return ( <> - + {icon != null && } + {titleElement1} + {titleElement2} + {/* TODO (nd: 08/07/2024) Convert to StyledText once designs are resolved */} { childrenPadding = `${SPACING.spacing16} ${SPACING.spacing24} ${SPACING.spacing24}`, children, footer, + titleElement1, + titleElement2, ...styleProps } = props @@ -58,6 +62,8 @@ export const LegacyModal = (props: LegacyModalProps): JSX.Element => { - {truncateString(fileRunTimeParameter?.file?.name ?? '', 35, 18)} + {truncateString(fileRunTimeParameter?.file?.file?.name ?? '', 35, 18)} void + onConfirmClick: () => void + missingSteps: string[] +} +export const ConfirmMissingStepsModal = ( + props: ConfirmMissingStepsModalProps +): JSX.Element | null => { + const { missingSteps, onCloseClick, onConfirmClick } = props + const { t, i18n } = useTranslation(['protocol_setup', 'shared']) + + const confirmAttached = (): void => { + onConfirmClick() + onCloseClick() + } + + return ( + + + + {t('you_havent_confirmed', { + missingSteps: new Intl.ListFormat('en', { + style: 'short', + type: 'conjunction', + }).format(missingSteps.map(step => t(step))), + })} + + + + + {i18n.format(t('shared:go_back'), 'capitalize')} + + + {t('start_run')} + + + + ) +} diff --git a/app/src/organisms/Devices/ProtocolRun/ProtocolRunHeader.tsx b/app/src/organisms/Devices/ProtocolRun/ProtocolRunHeader.tsx index 6a704c96699..5d9821cc5a6 100644 --- a/app/src/organisms/Devices/ProtocolRun/ProtocolRunHeader.tsx +++ b/app/src/organisms/Devices/ProtocolRun/ProtocolRunHeader.tsx @@ -77,6 +77,7 @@ import { } from '../../../organisms/RunTimeControl/hooks' import { useIsHeaterShakerInProtocol } from '../../ModuleCard/hooks' import { ConfirmAttachmentModal } from '../../ModuleCard/ConfirmAttachmentModal' +import { ConfirmMissingStepsModal } from './ConfirmMissingStepsModal' import { useProtocolDetailsForRun, useProtocolAnalysisErrors, @@ -132,6 +133,7 @@ interface ProtocolRunHeaderProps { robotName: string runId: string makeHandleJumpToStep: (index: number) => () => void + missingSetupSteps: string[] } export function ProtocolRunHeader({ @@ -139,6 +141,7 @@ export function ProtocolRunHeader({ robotName, runId, makeHandleJumpToStep, + missingSetupSteps, }: ProtocolRunHeaderProps): JSX.Element | null { const { t } = useTranslation(['run_details', 'shared']) const navigate = useNavigate() @@ -447,6 +450,7 @@ export function ProtocolRunHeader({ isDoorOpen={isDoorOpen} isFixtureMismatch={isFixtureMismatch} isResetRunLoadingRef={isResetRunLoadingRef} + missingSetupSteps={missingSetupSteps} /> @@ -591,6 +595,7 @@ interface ActionButtonProps { isDoorOpen: boolean isFixtureMismatch: boolean isResetRunLoadingRef: React.MutableRefObject + missingSetupSteps: string[] } // TODO(jh, 04-22-2024): Refactor switch cases into separate factories to increase readability and testability. @@ -603,6 +608,7 @@ function ActionButton(props: ActionButtonProps): JSX.Element { isDoorOpen, isFixtureMismatch, isResetRunLoadingRef, + missingSetupSteps, } = props const navigate = useNavigate() const { t } = useTranslation(['run_details', 'shared']) @@ -682,12 +688,20 @@ function ActionButton(props: ActionButtonProps): JSX.Element { ) const { confirm: confirmAttachment, - showConfirmation: showConfirmationModal, - cancel: cancelExit, + showConfirmation: showHSConfirmationModal, + cancel: cancelExitHSConfirmation, } = useConditionalConfirm( handleProceedToRunClick, !configBypassHeaterShakerAttachmentConfirmation ) + const { + confirm: confirmMissingSteps, + showConfirmation: showMissingStepsConfirmationModal, + cancel: cancelExitMissingStepsConfirmation, + } = useConditionalConfirm( + handleProceedToRunClick, + missingSetupSteps.length !== 0 + ) const robotAnalyticsData = useRobotAnalyticsData(robotName) const isHeaterShakerInProtocol = useIsHeaterShakerInProtocol() @@ -745,6 +759,11 @@ function ActionButton(props: ActionButtonProps): JSX.Element { handleButtonClick = () => { if (isHeaterShakerShaking && isHeaterShakerInProtocol) { setShowIsShakingModal(true) + } else if ( + missingSetupSteps.length !== 0 && + (runStatus === RUN_STATUS_IDLE || runStatus === RUN_STATUS_STOPPED) + ) { + confirmMissingSteps() } else if ( isHeaterShakerInProtocol && !isHeaterShakerShaking && @@ -825,13 +844,21 @@ function ActionButton(props: ActionButtonProps): JSX.Element { startRun={play} /> )} - {showConfirmationModal && ( + {showHSConfirmationModal && ( )} + {showMissingStepsConfirmationModal && ( + + )} + {} ) } diff --git a/app/src/organisms/Devices/ProtocolRun/ProtocolRunSetup.tsx b/app/src/organisms/Devices/ProtocolRun/ProtocolRunSetup.tsx index 19c29827c15..7ea1386768d 100644 --- a/app/src/organisms/Devices/ProtocolRun/ProtocolRunSetup.tsx +++ b/app/src/organisms/Devices/ProtocolRun/ProtocolRunSetup.tsx @@ -16,6 +16,7 @@ import { SPACING, LegacyStyledText, TYPOGRAPHY, + FLEX_MAX_CONTENT, } from '@opentrons/components' import { FLEX_ROBOT_TYPE, OT2_ROBOT_TYPE } from '@opentrons/shared-data' @@ -48,8 +49,6 @@ import { SetupLiquids } from './SetupLiquids' import { EmptySetupStep } from './EmptySetupStep' import { HowLPCWorksModal } from './SetupLabwarePositionCheck/HowLPCWorksModal' -import type { ProtocolCalibrationStatus } from '../hooks' - const ROBOT_CALIBRATION_STEP_KEY = 'robot_calibration_step' as const const MODULE_SETUP_KEY = 'module_setup_step' as const const LPC_KEY = 'labware_position_check_step' as const @@ -63,16 +62,33 @@ export type StepKey = | typeof LABWARE_SETUP_KEY | typeof LIQUID_SETUP_KEY +export type MissingStep = + | 'applied_labware_offsets' + | 'labware_placement' + | 'liquids' + +export type MissingSteps = MissingStep[] + +export const initialMissingSteps = (): MissingSteps => [ + 'applied_labware_offsets', + 'labware_placement', + 'liquids', +] + interface ProtocolRunSetupProps { protocolRunHeaderRef: React.RefObject | null robotName: string runId: string + setMissingSteps: (missingSteps: MissingSteps) => void + missingSteps: MissingSteps } export function ProtocolRunSetup({ protocolRunHeaderRef, robotName, runId, + setMissingSteps, + missingSteps, }: ProtocolRunSetupProps): JSX.Element | null { const { t, i18n } = useTranslation('protocol_setup') const robotProtocolAnalysis = useMostRecentCompletedAnalysis(runId) @@ -147,6 +163,15 @@ export function ProtocolRunSetup({ return true }) + const [ + labwareSetupComplete, + setLabwareSetupComplete, + ] = React.useState(false) + const [liquidSetupComplete, setLiquidSetupComplete] = React.useState( + false + ) + const [lpcComplete, setLpcComplete] = React.useState(false) + if (robot == null) return null const liquids = protocolAnalysis?.liquids ?? [] @@ -171,7 +196,11 @@ export function ProtocolRunSetup({ const StepDetailMap: Record< StepKey, - { stepInternals: JSX.Element; description: string } + { + stepInternals: JSX.Element + description: string + rightElProps: StepRightElementProps + } > = { [ROBOT_CALIBRATION_STEP_KEY]: { stepInternals: ( @@ -193,6 +222,15 @@ export function ProtocolRunSetup({ description: isFlex ? t(`${ROBOT_CALIBRATION_STEP_KEY}_description_pipettes_only`) : t(`${ROBOT_CALIBRATION_STEP_KEY}_description`), + rightElProps: { + stepKey: ROBOT_CALIBRATION_STEP_KEY, + complete: calibrationStatusRobot.complete, + completeText: t('calibration_ready'), + missingHardware: isMissingPipette, + incompleteText: t('calibration_needed'), + missingHardwareText: t('action_needed'), + incompleteElement: null, + }, }, [MODULE_SETUP_KEY]: { stepInternals: ( @@ -209,47 +247,99 @@ export function ProtocolRunSetup({ description: isFlex ? flexDeckHardwareDescription : ot2DeckHardwareDescription, + rightElProps: { + stepKey: MODULE_SETUP_KEY, + complete: + calibrationStatusRobot.complete && calibrationStatusModules.complete, + completeText: isFlex ? t('calibration_ready') : '', + incompleteText: isFlex ? t('calibration_needed') : t('action_needed'), + missingHardware: isMissingModule || isFixtureMismatch, + missingHardwareText: t('action_needed'), + incompleteElement: null, + }, }, [LPC_KEY]: { stepInternals: ( { - setExpandedStepKey(LABWARE_SETUP_KEY) + setOffsetsConfirmed={confirmed => { + setLpcComplete(confirmed) + if (confirmed) { + setExpandedStepKey(LABWARE_SETUP_KEY) + setMissingSteps( + missingSteps.filter(step => step !== 'applied_labware_offsets') + ) + } }} + offsetsConfirmed={lpcComplete} /> ), description: t('labware_position_check_step_description'), + rightElProps: { + stepKey: LPC_KEY, + complete: lpcComplete, + completeText: t('offsets_ready'), + incompleteText: null, + incompleteElement: , + }, }, [LABWARE_SETUP_KEY]: { stepInternals: ( v === LABWARE_SETUP_KEY) === - targetStepKeyInOrder.length - 1 - ? null - : LIQUID_SETUP_KEY - } - expandStep={setExpandedStepKey} + labwareConfirmed={labwareSetupComplete} + setLabwareConfirmed={(confirmed: boolean) => { + setLabwareSetupComplete(confirmed) + if (confirmed) { + setMissingSteps( + missingSteps.filter(step => step !== 'labware_placement') + ) + const nextStep = + targetStepKeyInOrder.findIndex(v => v === LABWARE_SETUP_KEY) === + targetStepKeyInOrder.length - 1 + ? null + : LIQUID_SETUP_KEY + setExpandedStepKey(nextStep) + } + }} /> ), description: t(`${LABWARE_SETUP_KEY}_description`), + rightElProps: { + stepKey: LABWARE_SETUP_KEY, + complete: labwareSetupComplete, + completeText: t('placements_ready'), + incompleteText: null, + incompleteElement: null, + }, }, [LIQUID_SETUP_KEY]: { stepInternals: ( { + setLiquidSetupComplete(confirmed) + if (confirmed) { + setMissingSteps(missingSteps.filter(step => step !== 'liquids')) + setExpandedStepKey(null) + } + }} /> ), description: hasLiquids ? t(`${LIQUID_SETUP_KEY}_description`) : i18n.format(t('liquids_not_in_the_protocol'), 'capitalize'), + rightElProps: { + stepKey: LIQUID_SETUP_KEY, + complete: liquidSetupComplete, + completeText: t('liquids_ready'), + incompleteText: null, + incompleteElement: null, + }, }, } @@ -295,17 +385,7 @@ export function ProtocolRunSetup({ }} rightElement={ } > @@ -329,81 +409,110 @@ export function ProtocolRunSetup({ ) } -interface StepRightElementProps { - stepKey: StepKey - calibrationStatusRobot: ProtocolCalibrationStatus - calibrationStatusModules?: ProtocolCalibrationStatus - runHasStarted: boolean - isFlex: boolean - isMissingModule: boolean - isFixtureMismatch: boolean - isMissingPipette: boolean +interface NoHardwareRequiredStepCompletion { + stepKey: Exclude< + StepKey, + typeof ROBOT_CALIBRATION_STEP_KEY | typeof MODULE_SETUP_KEY + > + complete: boolean + incompleteText: string | null + incompleteElement: JSX.Element | null + completeText: string +} + +interface HardwareRequiredStepCompletion { + stepKey: typeof ROBOT_CALIBRATION_STEP_KEY | typeof MODULE_SETUP_KEY + complete: boolean + missingHardware: boolean + incompleteText: string | null + incompleteElement: JSX.Element | null + completeText: string + missingHardwareText: string } -function StepRightElement(props: StepRightElementProps): JSX.Element | null { - const { - stepKey, - runHasStarted, - calibrationStatusRobot, - calibrationStatusModules, - isFlex, - isMissingModule, - isFixtureMismatch, - isMissingPipette, - } = props - const { t } = useTranslation('protocol_setup') - const isActionNeeded = isMissingModule || isFixtureMismatch - if ( - !runHasStarted && - (stepKey === ROBOT_CALIBRATION_STEP_KEY || stepKey === MODULE_SETUP_KEY) - ) { - const moduleAndDeckStatus = isActionNeeded - ? { complete: false } - : calibrationStatusModules - const calibrationStatus = - stepKey === ROBOT_CALIBRATION_STEP_KEY - ? calibrationStatusRobot - : moduleAndDeckStatus +type StepRightElementProps = + | NoHardwareRequiredStepCompletion + | HardwareRequiredStepCompletion - let statusText = t('calibration_ready') - if ( - stepKey === ROBOT_CALIBRATION_STEP_KEY && - !calibrationStatusRobot.complete - ) { - statusText = isMissingPipette - ? t('action_needed') - : t('calibration_needed') - } else if (stepKey === MODULE_SETUP_KEY && !calibrationStatus?.complete) { - statusText = isActionNeeded ? t('action_needed') : t('calibration_needed') - } +const stepRequiresHW = ( + props: StepRightElementProps +): props is HardwareRequiredStepCompletion => + props.stepKey === ROBOT_CALIBRATION_STEP_KEY || + props.stepKey === MODULE_SETUP_KEY - // do not render calibration ready status icon for OT-2 module setup - return isFlex || - !( - stepKey === MODULE_SETUP_KEY && statusText === t('calibration_ready') - ) ? ( +function StepRightElement(props: StepRightElementProps): JSX.Element | null { + if (props.complete) { + return ( + + + + {props.completeText} + + + ) + } else if (stepRequiresHW(props) && props.missingHardware) { + return ( + + + + {props.missingHardwareText} + + + ) + } else if (props.incompleteText != null) { + return ( - {statusText} + {props.incompleteText} - ) : null - } else if (stepKey === LPC_KEY) { - return + ) + } else if (props.incompleteElement != null) { + return props.incompleteElement } else { return null } diff --git a/app/src/organisms/Devices/ProtocolRun/SetupLabware/LabwareStackModal.tsx b/app/src/organisms/Devices/ProtocolRun/SetupLabware/LabwareStackModal.tsx new file mode 100644 index 00000000000..b65a8b38eb4 --- /dev/null +++ b/app/src/organisms/Devices/ProtocolRun/SetupLabware/LabwareStackModal.tsx @@ -0,0 +1,257 @@ +import * as React from 'react' +import { useTranslation } from 'react-i18next' +import { useSelector } from 'react-redux' +import { css } from 'styled-components' +import { + ALIGN_CENTER, + Box, + COLORS, + DeckInfoLabel, + DIRECTION_COLUMN, + Flex, + JUSTIFY_CENTER, + JUSTIFY_SPACE_BETWEEN, + LabwareStackRender, + SPACING, + StyledText, +} from '@opentrons/components' +import { Modal } from '../../../../molecules/Modal' +import { getIsOnDevice } from '../../../../redux/config' +import { useMostRecentCompletedAnalysis } from '../../../LabwarePositionCheck/useMostRecentCompletedAnalysis' +import { LegacyModal } from '../../../../molecules/LegacyModal' +import { getLocationInfoNames } from '../utils/getLocationInfoNames' +import { getSlotLabwareDefinition } from '../utils/getSlotLabwareDefinition' +import { Divider } from '../../../../atoms/structure' +import { getModuleImage } from '../SetupModuleAndDeck/utils' +import { getModuleDisplayName } from '@opentrons/shared-data' +import tiprackAdapter from '../../../../assets/images/labware/opentrons_flex_96_tiprack_adapter.png' + +const HIDE_SCROLLBAR = css` + ::-webkit-scrollbar { + display: none; + } +` + +interface LabwareStackModalProps { + labwareIdTop: string + runId: string + closeModal: () => void +} + +export const LabwareStackModal = ( + props: LabwareStackModalProps +): JSX.Element | null => { + const { labwareIdTop, runId, closeModal } = props + const { t } = useTranslation('protocol_setup') + const isOnDevice = useSelector(getIsOnDevice) + const protocolData = useMostRecentCompletedAnalysis(runId) + if (protocolData == null) { + return null + } + const commands = protocolData?.commands ?? [] + const { + slotName, + adapterName, + adapterId, + moduleModel, + labwareName, + labwareNickname, + } = getLocationInfoNames(labwareIdTop, commands) + + const topDefinition = getSlotLabwareDefinition(labwareIdTop, commands) + const adapterDef = getSlotLabwareDefinition(adapterId ?? '', commands) + const moduleDisplayName = + moduleModel != null ? getModuleDisplayName(moduleModel) : null ?? '' + const tiprackAdapterImg = ( + + ) + const moduleImg = + moduleModel != null ? ( + + ) : null + + return isOnDevice ? ( + + + + + ), + onClick: closeModal, + }} + > + + <> + + + + + + + {adapterDef != null ? ( + <> + + + {adapterDef.parameters.loadName === + 'opentrons_flex_96_tiprack_adapter' ? ( + tiprackAdapterImg + ) : ( + + )} + + {moduleModel != null ? ( + + ) : null} + + ) : null} + {moduleModel != null ? ( + + + {moduleImg} + + ) : null} + + + ) : ( + + + + {t('stacked_slot')} + + } + childrenPadding={0} + marginLeft="0" + > + + + <> + + + + + + + {adapterDef != null ? ( + <> + + + + + + + ) : null} + {moduleModel != null ? ( + + + {moduleImg} + + ) : null} + + + + ) +} + +interface LabwareStackLabelProps { + text: string + subText?: string + isOnDevice?: boolean +} +function LabwareStackLabel(props: LabwareStackLabelProps): JSX.Element { + const { text, subText, isOnDevice = false } = props + return isOnDevice ? ( + + {text} + {subText != null ? ( + + {subText} + + ) : null} + + ) : ( + + {text} + {subText != null ? ( + + {subText} + + ) : null} + + ) +} diff --git a/app/src/organisms/Devices/ProtocolRun/SetupLabware/SetupLabwareMap.tsx b/app/src/organisms/Devices/ProtocolRun/SetupLabware/SetupLabwareMap.tsx index 533f134590d..ae8f3bbea02 100644 --- a/app/src/organisms/Devices/ProtocolRun/SetupLabware/SetupLabwareMap.tsx +++ b/app/src/organisms/Devices/ProtocolRun/SetupLabware/SetupLabwareMap.tsx @@ -27,6 +27,7 @@ import type { CompletedProtocolAnalysis, ProtocolAnalysisOutput, } from '@opentrons/shared-data' +import { LabwareStackModal } from './LabwareStackModal' interface SetupLabwareMapProps { runId: string @@ -38,6 +39,11 @@ export function SetupLabwareMap({ protocolAnalysis, }: SetupLabwareMapProps): JSX.Element | null { // early return null if no protocol analysis + const [ + labwareStackDetailsLabwareId, + setLabwareStackDetailsLabwareId, + ] = React.useState(null) + if (protocolAnalysis == null) return null const commands = protocolAnalysis.commands @@ -76,7 +82,15 @@ export function SetupLabwareMap({ nestedLabwareDef: topLabwareDefinition, moduleChildren: ( - <> + // open modal + { + if (topLabwareDefinition != null) { + setLabwareStackDetailsLabwareId(topLabwareId) + } + }} + cursor="pointer" + > {topLabwareDefinition != null && topLabwareId != null ? ( ) : null} - + ), } }) @@ -143,6 +157,15 @@ export function SetupLabwareMap({ commands={commands} /> + {labwareStackDetailsLabwareId != null && ( + { + setLabwareStackDetailsLabwareId(null) + }} + /> + )} ) } diff --git a/app/src/organisms/Devices/ProtocolRun/SetupLabware/__tests__/SetupLabware.test.tsx b/app/src/organisms/Devices/ProtocolRun/SetupLabware/__tests__/SetupLabware.test.tsx index d6a6ab4b05e..e92169bcb1d 100644 --- a/app/src/organisms/Devices/ProtocolRun/SetupLabware/__tests__/SetupLabware.test.tsx +++ b/app/src/organisms/Devices/ProtocolRun/SetupLabware/__tests__/SetupLabware.test.tsx @@ -35,14 +35,17 @@ const ROBOT_NAME = 'otie' const RUN_ID = '1' const render = () => { + let labwareConfirmed = false + const confirmLabware = vi.fn(confirmed => { + labwareConfirmed = confirmed + }) return renderWithProviders( , { diff --git a/app/src/organisms/Devices/ProtocolRun/SetupLabware/index.tsx b/app/src/organisms/Devices/ProtocolRun/SetupLabware/index.tsx index 66b7bcdc1bc..526b944f425 100644 --- a/app/src/organisms/Devices/ProtocolRun/SetupLabware/index.tsx +++ b/app/src/organisms/Devices/ProtocolRun/SetupLabware/index.tsx @@ -16,22 +16,18 @@ import { useModuleRenderInfoForProtocolById, useStoredProtocolAnalysis, } from '../../hooks' -import { BackToTopButton } from '../BackToTopButton' import { SetupLabwareMap } from './SetupLabwareMap' import { SetupLabwareList } from './SetupLabwareList' -import type { StepKey } from '../ProtocolRunSetup' - interface SetupLabwareProps { - protocolRunHeaderRef: React.RefObject | null robotName: string runId: string - nextStep: StepKey | null - expandStep: (step: StepKey) => void + labwareConfirmed: boolean + setLabwareConfirmed: (confirmed: boolean) => void } export function SetupLabware(props: SetupLabwareProps): JSX.Element { - const { robotName, runId, nextStep, expandStep, protocolRunHeaderRef } = props + const { robotName, runId, labwareConfirmed, setLabwareConfirmed } = props const { t } = useTranslation('protocol_setup') const robotProtocolAnalysis = useMostRecentCompletedAnalysis(runId) const storedProtocolAnalysis = useStoredProtocolAnalysis(runId) @@ -71,22 +67,14 @@ export function SetupLabware(props: SetupLabwareProps): JSX.Element { )} - {nextStep == null ? ( - - ) : ( - { - expandStep(nextStep) - }} - > - {t('proceed_to_liquid_setup_step')} - - )} + { + setLabwareConfirmed(true) + }} + disabled={labwareConfirmed} + > + {t('confirm_placements')} + ) diff --git a/app/src/organisms/Devices/ProtocolRun/SetupLabwarePositionCheck/__tests__/SetupLabwarePositionCheck.test.tsx b/app/src/organisms/Devices/ProtocolRun/SetupLabwarePositionCheck/__tests__/SetupLabwarePositionCheck.test.tsx index 0bf4aaebbfc..0c0150937ad 100644 --- a/app/src/organisms/Devices/ProtocolRun/SetupLabwarePositionCheck/__tests__/SetupLabwarePositionCheck.test.tsx +++ b/app/src/organisms/Devices/ProtocolRun/SetupLabwarePositionCheck/__tests__/SetupLabwarePositionCheck.test.tsx @@ -42,10 +42,15 @@ const ROBOT_NAME = 'otie' const RUN_ID = '1' const render = () => { + let areOffsetsConfirmed = false + const confirmOffsets = vi.fn((offsetsConfirmed: boolean) => { + areOffsetsConfirmed = offsetsConfirmed + }) return renderWithProviders( diff --git a/app/src/organisms/Devices/ProtocolRun/SetupLabwarePositionCheck/index.tsx b/app/src/organisms/Devices/ProtocolRun/SetupLabwarePositionCheck/index.tsx index 66484717ef0..21862539e35 100644 --- a/app/src/organisms/Devices/ProtocolRun/SetupLabwarePositionCheck/index.tsx +++ b/app/src/organisms/Devices/ProtocolRun/SetupLabwarePositionCheck/index.tsx @@ -32,7 +32,8 @@ import { useNotifyRunQuery } from '../../../../resources/runs' import type { LabwareOffset } from '@opentrons/api-client' interface SetupLabwarePositionCheckProps { - expandLabwareStep: () => void + offsetsConfirmed: boolean + setOffsetsConfirmed: (confirmed: boolean) => void robotName: string runId: string } @@ -40,7 +41,7 @@ interface SetupLabwarePositionCheckProps { export function SetupLabwarePositionCheck( props: SetupLabwarePositionCheckProps ): JSX.Element { - const { robotName, runId, expandLabwareStep } = props + const { robotName, runId, setOffsetsConfirmed, offsetsConfirmed } = props const { t, i18n } = useTranslation('protocol_setup') const robotType = useRobotType(robotName) @@ -75,7 +76,13 @@ export function SetupLabwarePositionCheck( const robotProtocolAnalysis = useMostRecentCompletedAnalysis(runId) const storedProtocolAnalysis = useStoredProtocolAnalysis(runId) const protocolData = robotProtocolAnalysis ?? storedProtocolAnalysis - const [targetProps, tooltipProps] = useHoverTooltip({ + const [runLPCTargetProps, runLPCTooltipProps] = useHoverTooltip({ + placement: TOOLTIP_LEFT, + }) + const [ + confirmOffsetsTargetProps, + confirmOffsetsTooltipProps, + ] = useHoverTooltip({ placement: TOOLTIP_LEFT, }) @@ -114,6 +121,22 @@ export function SetupLabwarePositionCheck( )} { + setOffsetsConfirmed(true) + }} + id="LPC_setOffsetsConfirmed" + padding={`${SPACING.spacing8} ${SPACING.spacing16}`} + {...confirmOffsetsTargetProps} + disabled={offsetsConfirmed || lpcDisabledReason !== null} + > + {t('confirm_offsets')} + + {lpcDisabledReason !== null ? ( + + {lpcDisabledReason} + + ) : null} + { @@ -121,21 +144,16 @@ export function SetupLabwarePositionCheck( setIsShowingLPCSuccessToast(false) }} id="LabwareSetup_checkLabwarePositionsButton" - {...targetProps} + {...runLPCTargetProps} disabled={lpcDisabledReason !== null} > {t('run_labware_position_check')} - + {lpcDisabledReason !== null ? ( - {lpcDisabledReason} + + {lpcDisabledReason} + ) : null} - - {t('proceed_to_labware_setup_step')} - {LPCWizard} diff --git a/app/src/organisms/Devices/ProtocolRun/SetupLiquids/SetupLiquidsMap.tsx b/app/src/organisms/Devices/ProtocolRun/SetupLiquids/SetupLiquidsMap.tsx index 4a73d6a3906..3bfd0d9c294 100644 --- a/app/src/organisms/Devices/ProtocolRun/SetupLiquids/SetupLiquidsMap.tsx +++ b/app/src/organisms/Devices/ProtocolRun/SetupLiquids/SetupLiquidsMap.tsx @@ -108,7 +108,9 @@ export function SetupLiquidsMap( setHoverLabwareId('') }} onClick={() => { - if (labwareHasLiquid) setLiquidDetailsLabwareId(topLabwareId) + if (labwareHasLiquid) { + setLiquidDetailsLabwareId(topLabwareId) + } }} cursor={labwareHasLiquid ? 'pointer' : ''} > @@ -169,8 +171,9 @@ export function SetupLiquidsMap( setHoverLabwareId('') }} onClick={() => { - if (labwareHasLiquid) + if (labwareHasLiquid) { setLiquidDetailsLabwareId(topLabwareId) + } }} cursor={labwareHasLiquid ? 'pointer' : ''} > diff --git a/app/src/organisms/Devices/ProtocolRun/SetupLiquids/__tests__/SetupLiquids.test.tsx b/app/src/organisms/Devices/ProtocolRun/SetupLiquids/__tests__/SetupLiquids.test.tsx index 1c3dc33181e..06e48c49738 100644 --- a/app/src/organisms/Devices/ProtocolRun/SetupLiquids/__tests__/SetupLiquids.test.tsx +++ b/app/src/organisms/Devices/ProtocolRun/SetupLiquids/__tests__/SetupLiquids.test.tsx @@ -7,27 +7,35 @@ import { i18n } from '../../../../../i18n' import { SetupLiquids } from '../index' import { SetupLiquidsList } from '../SetupLiquidsList' import { SetupLiquidsMap } from '../SetupLiquidsMap' -import { BackToTopButton } from '../../BackToTopButton' vi.mock('../SetupLiquidsList') vi.mock('../SetupLiquidsMap') -vi.mock('../../BackToTopButton') -const render = (props: React.ComponentProps) => { - return renderWithProviders( - , - { - i18nInstance: i18n, +describe('SetupLiquids', () => { + const render = ( + props: React.ComponentProps & { + startConfirmed?: boolean } - ) -} + ) => { + let isConfirmed = + props?.startConfirmed == null ? false : props.startConfirmed + const confirmFn = vi.fn((confirmed: boolean) => { + isConfirmed = confirmed + }) + return renderWithProviders( + , + { + i18nInstance: i18n, + } + ) + } -describe('SetupLiquids', () => { let props: React.ComponentProps beforeEach(() => { vi.mocked(SetupLiquidsList).mockReturnValue( @@ -36,16 +44,13 @@ describe('SetupLiquids', () => { vi.mocked(SetupLiquidsMap).mockReturnValue(
Mock setup liquids map
) - vi.mocked(BackToTopButton).mockReturnValue( - - ) }) it('renders the list and map view buttons and proceed button', () => { render(props) screen.getByRole('button', { name: 'List View' }) screen.getByRole('button', { name: 'Map View' }) - screen.getByRole('button', { name: 'Mock BackToTopButton' }) + screen.getByRole('button', { name: 'Confirm placements' }) }) it('renders the map view when you press that toggle button', () => { render(props) diff --git a/app/src/organisms/Devices/ProtocolRun/SetupLiquids/index.tsx b/app/src/organisms/Devices/ProtocolRun/SetupLiquids/index.tsx index daa2a7e114f..243bfeb3ed6 100644 --- a/app/src/organisms/Devices/ProtocolRun/SetupLiquids/index.tsx +++ b/app/src/organisms/Devices/ProtocolRun/SetupLiquids/index.tsx @@ -6,10 +6,10 @@ import { SPACING, DIRECTION_COLUMN, ALIGN_CENTER, + PrimaryButton, } from '@opentrons/components' import { useToggleGroup } from '../../../../molecules/ToggleGroup/useToggleGroup' import { ANALYTICS_LIQUID_SETUP_VIEW_TOGGLE } from '../../../../redux/analytics' -import { BackToTopButton } from '../BackToTopButton' import { SetupLiquidsList } from './SetupLiquidsList' import { SetupLiquidsMap } from './SetupLiquidsMap' @@ -19,17 +19,19 @@ import type { } from '@opentrons/shared-data' interface SetupLiquidsProps { - protocolRunHeaderRef: React.RefObject | null - robotName: string runId: string protocolAnalysis: CompletedProtocolAnalysis | ProtocolAnalysisOutput | null + isLiquidSetupConfirmed: boolean + setLiquidSetupConfirmed: (confirmed: boolean) => void + robotName: string } export function SetupLiquids({ - protocolRunHeaderRef, - robotName, runId, protocolAnalysis, + isLiquidSetupConfirmed, + setLiquidSetupConfirmed, + robotName, }: SetupLiquidsProps): JSX.Element { const { t } = useTranslation('protocol_setup') const [selectedValue, toggleGroup] = useToggleGroup( @@ -51,12 +53,14 @@ export function SetupLiquids({ )} - + { + setLiquidSetupConfirmed(true) + }} + disabled={isLiquidSetupConfirmed} + > + {t('confirm_placements')} + ) diff --git a/app/src/organisms/Devices/ProtocolRun/SetupModuleAndDeck/__tests__/utils.test.ts b/app/src/organisms/Devices/ProtocolRun/SetupModuleAndDeck/__tests__/utils.test.ts index 6a86b6daf55..2e4639a3c98 100644 --- a/app/src/organisms/Devices/ProtocolRun/SetupModuleAndDeck/__tests__/utils.test.ts +++ b/app/src/organisms/Devices/ProtocolRun/SetupModuleAndDeck/__tests__/utils.test.ts @@ -9,6 +9,13 @@ describe('getModuleImage', () => { ) }) + it('should render the high res magnetic module image when the model is a magnetic module gen 1 high res', () => { + const result = getModuleImage('magneticModuleV1', true) + expect(result).toEqual( + '/app/src/assets/images/modules/magneticModuleV2@3x.png' + ) + }) + it('should render the magnetic module image when the model is a magnetic module gen 2', () => { const result = getModuleImage('magneticModuleV2') expect(result).toEqual( @@ -30,6 +37,13 @@ describe('getModuleImage', () => { ) }) + it('should render the high res temperature module image when the model is a temperature module high res', () => { + const result = getModuleImage('temperatureModuleV2', true) + expect(result).toEqual( + '/app/src/assets/images/modules/temperatureModuleV2@3x.png' + ) + }) + it('should render the heater-shaker module image when the model is a heater-shaker module gen 1', () => { const result = getModuleImage('heaterShakerModuleV1') expect(result).toEqual( @@ -37,11 +51,25 @@ describe('getModuleImage', () => { ) }) + it('should render the high res heater-shaker module image when the model is a heater-shaker module gen 1 high res', () => { + const result = getModuleImage('heaterShakerModuleV1', true) + expect(result).toEqual( + '/app/src/assets/images/modules/heaterShakerModuleV1@3x.png' + ) + }) + it('should render the thermocycler module image when the model is a thermocycler module gen 1', () => { const result = getModuleImage('thermocyclerModuleV1') expect(result).toEqual('/app/src/assets/images/thermocycler_closed.png') }) + it('should render the high res thermocycler module image when the model is a thermocycler module gen 1 high res', () => { + const result = getModuleImage('thermocyclerModuleV1', true) + expect(result).toEqual( + '/app/src/assets/images/modules/thermocyclerModuleV1@3x.png' + ) + }) + it('should render the thermocycler module image when the model is a thermocycler module gen 2', () => { const result = getModuleImage('thermocyclerModuleV2') expect(result).toEqual( diff --git a/app/src/organisms/Devices/ProtocolRun/SetupModuleAndDeck/utils.ts b/app/src/organisms/Devices/ProtocolRun/SetupModuleAndDeck/utils.ts index c5ac5c7984e..f5bd5187ad1 100644 --- a/app/src/organisms/Devices/ProtocolRun/SetupModuleAndDeck/utils.ts +++ b/app/src/organisms/Devices/ProtocolRun/SetupModuleAndDeck/utils.ts @@ -15,6 +15,10 @@ import magneticModule from '../../../../assets/images/magnetic_module_gen_2_tran import temperatureModule from '../../../../assets/images/temp_deck_gen_2_transparent.png' import thermoModuleGen1 from '../../../../assets/images/thermocycler_closed.png' import heaterShakerModule from '../../../../assets/images/heater_shaker_module_transparent.png' +import magneticModuleHighRes from '../../../../assets/images/modules/magneticModuleV2@3x.png' +import temperatureModuleHighRes from '../../../../assets/images/modules/temperatureModuleV2@3x.png' +import thermoModuleGen1HighRes from '../../../../assets/images/modules/thermocyclerModuleV1@3x.png' +import heaterShakerModuleHighRes from '../../../../assets/images/modules/heaterShakerModuleV1@3x.png' import thermoModuleGen2 from '../../../../assets/images/thermocycler_gen_2_closed.png' import magneticBlockGen1 from '../../../../assets/images/magnetic_block_gen_1.png' import stagingAreaMagneticBlockGen1 from '../../../../assets/images/staging_area_magnetic_block_gen_1.png' @@ -25,18 +29,21 @@ import wasteChuteStagingArea from '../../../../assets/images/waste_chute_with_st import type { CutoutFixtureId, ModuleModel } from '@opentrons/shared-data' -export function getModuleImage(model: ModuleModel): string { +export function getModuleImage( + model: ModuleModel, + highRes: boolean = false +): string { switch (model) { case 'magneticModuleV1': case 'magneticModuleV2': - return magneticModule + return highRes ? magneticModuleHighRes : magneticModule case 'temperatureModuleV1': case 'temperatureModuleV2': - return temperatureModule + return highRes ? temperatureModuleHighRes : temperatureModule case 'heaterShakerModuleV1': - return heaterShakerModule + return highRes ? heaterShakerModuleHighRes : heaterShakerModule case 'thermocyclerModuleV1': - return thermoModuleGen1 + return highRes ? thermoModuleGen1HighRes : thermoModuleGen1 case 'thermocyclerModuleV2': return thermoModuleGen2 case 'magneticBlockV1': diff --git a/app/src/organisms/Devices/ProtocolRun/__tests__/ProtocolRunHeader.test.tsx b/app/src/organisms/Devices/ProtocolRun/__tests__/ProtocolRunHeader.test.tsx index 70b16c61b55..872dff5771f 100644 --- a/app/src/organisms/Devices/ProtocolRun/__tests__/ProtocolRunHeader.test.tsx +++ b/app/src/organisms/Devices/ProtocolRun/__tests__/ProtocolRunHeader.test.tsx @@ -97,7 +97,9 @@ import { ProtocolDropTipModal, useProtocolDropTipModal, } from '../ProtocolDropTipModal' +import { ConfirmMissingStepsModal } from '../ConfirmMissingStepsModal' +import type { MissingSteps } from '../ProtocolRunSetup' import type { UseQueryResult } from 'react-query' import type { NavigateFunction } from 'react-router-dom' import type { Mock } from 'vitest' @@ -153,6 +155,7 @@ vi.mock('../../../ProtocolUpload/hooks/useMostRecentRunId') vi.mock('../../../../resources/runs') vi.mock('../../../ErrorRecoveryFlows') vi.mock('../ProtocolDropTipModal') +vi.mock('../ConfirmMissingStepsModal') const ROBOT_NAME = 'otie' const RUN_ID = '95e67900-bc9f-4fbf-92c6-cc4d7226a51b' @@ -215,6 +218,7 @@ const mockDoorStatus = { doorRequiredClosedForProtocol: true, }, } +let mockMissingSteps: MissingSteps = [] const render = () => { return renderWithProviders( @@ -224,6 +228,7 @@ const render = () => { robotName={ROBOT_NAME} runId={RUN_ID} makeHandleJumpToStep={vi.fn(() => vi.fn())} + missingSetupSteps={mockMissingSteps} /> , { i18nInstance: i18n } @@ -240,7 +245,7 @@ describe('ProtocolRunHeader', () => { mockTrackProtocolRunEvent = vi.fn(() => new Promise(resolve => resolve({}))) mockCloseCurrentRun = vi.fn() mockDetermineTipStatus = vi.fn() - + mockMissingSteps = [] vi.mocked(useTrackEvent).mockReturnValue(mockTrackEvent) vi.mocked(ConfirmCancelModal).mockReturnValue(
Mock ConfirmCancelModal
@@ -267,6 +272,9 @@ describe('ProtocolRunHeader', () => { vi.mocked(ConfirmAttachmentModal).mockReturnValue(
mock confirm attachment modal
) + vi.mocked(ConfirmMissingStepsModal).mockReturnValue( +
mock missing steps modal
+ ) when(vi.mocked(useProtocolAnalysisErrors)).calledWith(RUN_ID).thenReturn({ analysisErrors: null, }) diff --git a/app/src/organisms/Devices/ProtocolRun/__tests__/ProtocolRunSetup.test.tsx b/app/src/organisms/Devices/ProtocolRun/__tests__/ProtocolRunSetup.test.tsx index 89238cbaa01..e4fbc00e234 100644 --- a/app/src/organisms/Devices/ProtocolRun/__tests__/ProtocolRunSetup.test.tsx +++ b/app/src/organisms/Devices/ProtocolRun/__tests__/ProtocolRunSetup.test.tsx @@ -40,6 +40,7 @@ import { SetupLiquids } from '../SetupLiquids' import { SetupModuleAndDeck } from '../SetupModuleAndDeck' import { EmptySetupStep } from '../EmptySetupStep' import { ProtocolRunSetup } from '../ProtocolRunSetup' +import type { MissingSteps } from '../ProtocolRunSetup' import { useNotifyRunQuery } from '../../../../resources/runs' import type * as SharedData from '@opentrons/shared-data' @@ -68,12 +69,18 @@ vi.mock('@opentrons/shared-data', async importOriginal => { const ROBOT_NAME = 'otie' const RUN_ID = '1' const MOCK_PROTOCOL_LIQUID_KEY = { liquids: [] } +let mockMissingSteps: MissingSteps = [] +const mockSetMissingSteps = vi.fn((missingSteps: MissingSteps) => { + mockMissingSteps = missingSteps +}) const render = () => { return renderWithProviders( , { i18nInstance: i18n, @@ -83,6 +90,7 @@ const render = () => { describe('ProtocolRunSetup', () => { beforeEach(() => { + mockMissingSteps = [] when(vi.mocked(useIsFlex)).calledWith(ROBOT_NAME).thenReturn(false) when(vi.mocked(useMostRecentCompletedAnalysis)) .calledWith(RUN_ID) @@ -121,7 +129,6 @@ describe('ProtocolRunSetup', () => { when(vi.mocked(SetupLabware)) .calledWith( expect.objectContaining({ - protocolRunHeaderRef: null, robotName: ROBOT_NAME, runId: RUN_ID, }), @@ -146,6 +153,9 @@ describe('ProtocolRunSetup', () => { when(vi.mocked(useRunPipetteInfoByMount)) .calledWith(RUN_ID) .thenReturn({ left: null, right: null }) + when(vi.mocked(useModuleCalibrationStatus)) + .calledWith(ROBOT_NAME, RUN_ID) + .thenReturn({ complete: true }) }) afterEach(() => { vi.resetAllMocks() @@ -181,13 +191,6 @@ describe('ProtocolRunSetup', () => { screen.getByText('Calibration needed') }) - it('does not render calibration status when run has started', () => { - when(vi.mocked(useRunHasStarted)).calledWith(RUN_ID).thenReturn(true) - render() - expect(screen.queryByText('Calibration needed')).toBeNull() - expect(screen.queryByText('Calibration ready')).toBeNull() - }) - describe('when no modules are in the protocol', () => { it('renders robot calibration setup for OT-2', () => { render() @@ -426,10 +429,6 @@ describe('ProtocolRunSetup', () => { when(vi.mocked(useRunHasStarted)).calledWith(RUN_ID).thenReturn(true) render() - await new Promise(resolve => setTimeout(resolve, 1000)) - expect(screen.getByText('Mock SetupRobotCalibration')).not.toBeVisible() - expect(screen.getByText('Mock SetupModules')).not.toBeVisible() - expect(screen.getByText('Mock SetupLabware')).not.toBeVisible() screen.getByText('Setup is view-only once run has started') }) diff --git a/app/src/organisms/Devices/ProtocolRun/utils/__tests__/getLocationInfoNames.test.ts b/app/src/organisms/Devices/ProtocolRun/utils/__tests__/getLocationInfoNames.test.ts index 5f6a14090f0..f917f64035f 100644 --- a/app/src/organisms/Devices/ProtocolRun/utils/__tests__/getLocationInfoNames.test.ts +++ b/app/src/organisms/Devices/ProtocolRun/utils/__tests__/getLocationInfoNames.test.ts @@ -151,6 +151,7 @@ describe('getLocationInfoNames', () => { labwareName: LABWARE_DISPLAY_NAME, moduleModel: MOCK_MODEL, adapterName: ADAPTER_DISPLAY_NAME, + adapterId: ADAPTER_ID, } expect( getLocationInfoNames(LABWARE_ID, MOCK_ADAPTER_MOD_COMMANDS as any) @@ -161,6 +162,7 @@ describe('getLocationInfoNames', () => { slotName: SLOT, labwareName: LABWARE_DISPLAY_NAME, adapterName: ADAPTER_DISPLAY_NAME, + adapterId: ADAPTER_ID, } expect( getLocationInfoNames(LABWARE_ID, MOCK_ADAPTER_COMMANDS as any) diff --git a/app/src/organisms/Devices/ProtocolRun/utils/getLocationInfoNames.ts b/app/src/organisms/Devices/ProtocolRun/utils/getLocationInfoNames.ts index c01d46259f5..c3404945dcb 100644 --- a/app/src/organisms/Devices/ProtocolRun/utils/getLocationInfoNames.ts +++ b/app/src/organisms/Devices/ProtocolRun/utils/getLocationInfoNames.ts @@ -9,8 +9,10 @@ import type { export interface LocationInfoNames { slotName: string labwareName: string + labwareNickname?: string adapterName?: string moduleModel?: ModuleModel + adapterId?: string } export function getLocationInfoNames( @@ -39,6 +41,7 @@ export function getLocationInfoNames( loadLabwareCommand.result?.definition != null ? getLabwareDisplayName(loadLabwareCommand.result?.definition) : '' + const labwareNickname = loadLabwareCommand.params.displayName const labwareLocation = loadLabwareCommand.params.location @@ -79,8 +82,10 @@ export function getLocationInfoNames( return { slotName: loadedAdapterCommand?.params.location.slotName, labwareName, + labwareNickname, adapterName: loadedAdapterCommand?.result?.definition.metadata.displayName, + adapterId: loadedAdapterCommand?.result?.labwareId, } } else if ( loadedAdapterCommand?.params.location !== 'offDeck' && @@ -96,8 +101,10 @@ export function getLocationInfoNames( ? { slotName: loadModuleCommandUnderAdapter.params.location.slotName, labwareName, + labwareNickname, adapterName: loadedAdapterCommand.result?.definition.metadata.displayName, + adapterId: loadedAdapterCommand?.result?.labwareId, moduleModel: loadModuleCommandUnderAdapter.params.model, } : { slotName: '', labwareName } diff --git a/app/src/organisms/InterventionModal/__tests__/InterventionModal.test.tsx b/app/src/organisms/InterventionModal/__tests__/InterventionModal.test.tsx index 06f4f0a22a3..e1a6830d251 100644 --- a/app/src/organisms/InterventionModal/__tests__/InterventionModal.test.tsx +++ b/app/src/organisms/InterventionModal/__tests__/InterventionModal.test.tsx @@ -1,11 +1,13 @@ import * as React from 'react' -import { fireEvent, screen } from '@testing-library/react' +import { fireEvent, renderHook, screen } from '@testing-library/react' import { describe, it, expect, vi, beforeEach } from 'vitest' + +import { RUN_STATUS_RUNNING, RUN_STATUS_STOPPED } from '@opentrons/api-client' import { getLabwareDefURI } from '@opentrons/shared-data' -import { renderWithProviders } from '../../../__testing-utils__' +import { renderWithProviders } from '../../../__testing-utils__' +import { mockTipRackDefinition } from '../../../redux/custom-labware/__fixtures__' import { i18n } from '../../../i18n' -import { InterventionModal } from '..' import { mockPauseCommandWithoutStartTime, mockPauseCommandWithStartTime, @@ -13,9 +15,11 @@ import { mockMoveLabwareCommandFromModule, truncatedCommandMessage, } from '../__fixtures__' -import { mockTipRackDefinition } from '../../../redux/custom-labware/__fixtures__' +import { InterventionModal, useInterventionModal } from '..' import { useIsFlex } from '../../Devices/hooks' + import type { CompletedProtocolAnalysis } from '@opentrons/shared-data' +import type { RunData } from '@opentrons/api-client' const ROBOT_NAME = 'Otie' @@ -23,6 +27,61 @@ const mockOnResumeHandler = vi.fn() vi.mock('../../Devices/hooks') +describe('useInterventionModal', () => { + const defaultProps = { + runData: { id: 'run1' } as RunData, + lastRunCommand: mockPauseCommandWithStartTime, + runStatus: RUN_STATUS_RUNNING, + robotName: 'TestRobot', + analysis: null, + } + + it('should return showModal true when conditions are met', () => { + const { result } = renderHook(() => useInterventionModal(defaultProps)) + + expect(result.current.showModal).toBe(true) + expect(result.current.modalProps).not.toBeNull() + }) + + it('should return showModal false when runStatus is terminal', () => { + const props = { ...defaultProps, runStatus: RUN_STATUS_STOPPED } + + const { result } = renderHook(() => useInterventionModal(props)) + + expect(result.current.showModal).toBe(false) + expect(result.current.modalProps).toBeNull() + }) + + it('should return showModal false when lastRunCommand is null', () => { + const props = { ...defaultProps, lastRunCommand: null } + + const { result } = renderHook(() => useInterventionModal(props)) + + expect(result.current.showModal).toBe(false) + expect(result.current.modalProps).toBeNull() + }) + + it('should return showModal false when robotName is null', () => { + const props = { ...defaultProps, robotName: null } + + const { result } = renderHook(() => useInterventionModal(props)) + + expect(result.current.showModal).toBe(false) + expect(result.current.modalProps).toBeNull() + }) + + it('should return correct modalProps when showModal is true', () => { + const { result } = renderHook(() => useInterventionModal(defaultProps)) + + expect(result.current.modalProps).toEqual({ + command: mockPauseCommandWithStartTime, + run: defaultProps.runData, + robotName: 'TestRobot', + analysis: null, + }) + }) +}) + const render = (props: React.ComponentProps) => { return renderWithProviders(, { i18nInstance: i18n, diff --git a/app/src/organisms/InterventionModal/index.tsx b/app/src/organisms/InterventionModal/index.tsx index 1e7cb9e8475..6c3002ce26b 100644 --- a/app/src/organisms/InterventionModal/index.tsx +++ b/app/src/organisms/InterventionModal/index.tsx @@ -19,6 +19,12 @@ import { TYPOGRAPHY, LegacyStyledText, } from '@opentrons/components' +import { + RUN_STATUS_FAILED, + RUN_STATUS_FINISHING, + RUN_STATUS_STOPPED, + RUN_STATUS_SUCCEEDED, +} from '@opentrons/api-client' import { SmallButton } from '../../atoms/buttons' import { Modal } from '../../molecules/Modal' @@ -26,30 +32,66 @@ import { InterventionModal as InterventionModalMolecule } from '../../molecules/ import { getIsOnDevice } from '../../redux/config' import { PauseInterventionContent } from './PauseInterventionContent' import { MoveLabwareInterventionContent } from './MoveLabwareInterventionContent' +import { isInterventionCommand } from './utils' +import { useRobotType } from '../Devices/hooks' -import type { RunCommandSummary, RunData } from '@opentrons/api-client' import type { IconName } from '@opentrons/components' import type { CompletedProtocolAnalysis } from '@opentrons/shared-data' -import { useRobotType } from '../Devices/hooks' +import type { + RunCommandSummary, + RunData, + RunStatus, +} from '@opentrons/api-client' -const LEARN_ABOUT_MANUAL_STEPS_URL = - 'https://support.opentrons.com/s/article/Manual-protocol-steps' +const TERMINAL_RUN_STATUSES: RunStatus[] = [ + RUN_STATUS_STOPPED, + RUN_STATUS_FAILED, + RUN_STATUS_FINISHING, + RUN_STATUS_SUCCEEDED, +] -const CONTENT_STYLE = { - display: DISPLAY_FLEX, - flexDirection: DIRECTION_COLUMN, - alignItems: ALIGN_FLEX_START, - gridGap: SPACING.spacing24, - padding: SPACING.spacing32, - borderRadius: BORDERS.borderRadius8, -} as const +export interface UseInterventionModalProps { + runData: RunData | null + lastRunCommand: RunCommandSummary | null + runStatus: RunStatus | null + robotName: string | null + analysis: CompletedProtocolAnalysis | null +} -const FOOTER_STYLE = { - display: DISPLAY_FLEX, - width: '100%', - alignItems: ALIGN_CENTER, - justifyContent: JUSTIFY_SPACE_BETWEEN, -} as const +export type UseInterventionModalResult = + | { showModal: false; modalProps: null } + | { showModal: true; modalProps: Omit } + +// If showModal is true, modalProps are guaranteed not to be null. +export function useInterventionModal({ + runData, + lastRunCommand, + runStatus, + robotName, + analysis, +}: UseInterventionModalProps): UseInterventionModalResult { + const isValidIntervention = + lastRunCommand != null && + robotName != null && + isInterventionCommand(lastRunCommand) && + runData != null && + runStatus != null && + !TERMINAL_RUN_STATUSES.includes(runStatus) + + if (!isValidIntervention) { + return { showModal: false, modalProps: null } + } else { + return { + showModal: true, + modalProps: { + command: lastRunCommand, + run: runData, + robotName, + analysis, + }, + } + } +} export interface InterventionModalProps { robotName: string @@ -71,25 +113,28 @@ export function InterventionModal({ const robotType = useRobotType(robotName) const childContent = React.useMemo(() => { - if ( - command.commandType === 'waitForResume' || - command.commandType === 'pause' // legacy pause command - ) { - return ( - - ) - } else if (command.commandType === 'moveLabware') { - return ( - - ) - } else { - return null + switch (command.commandType) { + case 'waitForResume': + case 'pause': // legacy pause command + return ( + + ) + case 'moveLabware': + return ( + + ) + default: + console.warn( + 'Unhandled command passed to InterventionModal: ', + command.commandType + ) + return null } }, [ command.id, @@ -98,21 +143,33 @@ export function InterventionModal({ run.modules.map(m => m.id).join(), ]) - let iconName: IconName | null = null - let headerTitle = '' - let headerTitleOnDevice = '' - if ( - command.commandType === 'waitForResume' || - command.commandType === 'pause' // legacy pause command - ) { - iconName = 'pause-circle' - headerTitle = t('pause_on', { robot_name: robotName }) - headerTitleOnDevice = t('pause') - } else if (command.commandType === 'moveLabware') { - iconName = 'move-xy-circle' - headerTitle = t('move_labware_on', { robot_name: robotName }) - headerTitleOnDevice = t('move_labware') - } + const { iconName, headerTitle, headerTitleOnDevice } = (() => { + switch (command.commandType) { + case 'waitForResume': + case 'pause': + return { + iconName: 'pause-circle' as IconName, + headerTitle: t('pause_on', { robot_name: robotName }), + headerTitleOnDevice: t('pause'), + } + case 'moveLabware': + return { + iconName: 'move-xy-circle' as IconName, + headerTitle: t('move_labware_on', { robot_name: robotName }), + headerTitleOnDevice: t('move_labware'), + } + default: + console.warn( + 'Unhandled command passed to InterventionModal: ', + command.commandType + ) + return { + iconName: null, + headerTitle: '', + headerTitleOnDevice: '', + } + } + })() // TODO(bh, 2023-7-18): this is a one-off modal implementation for desktop // reimplement when design system shares a modal component between desktop/ODD @@ -171,3 +228,22 @@ export function InterventionModal({ ) } + +const LEARN_ABOUT_MANUAL_STEPS_URL = + 'https://support.opentrons.com/s/article/Manual-protocol-steps' + +const CONTENT_STYLE = { + display: DISPLAY_FLEX, + flexDirection: DIRECTION_COLUMN, + alignItems: ALIGN_FLEX_START, + gridGap: SPACING.spacing24, + padding: SPACING.spacing32, + borderRadius: BORDERS.borderRadius8, +} as const + +const FOOTER_STYLE = { + display: DISPLAY_FLEX, + width: '100%', + alignItems: ALIGN_CENTER, + justifyContent: JUSTIFY_SPACE_BETWEEN, +} as const diff --git a/app/src/organisms/Navigation/__tests__/Navigation.test.tsx b/app/src/organisms/Navigation/__tests__/Navigation.test.tsx index dc132b39749..d1056531976 100644 --- a/app/src/organisms/Navigation/__tests__/Navigation.test.tsx +++ b/app/src/organisms/Navigation/__tests__/Navigation.test.tsx @@ -2,20 +2,17 @@ import * as React from 'react' import { MemoryRouter } from 'react-router-dom' import { fireEvent, screen } from '@testing-library/react' import { beforeEach, describe, expect, it, vi } from 'vitest' -import { when } from 'vitest-when' import { renderWithProviders } from '../../../__testing-utils__' import { i18n } from '../../../i18n' import { getLocalRobot } from '../../../redux/discovery' import { mockConnectedRobot } from '../../../redux/discovery/__fixtures__' -import { useFeatureFlag } from '../../../redux/config' import { useNetworkConnection } from '../../../resources/networking/hooks/useNetworkConnection' import { NavigationMenu } from '../NavigationMenu' import { Navigation } from '..' vi.mock('../../../resources/networking/hooks/useNetworkConnection') vi.mock('../../../redux/discovery') -vi.mock('../../../redux/config') vi.mock('../NavigationMenu') mockConnectedRobot.name = '12345678901234567' @@ -66,6 +63,9 @@ describe('Navigation', () => { const allProtocols = screen.getByRole('link', { name: 'Protocols' }) expect(allProtocols).toHaveAttribute('href', '/protocols') + const quickTransfer = screen.getByRole('link', { name: 'Quick Transfer' }) + expect(quickTransfer).toHaveAttribute('href', '/quick-transfer') + const instruments = screen.getByRole('link', { name: 'Instruments' }) expect(instruments).toHaveAttribute('href', '/instruments') @@ -75,15 +75,6 @@ describe('Navigation', () => { expect(screen.queryByText('Get started')).not.toBeInTheDocument() expect(screen.queryByLabelText('network icon')).not.toBeInTheDocument() }) - it('should render quick transfer tab if feature flag is on', () => { - when(vi.mocked(useFeatureFlag)) - .calledWith('enableQuickTransfer') - .thenReturn(true) - render(props) - screen.getByRole('link', { name: '123456789012...' }) // because of the truncate function - const quickTransfer = screen.getByRole('link', { name: 'Quick Transfer' }) - expect(quickTransfer).toHaveAttribute('href', '/quick-transfer') - }) it('should render a network icon', () => { vi.mocked(useNetworkConnection).mockReturnValue({ isEthernetConnected: false, diff --git a/app/src/organisms/Navigation/index.tsx b/app/src/organisms/Navigation/index.tsx index 2d920726f03..a9a55f53e63 100644 --- a/app/src/organisms/Navigation/index.tsx +++ b/app/src/organisms/Navigation/index.tsx @@ -29,12 +29,12 @@ import { ODD_FOCUS_VISIBLE } from '../../atoms/buttons/constants' import { useNetworkConnection } from '../../resources/networking/hooks/useNetworkConnection' import { getLocalRobot } from '../../redux/discovery' -import { useFeatureFlag } from '../../redux/config' import { NavigationMenu } from './NavigationMenu' import type { ON_DEVICE_DISPLAY_PATHS } from '../../App/OnDeviceDisplayApp' -let NAV_LINKS: Array = [ +const NAV_LINKS: Array = [ '/protocols', + '/quick-transfer', '/instruments', '/robot-settings', ] @@ -68,15 +68,6 @@ export function Navigation(props: NavigationProps): JSX.Element { const localRobot = useSelector(getLocalRobot) const [showNavMenu, setShowNavMenu] = React.useState(false) const robotName = localRobot?.name != null ? localRobot.name : 'no name' - const enableQuickTransferFF = useFeatureFlag('enableQuickTransfer') - if (enableQuickTransferFF) { - NAV_LINKS = [ - '/protocols', - '/quick-transfer', - '/instruments', - '/robot-settings', - ] - } // We need to display an icon for what type of network connection (if any) // is active next to the robot's name. The designs call for it to change color diff --git a/app/src/organisms/ProtocolSetupLabware/__tests__/ProtocolSetupLabware.test.tsx b/app/src/organisms/ProtocolSetupLabware/__tests__/ProtocolSetupLabware.test.tsx index 8182a8b73b3..99b3c555dd5 100644 --- a/app/src/organisms/ProtocolSetupLabware/__tests__/ProtocolSetupLabware.test.tsx +++ b/app/src/organisms/ProtocolSetupLabware/__tests__/ProtocolSetupLabware.test.tsx @@ -52,11 +52,17 @@ const mockRefetch = vi.fn() const mockCreateLiveCommand = vi.fn() const render = () => { + let confirmed = false + const setIsConfirmed = vi.fn((ready: boolean) => { + confirmed = ready + }) return renderWithProviders( , { @@ -117,6 +123,8 @@ describe('ProtocolSetupLabware', () => { expect(screen.queryByText('Map View')).toBeNull() fireEvent.click(screen.getByRole('button', { name: 'List View' })) screen.getByText('Labware') + screen.getByText('Labware name') + screen.getByText('Location') }) it('sends a latch-close command when the labware latch is open and the button is clicked', () => { diff --git a/app/src/organisms/ProtocolSetupLabware/index.tsx b/app/src/organisms/ProtocolSetupLabware/index.tsx index fa4d3926fdb..919d887f491 100644 --- a/app/src/organisms/ProtocolSetupLabware/index.tsx +++ b/app/src/organisms/ProtocolSetupLabware/index.tsx @@ -6,7 +6,6 @@ import styled, { css } from 'styled-components' import { ALIGN_CENTER, ALIGN_FLEX_START, - ALIGN_STRETCH, BORDERS, Box, COLORS, @@ -22,6 +21,7 @@ import { SPACING, LegacyStyledText, TYPOGRAPHY, + Chip, } from '@opentrons/components' import { FLEX_ROBOT_TYPE, @@ -37,7 +37,7 @@ import { useModulesQuery, } from '@opentrons/react-api-client' -import { FloatingActionButton } from '../../atoms/buttons' +import { FloatingActionButton, SmallButton } from '../../atoms/buttons' import { ODDBackButton } from '../../molecules/ODDBackButton' import { getTopPortalEl } from '../../App/portal' import { Modal } from '../../molecules/Modal' @@ -49,6 +49,7 @@ import { getProtocolModulesInfo } from '../Devices/ProtocolRun/utils/getProtocol import { getAttachedProtocolModuleMatches } from '../ProtocolSetupModulesAndDeck/utils' import { getNestedLabwareInfo } from '../Devices/ProtocolRun/SetupLabware/getNestedLabwareInfo' import { LabwareMapView } from './LabwareMapView' +import { LabwareStackModal } from '../Devices/ProtocolRun/SetupLabware/LabwareStackModal' import type { UseQueryResult } from 'react-query' import type { @@ -77,11 +78,15 @@ const LabwareThumbnail = styled.svg` export interface ProtocolSetupLabwareProps { runId: string setSetupScreen: React.Dispatch> + isConfirmed: boolean + setIsConfirmed: (confirmed: boolean) => void } export function ProtocolSetupLabware({ runId, setSetupScreen, + isConfirmed, + setIsConfirmed, }: ProtocolSetupLabwareProps): JSX.Element { const { t } = useTranslation('protocol_setup') const [showMapView, setShowMapView] = React.useState(false) @@ -144,6 +149,7 @@ export function ProtocolSetupLabware({ } let location: JSX.Element | string | null = null + let topLabwareId: string | null = null if ( selectedLabware != null && typeof selectedLabware.location === 'object' && @@ -172,6 +178,17 @@ export function ProtocolSetupLabware({ module.moduleId === selectedLabware.location.moduleId ) if (matchedModule != null) { + topLabwareId = + mostRecentAnalysis?.commands.find( + (command): command is LoadLabwareRunTimeCommand => { + return ( + command.commandType === 'loadLabware' && + typeof command.params.location === 'object' && + 'moduleId' in command.params.location && + command.params.location.moduleId === matchedModule.moduleId + ) + } + )?.result?.labwareId ?? null location = } } else if ( @@ -186,6 +203,17 @@ export function ProtocolSetupLabware({ command.result?.labwareId === adapterId )?.params.location if (adapterLocation != null && adapterLocation !== 'offDeck') { + topLabwareId = + mostRecentAnalysis?.commands.find( + (command): command is LoadLabwareRunTimeCommand => { + return ( + command.commandType === 'loadLabware' && + typeof command.params.location === 'object' && + 'labwareId' in command.params.location && + command.params.location.labwareId === adapterId + ) + } + )?.result?.labwareId ?? null if ('slotName' in adapterLocation) { location = } else if ('moduleId' in adapterLocation) { @@ -203,19 +231,16 @@ export function ProtocolSetupLabware({ <> {createPortal( <> - {showLabwareDetailsModal && selectedLabware != null ? ( + {showLabwareDetailsModal && + topLabwareId == null && + selectedLabware != null ? ( { setShowLabwareDetailsModal(false) setSelectedLabware(null) }} > - - - - + + + + ) : null} , getTopPortalEl() )} - { - setSetupScreen('prepare to run') - }} - /> + + { + setSetupScreen('prepare to run') + }} + /> + {isConfirmed ? ( + + ) : ( + { + setIsConfirmed(true) + setSetupScreen('prepare to run') + }} + /> + )} + )} + {showLabwareDetailsModal && topLabwareId != null ? ( + { + setSelectedLabware(null) + setShowLabwareDetailsModal(false) + }} + /> + ) : null} ) => { - return renderWithProviders(, { - i18nInstance: i18n, +describe('ProtocolSetupLiquids', () => { + let isConfirmed = false + const setIsConfirmed = vi.fn((confirmed: boolean) => { + isConfirmed = confirmed }) -} -describe('ProtocolSetupLiquids', () => { + const render = (props: React.ComponentProps) => { + return renderWithProviders(, { + i18nInstance: i18n, + }) + } + let props: React.ComponentProps beforeEach(() => { - props = { runId: RUN_ID_1, setSetupScreen: vi.fn() } + props = { + runId: RUN_ID_1, + setSetupScreen: vi.fn(), + isConfirmed, + setIsConfirmed, + } vi.mocked(parseLiquidsInLoadOrder).mockReturnValue( MOCK_LIQUIDS_IN_LOAD_ORDER ) diff --git a/app/src/organisms/ProtocolSetupLiquids/index.tsx b/app/src/organisms/ProtocolSetupLiquids/index.tsx index 1fb10cdb79d..883054c6963 100644 --- a/app/src/organisms/ProtocolSetupLiquids/index.tsx +++ b/app/src/organisms/ProtocolSetupLiquids/index.tsx @@ -5,6 +5,7 @@ import { BORDERS, COLORS, DIRECTION_COLUMN, + DIRECTION_ROW, Flex, Icon, JUSTIFY_FLEX_END, @@ -12,6 +13,7 @@ import { StyledText, TYPOGRAPHY, JUSTIFY_SPACE_BETWEEN, + Chip, } from '@opentrons/components' import { parseLiquidsInLoadOrder, @@ -19,6 +21,8 @@ import { } from '@opentrons/api-client' import { MICRO_LITERS } from '@opentrons/shared-data' import { ODDBackButton } from '../../molecules/ODDBackButton' +import { SmallButton } from '../../atoms/buttons' + import { useMostRecentCompletedAnalysis } from '../LabwarePositionCheck/useMostRecentCompletedAnalysis' import { getTotalVolumePerLiquidId } from '../Devices/ProtocolRun/SetupLiquids/utils' import { LiquidDetails } from './LiquidDetails' @@ -29,13 +33,17 @@ import type { SetupScreens } from '../../pages/ProtocolSetup' export interface ProtocolSetupLiquidsProps { runId: string setSetupScreen: React.Dispatch> + isConfirmed: boolean + setIsConfirmed: (confirmed: boolean) => void } export function ProtocolSetupLiquids({ runId, setSetupScreen, + isConfirmed, + setIsConfirmed, }: ProtocolSetupLiquidsProps): JSX.Element { - const { t } = useTranslation('protocol_setup') + const { t, i18n } = useTranslation('protocol_setup') const protocolData = useMostRecentCompletedAnalysis(runId) const liquidsInLoadOrder = parseLiquidsInLoadOrder( protocolData?.liquids ?? [], @@ -43,12 +51,34 @@ export function ProtocolSetupLiquids({ ) return ( <> - { - setSetupScreen('prepare to run') - }} - /> + + { + setSetupScreen('prepare to run') + }} + /> + {isConfirmed ? ( + + ) : ( + { + setIsConfirmed(true) + setSetupScreen('prepare to run') + }} + /> + )} + > + lpcDisabledReason: string | null + launchLPC: () => void + LPCWizard: JSX.Element | null + isConfirmed: boolean + setIsConfirmed: (confirmed: boolean) => void +} + +export function ProtocolSetupOffsets({ + runId, + setSetupScreen, + isConfirmed, + setIsConfirmed, + launchLPC, + lpcDisabledReason, + LPCWizard, +}: ProtocolSetupOffsetsProps): JSX.Element { + const { t } = useTranslation('protocol_setup') + const { makeSnackbar } = useToaster() + const mostRecentAnalysis = useMostRecentCompletedAnalysis(runId) + const makeDisabledReasonSnackbar = (): void => { + if (lpcDisabledReason != null) { + makeSnackbar(lpcDisabledReason) + } + } + + const labwareDefinitions = getLabwareDefinitionsFromCommands( + mostRecentAnalysis?.commands ?? [] + ) + const { data: runRecord } = useNotifyRunQuery(runId, { staleTime: Infinity }) + const currentOffsets = runRecord?.data?.labwareOffsets ?? [] + const sortedOffsets: LabwareOffset[] = + currentOffsets.length > 0 + ? currentOffsets + .map(offset => ({ + ...offset, + // convert into date to sort + createdAt: new Date(offset.createdAt), + })) + .sort((a, b) => a.createdAt.getTime() - b.createdAt.getTime()) + .map(offset => ({ + ...offset, + // convert back into string + createdAt: offset.createdAt.toISOString(), + })) + : [] + const nonIdentityOffsets = getLatestCurrentOffsets(sortedOffsets) + return ( + <> + {LPCWizard} + {LPCWizard == null && ( + <> + + { + setSetupScreen('prepare to run') + }} + /> + {isConfirmed ? ( + + ) : ( + { + setIsConfirmed(true) + setSetupScreen('prepare to run') + }} + /> + )} + + + { + if (lpcDisabledReason != null) { + makeDisabledReasonSnackbar() + } else { + launchLPC() + } + }} + /> + + )} + + ) +} diff --git a/app/src/organisms/QuickTransferFlow/QuickTransferAdvancedSettings/Mix.tsx b/app/src/organisms/QuickTransferFlow/QuickTransferAdvancedSettings/Mix.tsx index c0df132b52a..46727cb1228 100644 --- a/app/src/organisms/QuickTransferFlow/QuickTransferAdvancedSettings/Mix.tsx +++ b/app/src/organisms/QuickTransferFlow/QuickTransferAdvancedSettings/Mix.tsx @@ -85,6 +85,7 @@ export function Mix(props: MixProps): JSX.Element { type: mixAction, mixSettings: undefined, }) + onBack() } else { setCurrentStep(2) } diff --git a/app/src/organisms/QuickTransferFlow/QuickTransferAdvancedSettings/index.tsx b/app/src/organisms/QuickTransferFlow/QuickTransferAdvancedSettings/index.tsx index 7ec21c7c857..34a36d2d4fb 100644 --- a/app/src/organisms/QuickTransferFlow/QuickTransferAdvancedSettings/index.tsx +++ b/app/src/organisms/QuickTransferFlow/QuickTransferAdvancedSettings/index.tsx @@ -74,7 +74,7 @@ export function QuickTransferAdvancedSettings( } else if (state.path === 'multiAspirate') { pipettePathValue = t('pipette_path_multi_aspirate') } else if (state.path === 'multiDispense') { - pipettePathValue = t('pipette_path_multi_dispense', { + pipettePathValue = t('pipette_path_multi_dispense_volume_blowout', { volume: state.disposalVolume, blowOutLocation: getBlowoutValueCopy(), }) @@ -160,7 +160,10 @@ export function QuickTransferAdvancedSettings( state.transferType === 'transfer' || state.transferType === 'distribute', onClick: () => { - if (state.transferType === 'transfer') { + if ( + state.transferType === 'transfer' || + state.transferType === 'distribute' + ) { setSelectedSetting('aspirate_mix') } else { makeSnackbar(t('advanced_setting_disabled') as string) @@ -240,7 +243,10 @@ export function QuickTransferAdvancedSettings( state.transferType === 'transfer' || state.transferType === 'consolidate', onClick: () => { - if (state.transferType === 'transfer') { + if ( + state.transferType === 'transfer' || + state.transferType === 'consolidate' + ) { setSelectedSetting('dispense_mix') } else { makeSnackbar(t('advanced_setting_disabled') as string) @@ -293,7 +299,10 @@ export function QuickTransferAdvancedSettings( { option: 'dispense_blow_out', copy: t('blow_out'), - value: i18n.format(getBlowoutValueCopy(), 'capitalize'), + value: + state.transferType === 'distribute' + ? t('disabled') + : i18n.format(getBlowoutValueCopy(), 'capitalize'), enabled: state.transferType !== 'distribute', onClick: () => { if (state.transferType === 'distribute') { diff --git a/app/src/organisms/QuickTransferFlow/SelectDestWells.tsx b/app/src/organisms/QuickTransferFlow/SelectDestWells.tsx index 71378fb6eb2..6860815ac08 100644 --- a/app/src/organisms/QuickTransferFlow/SelectDestWells.tsx +++ b/app/src/organisms/QuickTransferFlow/SelectDestWells.tsx @@ -10,12 +10,17 @@ import { LegacyStyledText, JUSTIFY_CENTER, } from '@opentrons/components' +import { getAllDefinitions } from '@opentrons/shared-data' import { getTopPortalEl } from '../../App/portal' import { Modal } from '../../molecules/Modal' import { ChildNavigation } from '../../organisms/ChildNavigation' import { useToaster } from '../../organisms/ToasterOven' import { WellSelection } from '../../organisms/WellSelection' +import { + CIRCULAR_WELL_96_PLATE_DEFINITION_URI, + RECTANGULAR_WELL_96_PLATE_DEFINITION_URI, +} from './SelectSourceWells' import type { SmallButton } from '../../atoms/buttons' import type { ModalHeaderBaseProps } from '../../molecules/Modal/types' @@ -107,9 +112,17 @@ export function SelectDestWells(props: SelectDestWellsProps): JSX.Element { setSelectedWells({}) }, } - const labwareDefinition = + let labwareDefinition = state.destination === 'source' ? state.source : state.destination - + if (labwareDefinition?.parameters.format === '96Standard') { + const allDefinitions = getAllDefinitions() + if (Object.values(labwareDefinition.wells)[0].shape === 'circular') { + labwareDefinition = allDefinitions[CIRCULAR_WELL_96_PLATE_DEFINITION_URI] + } else { + labwareDefinition = + allDefinitions[RECTANGULAR_WELL_96_PLATE_DEFINITION_URI] + } + } return ( <> {createPortal( @@ -143,7 +156,7 @@ export function SelectDestWells(props: SelectDestWellsProps): JSX.Element { left="0" width="100%" > - {state.destination != null && state.source != null ? ( + {labwareDefinition != null ? ( { setSelectedWells(prevWells => without(Object.keys(prevWells), ...wells).reduce( diff --git a/app/src/organisms/QuickTransferFlow/SelectSourceWells.tsx b/app/src/organisms/QuickTransferFlow/SelectSourceWells.tsx index d7628bbcefe..342bb157193 100644 --- a/app/src/organisms/QuickTransferFlow/SelectSourceWells.tsx +++ b/app/src/organisms/QuickTransferFlow/SelectSourceWells.tsx @@ -7,6 +7,7 @@ import { POSITION_FIXED, SPACING, } from '@opentrons/components' +import { getAllDefinitions } from '@opentrons/shared-data' import { ChildNavigation } from '../../organisms/ChildNavigation' import { WellSelection } from '../../organisms/WellSelection' @@ -24,6 +25,11 @@ interface SelectSourceWellsProps { dispatch: React.Dispatch } +export const CIRCULAR_WELL_96_PLATE_DEFINITION_URI = + 'opentrons/thermoscientificnunc_96_wellplate_2000ul/1' +export const RECTANGULAR_WELL_96_PLATE_DEFINITION_URI = + 'opentrons/usascientific_96_wellplate_2.4ml_deep/1' + export function SelectSourceWells(props: SelectSourceWellsProps): JSX.Element { const { onNext, onBack, state, dispatch } = props const { i18n, t } = useTranslation(['quick_transfer', 'shared']) @@ -50,6 +56,17 @@ export function SelectSourceWells(props: SelectSourceWellsProps): JSX.Element { setSelectedWells({}) }, } + let displayLabwareDefinition = state.source + if (state.source?.parameters.format === '96Standard') { + const allDefinitions = getAllDefinitions() + if (Object.values(state.source.wells)[0].shape === 'circular') { + displayLabwareDefinition = + allDefinitions[CIRCULAR_WELL_96_PLATE_DEFINITION_URI] + } else { + displayLabwareDefinition = + allDefinitions[RECTANGULAR_WELL_96_PLATE_DEFINITION_URI] + } + } return ( <> @@ -71,14 +88,14 @@ export function SelectSourceWells(props: SelectSourceWellsProps): JSX.Element { left="0" width="100%" > - {state.source != null ? ( + {state.source != null && displayLabwareDefinition != null ? ( { setSelectedWells(prevWells => without(Object.keys(prevWells), ...wells).reduce( diff --git a/app/src/organisms/RunProgressMeter/__tests__/RunProgressMeter.test.tsx b/app/src/organisms/RunProgressMeter/__tests__/RunProgressMeter.test.tsx index 0cab1ef5adb..10ecdb7bf9e 100644 --- a/app/src/organisms/RunProgressMeter/__tests__/RunProgressMeter.test.tsx +++ b/app/src/organisms/RunProgressMeter/__tests__/RunProgressMeter.test.tsx @@ -13,7 +13,10 @@ import { } from '@opentrons/api-client' import { i18n } from '../../../i18n' -import { InterventionModal } from '../../InterventionModal' +import { + useInterventionModal, + InterventionModal, +} from '../../InterventionModal' import { ProgressBar } from '../../../atoms/ProgressBar' import { useRunStatus } from '../../RunTimeControl/hooks' import { useMostRecentCompletedAnalysis } from '../../LabwarePositionCheck/useMostRecentCompletedAnalysis' @@ -27,11 +30,7 @@ import { mockUseCommandResultNonDeterministic, NON_DETERMINISTIC_COMMAND_KEY, } from '../__fixtures__' -import { - mockMoveLabwareCommandFromSlot, - mockPauseCommandWithStartTime, - mockRunData, -} from '../../InterventionModal/__fixtures__' + import { RunProgressMeter } from '..' import { renderWithProviders } from '../../../__testing-utils__' import { useLastRunCommand } from '../../Devices/hooks/useLastRunCommand' @@ -70,7 +69,7 @@ describe('RunProgressMeter', () => { beforeEach(() => { vi.mocked(ProgressBar).mockReturnValue(
MOCK PROGRESS BAR
) vi.mocked(InterventionModal).mockReturnValue( -
MOCK INTERVENTION MODAL
+
MOCK_INTERVENTION_MODAL
) vi.mocked(useRunStatus).mockReturnValue(RUN_STATUS_RUNNING) when(useMostRecentCompletedAnalysis) @@ -96,6 +95,10 @@ describe('RunProgressMeter', () => { currentStepNumber: null, hasRunDiverged: true, }) + vi.mocked(useInterventionModal).mockReturnValue({ + showModal: false, + modalProps: {} as any, + }) props = { runId: NON_DETERMINISTIC_RUN_ID, @@ -119,31 +122,18 @@ describe('RunProgressMeter', () => { screen.getByText('Not started yet') screen.getByText('Download run log') }) - it('should render an intervention modal when lastRunCommand is a pause command', () => { - vi.mocked(useNotifyAllCommandsQuery).mockReturnValue({ - data: { data: [mockPauseCommandWithStartTime], meta: { totalLength: 1 } }, - } as any) - vi.mocked(useNotifyRunQuery).mockReturnValue({ - data: { data: { labware: [] } }, - } as any) - vi.mocked(useCommandQuery).mockReturnValue({ data: null } as any) - vi.mocked(useMostRecentCompletedAnalysis).mockReturnValue({} as any) - render(props) - }) - it('should render an intervention modal when lastRunCommand is a move labware command', () => { - vi.mocked(useNotifyAllCommandsQuery).mockReturnValue({ - data: { - data: [mockMoveLabwareCommandFromSlot], - meta: { totalLength: 1 }, - }, - } as any) - vi.mocked(useNotifyRunQuery).mockReturnValue({ - data: { data: mockRunData }, - } as any) + it('should render an intervention modal when showInterventionModal is true', () => { vi.mocked(useCommandQuery).mockReturnValue({ data: null } as any) - vi.mocked(useMostRecentCompletedAnalysis).mockReturnValue({} as any) + vi.mocked(useRunStatus).mockReturnValue(RUN_STATUS_IDLE) + vi.mocked(useInterventionModal).mockReturnValue({ + showModal: true, + modalProps: {} as any, + }) + render(props) + + screen.getByText('MOCK_INTERVENTION_MODAL') }) it('should render the correct run status when run status is completed', () => { diff --git a/app/src/organisms/RunProgressMeter/index.tsx b/app/src/organisms/RunProgressMeter/index.tsx index 5f120904a41..eecf73a96f9 100644 --- a/app/src/organisms/RunProgressMeter/index.tsx +++ b/app/src/organisms/RunProgressMeter/index.tsx @@ -30,17 +30,15 @@ import { useMostRecentCompletedAnalysis } from '../LabwarePositionCheck/useMostR import { getModalPortalEl } from '../../App/portal' import { Tooltip } from '../../atoms/Tooltip' import { useRunStatus } from '../RunTimeControl/hooks' -import { InterventionModal } from '../InterventionModal' +import { InterventionModal, useInterventionModal } from '../InterventionModal' import { ProgressBar } from '../../atoms/ProgressBar' import { useDownloadRunLog, useRobotType } from '../Devices/hooks' import { InterventionTicks } from './InterventionTicks' -import { isInterventionCommand } from '../InterventionModal/utils' import { useNotifyRunQuery, useNotifyAllCommandsQuery, } from '../../resources/runs' import { useRunningStepCounts } from '../../resources/protocols/hooks' -import { TERMINAL_RUN_STATUSES } from './constants' import { useRunProgressCopy } from './hooks' interface RunProgressMeterProps { @@ -51,10 +49,6 @@ interface RunProgressMeterProps { } export function RunProgressMeter(props: RunProgressMeterProps): JSX.Element { const { runId, robotName, makeHandleJumpToStep, resumeRunHandler } = props - const [ - interventionModalCommandKey, - setInterventionModalCommandKey, - ] = React.useState(null) const { t } = useTranslation('run_details') const robotType = useRobotType(robotName) const runStatus = useRunStatus(runId) @@ -91,23 +85,6 @@ export function RunProgressMeter(props: RunProgressMeterProps): JSX.Element { const { downloadRunLog } = useDownloadRunLog(robotName, runId) - React.useEffect(() => { - if ( - lastRunCommand != null && - interventionModalCommandKey != null && - lastRunCommand.key !== interventionModalCommandKey - ) { - // set intervention modal command key to null if different from current command key - setInterventionModalCommandKey(null) - } else if ( - lastRunCommand?.key != null && - isInterventionCommand(lastRunCommand) && - interventionModalCommandKey === null - ) { - setInterventionModalCommandKey(lastRunCommand.key) - } - }, [lastRunCommand, interventionModalCommandKey]) - const onDownloadClick: React.MouseEventHandler = e => { if (downloadIsDisabled) return false e.preventDefault() @@ -115,6 +92,17 @@ export function RunProgressMeter(props: RunProgressMeterProps): JSX.Element { downloadRunLog() } + const { + showModal: showIntervention, + modalProps: interventionProps, + } = useInterventionModal({ + robotName, + runStatus, + runData, + analysis, + lastRunCommand, + }) + const { progressPercentage, stepCountStr, @@ -132,20 +120,11 @@ export function RunProgressMeter(props: RunProgressMeterProps): JSX.Element { return ( <> - {interventionModalCommandKey != null && - lastRunCommand != null && - isInterventionCommand(lastRunCommand) && - analysisCommands != null && - runStatus != null && - runData != null && - !TERMINAL_RUN_STATUSES.includes(runStatus) + {showIntervention ? createPortal( , getModalPortalEl() ) diff --git a/app/src/pages/Devices/ProtocolRunDetails/index.tsx b/app/src/pages/Devices/ProtocolRunDetails/index.tsx index 2935bc86100..62798b55b4f 100644 --- a/app/src/pages/Devices/ProtocolRunDetails/index.tsx +++ b/app/src/pages/Devices/ProtocolRunDetails/index.tsx @@ -10,6 +10,8 @@ import { Box, COLORS, DIRECTION_COLUMN, + DIRECTION_ROW, + JUSTIFY_SPACE_AROUND, Flex, LegacyStyledText, OVERFLOW_SCROLL, @@ -29,7 +31,11 @@ import { } from '../../../organisms/Devices/hooks' import { ProtocolRunHeader } from '../../../organisms/Devices/ProtocolRun/ProtocolRunHeader' import { RunPreview } from '../../../organisms/RunPreview' -import { ProtocolRunSetup } from '../../../organisms/Devices/ProtocolRun/ProtocolRunSetup' +import { + ProtocolRunSetup, + initialMissingSteps, +} from '../../../organisms/Devices/ProtocolRun/ProtocolRunSetup' +import { BackToTopButton } from '../../../organisms/Devices/ProtocolRun/BackToTopButton' import { ProtocolRunModuleControls } from '../../../organisms/Devices/ProtocolRun/ProtocolRunModuleControls' import { ProtocolRunRuntimeParameters } from '../../../organisms/Devices/ProtocolRun/ProtocolRunRunTimeParameters' import { useCurrentRunId } from '../../../resources/runs' @@ -134,7 +140,6 @@ export function ProtocolRunDetails(): JSX.Element | null { React.useEffect(() => { dispatch(fetchProtocols()) }, [dispatch]) - return robot != null ? ( + >(initialMissingSteps()) + const makeHandleScrollToStep = (i: number) => () => { listRef.current?.scrollToIndex(i, true, -1 * JUMP_OFFSET_FROM_TOP_PX) } @@ -193,37 +202,68 @@ function PageContents(props: PageContentsProps): JSX.Element { setJumpedIndex(i) } const protocolRunDetailsContentByTab: { - [K in ProtocolRunDetailsTab]: JSX.Element | null + [K in ProtocolRunDetailsTab]: { + content: JSX.Element | null + backToTop: JSX.Element | null + } } = { - setup: ( - - ), - 'runtime-parameters': , - 'module-controls': ( - - ), - 'run-preview': ( - - ), + setup: { + content: ( + + ), + backToTop: ( + + + + ), + }, + 'runtime-parameters': { + content: , + backToTop: null, + }, + 'module-controls': { + content: ( + + ), + backToTop: null, + }, + 'run-preview': { + content: ( + + ), + backToTop: null, + }, } - - const protocolRunDetailsContent = protocolRunDetailsContentByTab[ - protocolRunDetailsTab - ] ?? ( + const tabDetails = protocolRunDetailsContentByTab[protocolRunDetailsTab] ?? { // default to the setup tab if no tab or nonexistent tab is passed as a param - - - ) + content: ( + + ), + backToTop: null, + } + const { content, backToTop } = tabDetails return ( <> @@ -232,6 +272,7 @@ function PageContents(props: PageContentsProps): JSX.Element { robotName={robotName} runId={runId} makeHandleJumpToStep={makeHandleJumpToStep} + missingSetupSteps={missingSteps} /> - {protocolRunDetailsContent} + {content} + {backToTop} ) } diff --git a/app/src/pages/ProtocolSetup/ConfirmSetupStepsCompleteModal.tsx b/app/src/pages/ProtocolSetup/ConfirmSetupStepsCompleteModal.tsx new file mode 100644 index 00000000000..1757704e597 --- /dev/null +++ b/app/src/pages/ProtocolSetup/ConfirmSetupStepsCompleteModal.tsx @@ -0,0 +1,68 @@ +import * as React from 'react' +import { useTranslation } from 'react-i18next' + +import { + DIRECTION_COLUMN, + Flex, + SPACING, + LegacyStyledText, +} from '@opentrons/components' + +import { SmallButton } from '../../atoms/buttons' +import { Modal } from '../../molecules/Modal' + +import type { ModalHeaderBaseProps } from '../../molecules/Modal/types' + +interface ConfirmSetupStepsCompleteModalProps { + onCloseClick: () => void + onConfirmClick: () => void + missingSteps: string[] +} + +export function ConfirmSetupStepsCompleteModal({ + onCloseClick, + missingSteps, + onConfirmClick, +}: ConfirmSetupStepsCompleteModalProps): JSX.Element { + const { i18n, t } = useTranslation(['protocol_setup', 'shared']) + const modalHeader: ModalHeaderBaseProps = { + title: t('are_you_sure_you_want_to_proceed'), + hasExitIcon: true, + } + + const handleStartRun = (): void => { + onConfirmClick() + onCloseClick() + } + + return ( + + + + {t('you_havent_confirmed', { + missingSteps: new Intl.ListFormat('en', { + style: 'short', + type: 'conjunction', + }).format(missingSteps), + })} + + + { + onCloseClick() + }} + /> + + + + + ) +} diff --git a/app/src/pages/ProtocolSetup/__tests__/ProtocolSetup.test.tsx b/app/src/pages/ProtocolSetup/__tests__/ProtocolSetup.test.tsx index 1be58ae82f8..5479f4693bd 100644 --- a/app/src/pages/ProtocolSetup/__tests__/ProtocolSetup.test.tsx +++ b/app/src/pages/ProtocolSetup/__tests__/ProtocolSetup.test.tsx @@ -2,7 +2,7 @@ import * as React from 'react' import { Route, MemoryRouter, Routes } from 'react-router-dom' import { fireEvent, screen } from '@testing-library/react' import { when } from 'vitest-when' -import { vi, it, describe, expect, beforeEach, afterEach } from 'vitest' +import { vi, it, describe, expect, beforeEach } from 'vitest' import { RUN_STATUS_IDLE, RUN_STATUS_STOPPED } from '@opentrons/api-client' import { @@ -39,10 +39,13 @@ import { ANALYTICS_PROTOCOL_RUN_ACTION } from '../../../redux/analytics' import { ProtocolSetupLiquids } from '../../../organisms/ProtocolSetupLiquids' import { getProtocolModulesInfo } from '../../../organisms/Devices/ProtocolRun/utils/getProtocolModulesInfo' import { ProtocolSetupModulesAndDeck } from '../../../organisms/ProtocolSetupModulesAndDeck' +import { ProtocolSetupLabware } from '../../../organisms/ProtocolSetupLabware' +import { ProtocolSetupOffsets } from '../../../organisms/ProtocolSetupOffsets' import { getUnmatchedModulesForProtocol } from '../../../organisms/ProtocolSetupModulesAndDeck/utils' import { useLaunchLPC } from '../../../organisms/LabwarePositionCheck/useLaunchLPC' import { ConfirmCancelRunModal } from '../../../organisms/OnDeviceDisplay/RunningProtocol' import { mockProtocolModuleInfo } from '../../../organisms/ProtocolSetupInstruments/__fixtures__' +import { getIncompleteInstrumentCount } from '../../../organisms/ProtocolSetupInstruments/utils' import { useProtocolHasRunTimeParameters, useRunControls, @@ -51,6 +54,7 @@ import { import { useIsHeaterShakerInProtocol } from '../../../organisms/ModuleCard/hooks' import { useDeckConfigurationCompatibility } from '../../../resources/deck_configuration/hooks' import { ConfirmAttachedModal } from '../../../pages/ProtocolSetup/ConfirmAttachedModal' +import { ConfirmSetupStepsCompleteModal } from '../../../pages/ProtocolSetup/ConfirmSetupStepsCompleteModal' import { ProtocolSetup } from '../../../pages/ProtocolSetup' import { useNotifyRunQuery } from '../../../resources/runs' import { ViewOnlyParameters } from '../../../organisms/ProtocolSetupParameters/ViewOnlyParameters' @@ -99,12 +103,15 @@ vi.mock('../../../organisms/ProtocolSetupParameters/ViewOnlyParameters') vi.mock( '../../../organisms/LabwarePositionCheck/useMostRecentCompletedAnalysis' ) +vi.mock('../../../organisms/ProtocolSetupInstruments/utils') vi.mock('../../../organisms/Devices/ProtocolRun/utils/getProtocolModulesInfo') vi.mock('../../../organisms/ProtocolSetupModulesAndDeck') vi.mock('../../../organisms/ProtocolSetupModulesAndDeck/utils') vi.mock('../../../organisms/OnDeviceDisplay/RunningProtocol') vi.mock('../../../organisms/RunTimeControl/hooks') vi.mock('../../../organisms/ProtocolSetupLiquids') +vi.mock('../../../organisms/ProtocolSetupLabware') +vi.mock('../../../organisms/ProtocolSetupOffsets') vi.mock('../../../organisms/ModuleCard/hooks') vi.mock('../../../redux/discovery/selectors') vi.mock('../ConfirmAttachedModal') @@ -112,6 +119,7 @@ vi.mock('../../../organisms/ToasterOven') vi.mock('../../../resources/deck_configuration/hooks') vi.mock('../../../resources/runs') vi.mock('../../../resources/deck_configuration') +vi.mock('../ConfirmSetupStepsCompleteModal') const render = (path = '/') => { return renderWithProviders( @@ -126,6 +134,12 @@ const render = (path = '/') => { ) } +const MockProtocolSetupLabware = vi.mocked(ProtocolSetupLabware) +const MockProtocolSetupLiquids = vi.mocked(ProtocolSetupLiquids) +const MockProtocolSetupOffsets = vi.mocked(ProtocolSetupOffsets) +const MockConfirmSetupStepsCompleteModal = vi.mocked( + ConfirmSetupStepsCompleteModal +) const ROBOT_NAME = 'fake-robot-name' const RUN_ID = 'my-run-id' const ROBOT_SERIAL_NUMBER = 'OT123' @@ -192,6 +206,30 @@ describe('ProtocolSetup', () => { beforeEach(() => { mockLaunchLPC = vi.fn() mockNavigate = vi.fn() + MockProtocolSetupLiquids.mockImplementation( + vi.fn(({ setIsConfirmed, setSetupScreen }) => { + setIsConfirmed(true) + setSetupScreen('prepare to run') + return
Mock ProtocolSetupLiquids
+ }) + ) + MockProtocolSetupLabware.mockImplementation( + vi.fn(({ setIsConfirmed, setSetupScreen }) => { + setIsConfirmed(true) + setSetupScreen('prepare to run') + return
Mock ProtocolSetupLabware
+ }) + ) + MockProtocolSetupOffsets.mockImplementation( + vi.fn(({ setIsConfirmed, setSetupScreen }) => { + setIsConfirmed(true) + setSetupScreen('prepare to run') + return
Mock ProtocolSetupOffsets
+ }) + ) + MockConfirmSetupStepsCompleteModal.mockReturnValue( +
Mock ConfirmSetupStepsCompleteModal
+ ) vi.mocked(useLPCDisabledReason).mockReturnValue(null) vi.mocked(useAttachedModules).mockReturnValue([]) vi.mocked(useModuleCalibrationStatus).mockReturnValue({ complete: true }) @@ -290,10 +328,6 @@ describe('ProtocolSetup', () => { .thenReturn({ trackProtocolRunEvent: mockTrackProtocolRunEvent }) }) - afterEach(() => { - vi.resetAllMocks() - }) - it('should render text, image, and buttons', () => { render(`/runs/${RUN_ID}/setup/`) screen.getByText('Prepare to run') @@ -305,9 +339,47 @@ describe('ProtocolSetup', () => { }) it('should play protocol when click play button', () => { + vi.mocked(useProtocolAnalysisAsDocumentQuery).mockReturnValue({ + data: { ...mockRobotSideAnalysis, liquids: mockLiquids }, + } as any) + when(vi.mocked(getProtocolModulesInfo)) + .calledWith( + { ...mockRobotSideAnalysis, liquids: mockLiquids }, + flexDeckDefV5 as any + ) + .thenReturn(mockProtocolModuleInfo) + when(vi.mocked(getUnmatchedModulesForProtocol)) + .calledWith([], mockProtocolModuleInfo) + .thenReturn({ missingModuleIds: [], remainingAttachedModules: [] }) + vi.mocked(getIncompleteInstrumentCount).mockReturnValue(0) + MockProtocolSetupLiquids.mockImplementation( + vi.fn(({ setIsConfirmed, setSetupScreen }) => { + setIsConfirmed(true) + setSetupScreen('prepare to run') + return
Mock ProtocolSetupLiquids
+ }) + ) + MockProtocolSetupLabware.mockImplementation( + vi.fn(({ setIsConfirmed, setSetupScreen }) => { + setIsConfirmed(true) + setSetupScreen('prepare to run') + return
Mock ProtocolSetupLabware
+ }) + ) + MockProtocolSetupOffsets.mockImplementation( + vi.fn(({ setIsConfirmed, setSetupScreen }) => { + setIsConfirmed(true) + setSetupScreen('prepare to run') + return
Mock ProtocolSetupOffsets
+ }) + ) render(`/runs/${RUN_ID}/setup/`) + fireEvent.click(screen.getByText('Labware Position Check')) + fireEvent.click(screen.getByText('Labware')) + fireEvent.click(screen.getByText('Liquids')) expect(mockPlay).toBeCalledTimes(0) fireEvent.click(screen.getByRole('button', { name: 'play' })) + expect(MockConfirmSetupStepsCompleteModal).toBeCalledTimes(0) expect(mockPlay).toBeCalledTimes(1) }) @@ -348,7 +420,25 @@ describe('ProtocolSetup', () => { render(`/runs/${RUN_ID}/setup/`) screen.getByText('1 initial liquid') fireEvent.click(screen.getByText('Liquids')) - expect(vi.mocked(ProtocolSetupLiquids)).toHaveBeenCalled() + expect(MockProtocolSetupLiquids).toHaveBeenCalled() + }) + + it('should launch protocol setup labware screen when click labware', () => { + vi.mocked(useProtocolAnalysisAsDocumentQuery).mockReturnValue({ + data: { ...mockRobotSideAnalysis, liquids: mockLiquids }, + } as any) + when(vi.mocked(getProtocolModulesInfo)) + .calledWith( + { ...mockRobotSideAnalysis, liquids: mockLiquids }, + flexDeckDefV5 as any + ) + .thenReturn(mockProtocolModuleInfo) + when(vi.mocked(getUnmatchedModulesForProtocol)) + .calledWith([], mockProtocolModuleInfo) + .thenReturn({ missingModuleIds: [], remainingAttachedModules: [] }) + render(`/runs/${RUN_ID}/setup`) + fireEvent.click(screen.getByTestId('SetupButton_Labware')) + expect(MockProtocolSetupLabware).toHaveBeenCalled() }) it('should launch view only parameters screen when click parameters', () => { @@ -376,14 +466,14 @@ describe('ProtocolSetup', () => { expect(vi.mocked(ViewOnlyParameters)).toHaveBeenCalled() }) - it('should launch LPC when clicked', () => { - vi.mocked(useLPCDisabledReason).mockReturnValue(null) + it('should launch offsets screen when click offsets', () => { + MockProtocolSetupOffsets.mockImplementation( + vi.fn(() =>
Mock ProtocolSetupOffsets
) + ) render(`/runs/${RUN_ID}/setup/`) - screen.getByText(/Recommended/) - screen.getByText(/1 offset applied/) fireEvent.click(screen.getByText('Labware Position Check')) - expect(mockLaunchLPC).toHaveBeenCalled() - screen.getByText('mock LPC Wizard') + expect(MockProtocolSetupOffsets).toHaveBeenCalled() + screen.getByText(/Mock ProtocolSetupOffsets/) }) it('should render a confirmation modal when heater-shaker is in a protocol and it is not shaking', () => { @@ -416,7 +506,21 @@ describe('ProtocolSetup', () => { }) it('calls trackProtocolRunEvent when tapping play button', () => { + vi.mocked(useProtocolAnalysisAsDocumentQuery).mockReturnValue({ + data: { ...mockRobotSideAnalysis, liquids: mockLiquids }, + } as any) + when(vi.mocked(getProtocolModulesInfo)) + .calledWith( + { ...mockRobotSideAnalysis, liquids: mockLiquids }, + flexDeckDefV5 as any + ) + .thenReturn(mockProtocolModuleInfo) + when(vi.mocked(getUnmatchedModulesForProtocol)) + .calledWith([], mockProtocolModuleInfo) + .thenReturn({ missingModuleIds: [], remainingAttachedModules: [] }) + vi.mocked(getIncompleteInstrumentCount).mockReturnValue(0) render(`/runs/${RUN_ID}/setup/`) + fireEvent.click(screen.getByRole('button', { name: 'play' })) expect(mockTrackProtocolRunEvent).toBeCalledTimes(1) expect(mockTrackProtocolRunEvent).toHaveBeenCalledWith({ diff --git a/app/src/pages/ProtocolSetup/index.tsx b/app/src/pages/ProtocolSetup/index.tsx index 36ce4220bcb..f152b0cc44a 100644 --- a/app/src/pages/ProtocolSetup/index.tsx +++ b/app/src/pages/ProtocolSetup/index.tsx @@ -60,6 +60,7 @@ import { getProtocolModulesInfo } from '../../organisms/Devices/ProtocolRun/util import { ProtocolSetupLabware } from '../../organisms/ProtocolSetupLabware' import { ProtocolSetupModulesAndDeck } from '../../organisms/ProtocolSetupModulesAndDeck' import { ProtocolSetupLiquids } from '../../organisms/ProtocolSetupLiquids' +import { ProtocolSetupOffsets } from '../../organisms/ProtocolSetupOffsets' import { ProtocolSetupInstruments } from '../../organisms/ProtocolSetupInstruments' import { ProtocolSetupDeckConfiguration } from '../../organisms/ProtocolSetupDeckConfiguration' import { useLaunchLPC } from '../../organisms/LabwarePositionCheck/useLaunchLPC' @@ -85,6 +86,7 @@ import { } from '../../redux/analytics' import { getIsHeaterShakerAttached } from '../../redux/config' import { ConfirmAttachedModal } from './ConfirmAttachedModal' +import { ConfirmSetupStepsCompleteModal } from './ConfirmSetupStepsCompleteModal' import { getLatestCurrentOffsets } from '../../organisms/Devices/ProtocolRun/SetupLabwarePositionCheck/utils' import { CloseButton, PlayButton } from './Buttons' import { useDeckConfigurationCompatibility } from '../../resources/deck_configuration/hooks' @@ -118,6 +120,8 @@ interface ProtocolSetupStepProps { subDetail?: string | null // disallow click handler, disabled styling disabled?: boolean + // disallow click handler, don't show CTA icons, allow styling + interactionDisabled?: boolean // display the reason the setup step is disabled disabledReason?: string | null // optional description @@ -137,12 +141,14 @@ export function ProtocolSetupStep({ detail, subDetail, disabled = false, + interactionDisabled = false, disabledReason, description, hasRightIcon = true, hasLeftIcon = true, fontSize = 'p', }: ProtocolSetupStepProps): JSX.Element { + const isInteractionDisabled = interactionDisabled || disabled const backgroundColorByStepStatus = { ready: COLORS.green35, 'not ready': COLORS.yellow35, @@ -185,9 +191,12 @@ export function ProtocolSetupStep({ return ( { - !disabled ? onClickSetupStep() : makeDisabledReasonSnackbar() + !isInteractionDisabled + ? onClickSetupStep() + : makeDisabledReasonSnackbar() }} width="100%" + data-testid={`SetupButton_${title}`} > {detail} @@ -249,7 +257,7 @@ export function ProtocolSetupStep({ {subDetail} - {disabled || !hasRightIcon ? null : ( + {interactionDisabled || !hasRightIcon ? null : ( > confirmAttachment: () => void + confirmStepsComplete: () => void play: () => void robotName: string runRecord: Run | null + labwareConfirmed: boolean + liquidsConfirmed: boolean + offsetsConfirmed: boolean } function PrepareToRun({ @@ -280,6 +292,10 @@ function PrepareToRun({ play, robotName, runRecord, + labwareConfirmed, + liquidsConfirmed, + offsetsConfirmed, + confirmStepsComplete, }: PrepareToRunProps): JSX.Element { const { t, i18n } = useTranslation(['protocol_setup', 'shared']) const navigate = useNavigate() @@ -335,7 +351,6 @@ function PrepareToRun({ }, [mostRecentAnalysis?.status]) const robotType = useRobotType(robotName) - const { launchLPC, LPCWizard } = useLaunchLPC(runId, robotType, protocolName) const onConfirmCancelClose = (): void => { setShowConfirmCancelModal(false) @@ -381,12 +396,7 @@ function PrepareToRun({ : null const isMissingModules = missingModuleIds.length > 0 - const lpcDisabledReason = useLPCDisabledReason({ - runId, - hasMissingModulesForOdd: isMissingModules, - hasMissingCalForOdd: - incompleteInstrumentCount != null && incompleteInstrumentCount > 0, - }) + const moduleCalibrationStatus = useModuleCalibrationStatus(robotName, runId) const runTimeParameters = mostRecentAnalysis?.runTimeParameters ?? [] @@ -510,24 +520,25 @@ function PrepareToRun({ if (isDoorOpen) { makeSnackbar(t('shared:close_robot_door') as string) } else { - if ( - isHeaterShakerInProtocol && - isReadyToRun && - runStatus === RUN_STATUS_IDLE - ) { - confirmAttachment() - } else { - if (isReadyToRun) { + if (isReadyToRun) { + if (runStatus === RUN_STATUS_IDLE && isHeaterShakerInProtocol) { + confirmAttachment() + } else if ( + runStatus === RUN_STATUS_IDLE && + !(labwareConfirmed && offsetsConfirmed && liquidsConfirmed) + ) { + confirmStepsComplete() + } else { play() trackProtocolRunEvent({ name: ANALYTICS_PROTOCOL_RUN_ACTION.START, properties: robotAnalyticsData ?? {}, }) - } else { - makeSnackbar( - i18n.format(t('complete_setup_before_proceeding'), 'capitalize') - ) } + } else { + makeSnackbar( + i18n.format(t('complete_setup_before_proceeding'), 'capitalize') + ) } } } @@ -752,22 +763,16 @@ function PrepareToRun({ /> { - launchLPC() + setSetupScreen('offsets') }} title={t('labware_position_check')} - detail={t( - lpcDisabledReason != null - ? 'currently_unavailable' - : 'recommended' - )} + detail={t('recommended')} subDetail={ latestCurrentOffsets.length > 0 ? t('offsets_applied', { count: latestCurrentOffsets.length }) : null } - status="general" - disabled={lpcDisabledReason != null} - disabledReason={lpcDisabledReason} + status={offsetsConfirmed ? 'ready' : 'general'} /> { @@ -776,25 +781,25 @@ function PrepareToRun({ title={t('parameters')} detail={parametersDetail} subDetail={null} - status="general" - disabled={!hasRunTimeParameters} + status="ready" + interactionDisabled={!hasRunTimeParameters} /> { setSetupScreen('labware') }} - title={t('labware')} + title={i18n.format(t('labware'), 'capitalize')} detail={labwareDetail} subDetail={labwareSubDetail} - status="general" + status={labwareConfirmed ? 'ready' : 'general'} disabled={labwareDetail == null} /> { setSetupScreen('liquids') }} - title={t('liquids')} - status="general" + title={i18n.format(t('liquids'), 'capitalize')} + status={liquidsConfirmed ? 'ready' : 'general'} detail={ liquidsInProtocol.length > 0 ? t('initial_liquids_num', { @@ -809,7 +814,6 @@ function PrepareToRun({ )} - {LPCWizard} {showConfirmCancelModal ? ( () as OnDeviceRouteParams const { data: runRecord } = useNotifyRunQuery(runId, { staleTime: Infinity }) const { analysisErrors } = useProtocolAnalysisErrors(runId) + const { t } = useTranslation(['protocol_setup']) const localRobot = useSelector(getLocalRobot) + const robotName = localRobot?.name != null ? localRobot.name : 'no name' const robotSerialNumber = localRobot?.status != null ? getRobotSerialNumber(localRobot) : null const trackEvent = useTrackEvent() @@ -849,7 +856,69 @@ export function ProtocolSetup(): JSX.Element { showAnalysisFailedModal, setShowAnalysisFailedModal, ] = React.useState(true) + const robotType = useRobotType(robotName) + const attachedModules = + useAttachedModules({ + refetchInterval: FETCH_DURATION_MS, + }) ?? [] + const protocolId = runRecord?.data?.protocolId ?? null + const { data: protocolRecord } = useProtocolQuery(protocolId, { + staleTime: Infinity, + }) + const mostRecentAnalysisSummary = last(protocolRecord?.data.analysisSummaries) + const [ + isPollingForCompletedAnalysis, + setIsPollingForCompletedAnalysis, + ] = React.useState(mostRecentAnalysisSummary?.status !== 'completed') + const { + data: mostRecentAnalysis = null, + } = useProtocolAnalysisAsDocumentQuery( + protocolId, + last(protocolRecord?.data.analysisSummaries)?.id ?? null, + { + enabled: protocolRecord != null && isPollingForCompletedAnalysis, + refetchInterval: ANALYSIS_POLL_MS, + } + ) + + React.useEffect(() => { + if (mostRecentAnalysis?.status === 'completed') { + setIsPollingForCompletedAnalysis(false) + } else { + setIsPollingForCompletedAnalysis(true) + } + }, [mostRecentAnalysis?.status]) + const deckDef = getDeckDefFromRobotType(robotType) + + const protocolModulesInfo = + mostRecentAnalysis != null + ? getProtocolModulesInfo(mostRecentAnalysis, deckDef) + : [] + + const { missingModuleIds } = getUnmatchedModulesForProtocol( + attachedModules, + protocolModulesInfo + ) + const isMissingModules = missingModuleIds.length > 0 + const { data: attachedInstruments } = useInstrumentsQuery() + + const incompleteInstrumentCount: number | null = + mostRecentAnalysis != null && attachedInstruments != null + ? getIncompleteInstrumentCount(mostRecentAnalysis, attachedInstruments) + : null + const lpcDisabledReason = useLPCDisabledReason({ + runId, + hasMissingModulesForOdd: isMissingModules, + hasMissingCalForOdd: + incompleteInstrumentCount != null && incompleteInstrumentCount > 0, + }) + const protocolName = + protocolRecord?.data.metadata.protocolName ?? + protocolRecord?.data.files[0].name ?? + '' + + const { launchLPC, LPCWizard } = useLaunchLPC(runId, robotType, protocolName) const handleProceedToRunClick = (): void => { trackEvent({ name: ANALYTICS_PROTOCOL_PROCEED_TO_RUN, @@ -862,8 +931,8 @@ export function ProtocolSetup(): JSX.Element { ) const { confirm: confirmAttachment, - showConfirmation: showConfirmationModal, - cancel: cancelExit, + showConfirmation: showHSConfirmationModal, + cancel: cancelExitHSConfirmation, } = useConditionalConfirm( handleProceedToRunClick, !configBypassHeaterShakerAttachmentConfirmation @@ -872,6 +941,22 @@ export function ProtocolSetup(): JSX.Element { const [providedFixtureOptions, setProvidedFixtureOptions] = React.useState< CutoutFixtureId[] >([]) + const [labwareConfirmed, setLabwareConfirmed] = React.useState(false) + const [liquidsConfirmed, setLiquidsConfirmed] = React.useState(false) + const [offsetsConfirmed, setOffsetsConfirmed] = React.useState(false) + const missingSteps = [ + !offsetsConfirmed ? t('applied_labware_offsets') : null, + !labwareConfirmed ? t('labware_placement') : null, + !liquidsConfirmed ? t('liquids') : null, + ].filter(s => s != null) + const { + confirm: confirmMissingSteps, + showConfirmation: showMissingStepsConfirmation, + cancel: cancelExitMissingStepsConfirmation, + } = useConditionalConfirm( + handleProceedToRunClick, + !(labwareConfirmed && liquidsConfirmed && offsetsConfirmed) + ) // orchestrate setup subpages/components const [setupScreen, setSetupScreen] = React.useState( @@ -883,9 +968,13 @@ export function ProtocolSetup(): JSX.Element { runId={runId} setSetupScreen={setSetupScreen} confirmAttachment={confirmAttachment} + confirmStepsComplete={confirmMissingSteps} play={play} - robotName={localRobot?.name != null ? localRobot.name : 'no name'} + robotName={robotName} runRecord={runRecord ?? null} + labwareConfirmed={labwareConfirmed} + liquidsConfirmed={liquidsConfirmed} + offsetsConfirmed={offsetsConfirmed} /> ), instruments: ( @@ -899,11 +988,32 @@ export function ProtocolSetup(): JSX.Element { setProvidedFixtureOptions={setProvidedFixtureOptions} /> ), + offsets: ( + + ), labware: ( - + ), liquids: ( - + ), 'deck configuration': ( error.detail)} /> ) : null} - {showConfirmationModal ? ( + {showMissingStepsConfirmation ? ( + + ) : null} + {showHSConfirmationModal ? ( diff --git a/app/src/pages/RunningProtocol/__tests__/RunningProtocol.test.tsx b/app/src/pages/RunningProtocol/__tests__/RunningProtocol.test.tsx index 1114f4964eb..bddb00263d4 100644 --- a/app/src/pages/RunningProtocol/__tests__/RunningProtocol.test.tsx +++ b/app/src/pages/RunningProtocol/__tests__/RunningProtocol.test.tsx @@ -44,6 +44,10 @@ import { useErrorRecoveryFlows, } from '../../../organisms/ErrorRecoveryFlows' import { useLastRunCommand } from '../../../organisms/Devices/hooks/useLastRunCommand' +import { + useInterventionModal, + InterventionModal, +} from '../../../organisms/InterventionModal' import type { UseQueryResult } from 'react-query' import type { ProtocolAnalyses, RunCommandSummary } from '@opentrons/api-client' @@ -64,6 +68,7 @@ vi.mock('../../../resources/runs') vi.mock('../../../redux/config') vi.mock('../../../organisms/ErrorRecoveryFlows') vi.mock('../../../organisms/Devices/hooks/useLastRunCommand') +vi.mock('../../../organisms/InterventionModal') const RUN_ID = 'run_id' const ROBOT_NAME = 'otie' @@ -159,6 +164,13 @@ describe('RunningProtocol', () => { isERActive: false, failedCommand: {} as any, }) + vi.mocked(useInterventionModal).mockReturnValue({ + showModal: false, + modalProps: {} as any, + }) + vi.mocked(InterventionModal).mockReturnValue( +
MOCK_INTERVENTION_MODAL
+ ) }) afterEach(() => { @@ -219,6 +231,16 @@ describe('RunningProtocol', () => { screen.getByText('MOCK ERROR RECOVERY') }) + it('should render an InterventionModal appropriately', () => { + vi.mocked(useInterventionModal).mockReturnValue({ + showModal: true, + modalProps: {} as any, + }) + render(`/runs/${RUN_ID}/run`) + + screen.getByText('MOCK_INTERVENTION_MODAL') + }) + // ToDo (kj:04/04/2023) need to figure out the way to simulate swipe it.todo('should render RunningProtocolCommandList when swiping left') // const [{ getByText }] = render(`/runs/${RUN_ID}/run`) diff --git a/app/src/pages/RunningProtocol/index.tsx b/app/src/pages/RunningProtocol/index.tsx index df35c6fa846..f1ca179167d 100644 --- a/app/src/pages/RunningProtocol/index.tsx +++ b/app/src/pages/RunningProtocol/index.tsx @@ -24,14 +24,15 @@ import { import { RUN_STATUS_STOP_REQUESTED, RUN_STATUS_BLOCKED_BY_OPEN_DOOR, - RUN_STATUS_FINISHING, } from '@opentrons/api-client' import { StepMeter } from '../../atoms/StepMeter' import { useMostRecentCompletedAnalysis } from '../../organisms/LabwarePositionCheck/useMostRecentCompletedAnalysis' import { useNotifyRunQuery } from '../../resources/runs' -import { InterventionModal } from '../../organisms/InterventionModal' -import { isInterventionCommand } from '../../organisms/InterventionModal/utils' +import { + InterventionModal, + useInterventionModal, +} from '../../organisms/InterventionModal' import { useRunStatus, useRunTimestamps, @@ -89,10 +90,6 @@ export function RunningProtocol(): JSX.Element { showConfirmCancelRunModal, setShowConfirmCancelRunModal, ] = React.useState(false) - const [ - interventionModalCommandKey, - setInterventionModalCommandKey, - ] = React.useState(null) const lastAnimatedCommand = React.useRef(null) const { ref, style, swipeType, setSwipeType } = useSwipe() const robotSideAnalysis = useMostRecentCompletedAnalysis(runId) @@ -124,6 +121,16 @@ export function RunningProtocol(): JSX.Element { const robotAnalyticsData = useRobotAnalyticsData(robotName) const robotType = useRobotType(robotName) const { isERActive, failedCommand } = useErrorRecoveryFlows(runId, runStatus) + const { + showModal: showIntervention, + modalProps: interventionProps, + } = useInterventionModal({ + runStatus, + lastRunCommand, + runData: runRecord?.data ?? null, + robotName, + analysis: robotSideAnalysis, + }) React.useEffect(() => { if ( @@ -143,23 +150,6 @@ export function RunningProtocol(): JSX.Element { } }, [currentOption, swipeType, setSwipeType]) - React.useEffect(() => { - if ( - lastRunCommand != null && - interventionModalCommandKey != null && - lastRunCommand.key !== interventionModalCommandKey - ) { - // set intervention modal command key to null if different from current command key - setInterventionModalCommandKey(null) - } else if ( - lastRunCommand?.key != null && - isInterventionCommand(lastRunCommand) && - interventionModalCommandKey === null - ) { - setInterventionModalCommandKey(lastRunCommand.key) - } - }, [lastRunCommand, interventionModalCommandKey]) - return ( <> {isERActive ? ( @@ -202,18 +192,8 @@ export function RunningProtocol(): JSX.Element { isActiveRun={true} /> ) : null} - {interventionModalCommandKey != null && - runRecord?.data != null && - lastRunCommand != null && - isInterventionCommand(lastRunCommand) && - runStatus !== RUN_STATUS_FINISHING ? ( - + {showIntervention ? ( + ) : null} { : `translate(${cornerOffsetFromSlot.x}, ${cornerOffsetFromSlot.y})` } ref={gRef} + onClick={props.onLabwareClick} > { + const { xDimension, yDimension } = definition.dimensions + return Math.round( + xDimension * Math.sin(SKEW_ANGLE_RADIANS) + + (yDimension / Math.cos(SKEW_ANGLE_RADIANS)) * + Math.cos(SKEW_ANGLE_RADIANS + ROTATE_ANGLE_RADIANS) + ) +} + +const getLabwareHeightIso = (definition: LabwareDefinition2): number => { + const { zDimension } = definition.dimensions + return Math.round(getLabwareFaceHeightIso(definition) + zDimension) +} + +const getXMinForViewbox = (definition: LabwareDefinition2): number => { + const { yDimension } = definition.dimensions + return Math.round( + (yDimension / Math.cos(SKEW_ANGLE_RADIANS)) * + Math.cos(Math.PI / 2 - (SKEW_ANGLE_RADIANS + ROTATE_ANGLE_RADIANS)) + ) +} + +const getLabwareWidthIso = (definition: LabwareDefinition2): number => { + const { xDimension } = definition.dimensions + return ( + getXMinForViewbox(definition) + xDimension * Math.cos(ROTATE_ANGLE_RADIANS) + ) +} + export const LabwareStackRender = ( props: LabwareStackRenderProps ): JSX.Element => { @@ -48,38 +82,58 @@ export const LabwareStackRender = ( const fillColorBottom = highlightBottom ? HIGHLIGHT_COLOR : COLORS.white // only one labware (top) - if (definitionBottom == null) { + if ( + definitionBottom == null || + definitionBottom.parameters.loadName === 'opentrons_flex_96_tiprack_adapter' + ) { const { xDimension, yDimension } = definitionTop.dimensions const isTopAdapter = definitionTop.metadata.displayCategory === 'adapter' return isTopAdapter ? ( // adapter render - - - - - + + ) : ( // isometric view of labware - + - - {wellLabelOption != null ? ( + + {wellLabelOption != null && + definitionTop.metadata.displayCategory !== 'adapter' ? ( ) : null} @@ -87,7 +141,7 @@ export const LabwareStackRender = ( - + ) } + const xMinForViewbox = Math.min( + ...[definitionTop, definitionBottom].map(def => getXMinForViewbox(def)) + ) + + const totalAssemblyHeight = + getLabwareHeightIso(definitionTop) + + STACK_SEPARATION_MM + + definitionBottom.dimensions.zDimension + return ( - + {/* bottom labware/adapter */} - - - {wellLabelOption != null && - definitionTop.metadata.displayCategory !== 'adapter' ? ( - + + + {wellLabelOption != null && + definitionBottom.metadata.displayCategory !== 'adapter' ? ( + + ) : null} + + - ) : null} - - - + + + )} {/* top labware/adapter */} - + {wellLabelOption != null && definitionTop.metadata.displayCategory !== 'adapter' ? ( - + ) } diff --git a/components/src/hardware-sim/Labware/labwareInternals/LabwareOutline.tsx b/components/src/hardware-sim/Labware/labwareInternals/LabwareOutline.tsx index 9eda65289ec..7478c671114 100644 --- a/components/src/hardware-sim/Labware/labwareInternals/LabwareOutline.tsx +++ b/components/src/hardware-sim/Labware/labwareInternals/LabwareOutline.tsx @@ -19,6 +19,7 @@ export interface LabwareOutlineProps { /** [legacy] override the border color */ stroke?: CSSProperties['stroke'] fill?: CSSProperties['fill'] + showRadius?: boolean } const OUTLINE_THICKNESS_MM = 1 @@ -32,6 +33,7 @@ export function LabwareOutline(props: LabwareOutlineProps): JSX.Element { highlight = false, stroke, fill, + showRadius = true, } = props const { parameters = { isTiprack }, @@ -62,6 +64,7 @@ export function LabwareOutline(props: LabwareOutlineProps): JSX.Element { stroke="#74B0FF" rx="8" ry="8" + showRadius={showRadius} /> ) : ( @@ -80,6 +84,7 @@ export function LabwareOutline(props: LabwareOutlineProps): JSX.Element { yDimension={dimensions.yDimension} stroke={stroke ?? (parameters.isTiprack ? '#979797' : COLORS.black90)} fill={backgroundFill} + showRadius={showRadius} /> )} @@ -90,9 +95,16 @@ interface LabwareBorderProps extends React.SVGProps { borderThickness: number xDimension: number yDimension: number + showRadius?: boolean } function LabwareBorder(props: LabwareBorderProps): JSX.Element { - const { borderThickness, xDimension, yDimension, ...svgProps } = props + const { + borderThickness, + xDimension, + yDimension, + showRadius = true, + ...svgProps + } = props return ( ) diff --git a/components/src/hardware-sim/Labware/labwareInternals/StaticLabware.tsx b/components/src/hardware-sim/Labware/labwareInternals/StaticLabware.tsx index c8341c94a07..4094ea1e038 100644 --- a/components/src/hardware-sim/Labware/labwareInternals/StaticLabware.tsx +++ b/components/src/hardware-sim/Labware/labwareInternals/StaticLabware.tsx @@ -24,6 +24,7 @@ export interface StaticLabwareProps { /** Optional callback to be executed when mouse leaves a well element */ onMouseLeaveWell?: (e: WellMouseEvent) => unknown fill?: CSSProperties['fill'] + showRadius?: boolean } const TipDecoration = React.memo(function TipDecoration(props: { @@ -58,6 +59,7 @@ export function StaticLabwareComponent(props: StaticLabwareProps): JSX.Element { onMouseEnterWell, onMouseLeaveWell, fill, + showRadius = true, } = props const { isTiprack } = definition.parameters @@ -68,6 +70,7 @@ export function StaticLabwareComponent(props: StaticLabwareProps): JSX.Element { definition={definition} highlight={highlight} fill={fill} + showRadius={showRadius} /> diff --git a/hardware-testing/hardware_testing/gravimetric/helpers.py b/hardware-testing/hardware_testing/gravimetric/helpers.py index 5c89712ac81..eaadca2c6a9 100644 --- a/hardware-testing/hardware_testing/gravimetric/helpers.py +++ b/hardware-testing/hardware_testing/gravimetric/helpers.py @@ -109,6 +109,11 @@ async def _thread_manager_build_hw_api( extra_labware=extra_labware, hardware_simulator=ThreadManager(_thread_manager_build_hw_api), robot_type="Flex", + # use_virtual_hardware=False makes this simulation work unlike + # opentrons_simulate, app-side analysis, and server-side analysis. + # We need to do this because some of our hardware testing scripts still + # interact directly with the OT3API and there is no way to tell Protocol + # Engine's hardware virtualization about those updates. use_virtual_hardware=False, ) else: diff --git a/robot-server/robot_server/data_files/data_files_store.py b/robot-server/robot_server/data_files/data_files_store.py index 785046d80a1..a209dfc8e3a 100644 --- a/robot-server/robot_server/data_files/data_files_store.py +++ b/robot-server/robot_server/data_files/data_files_store.py @@ -145,26 +145,17 @@ def remove(self, file_id: str) -> None: transaction.execute(select_ids_used_in_runs).scalars().all() ) if len(files_used_in_analyses) + len(files_used_in_runs) > 0: - analysis_usage_text = ( - f" analyses: {files_used_in_analyses}" - if len(files_used_in_analyses) > 0 - else None - ) - runs_usage_text = ( - f" runs: {files_used_in_runs}" - if len(files_used_in_runs) > 0 - else None - ) - conjunction = " and " if analysis_usage_text and runs_usage_text else "" + raise FileInUseError( data_file_id=file_id, - message=f"Cannot remove file {file_id} as it is being used in" - f" existing{analysis_usage_text or ''}{conjunction}{runs_usage_text or ''}.", + ids_used_in_runs=files_used_in_runs, + ids_used_in_analyses=files_used_in_analyses, ) - transaction.execute(delete_statement) - + result = transaction.execute(delete_statement) + if result.rowcount < 1: + raise FileIdNotFoundError(file_id) file_dir = self._data_files_directory.joinpath(file_id) - if file_dir: + if file_dir.exists(): for file in file_dir.glob("*"): file.unlink() file_dir.rmdir() diff --git a/robot-server/robot_server/data_files/models.py b/robot-server/robot_server/data_files/models.py index 7701d130924..f2da43bb0f6 100644 --- a/robot-server/robot_server/data_files/models.py +++ b/robot-server/robot_server/data_files/models.py @@ -1,6 +1,6 @@ """Data files models.""" from datetime import datetime -from typing import Literal +from typing import Literal, Set from pydantic import Field @@ -32,7 +32,25 @@ def __init__(self, data_file_id: str) -> None: class FileInUseError(GeneralError): """Error raised when a file being removed is in use.""" - def __init__(self, data_file_id: str, message: str) -> None: + def __init__( + self, + data_file_id: str, + ids_used_in_runs: Set[str], + ids_used_in_analyses: Set[str], + ) -> None: + analysis_usage_text = ( + f" analyses: {ids_used_in_analyses}" + if len(ids_used_in_analyses) > 0 + else None + ) + runs_usage_text = ( + f" runs: {ids_used_in_runs}" if len(ids_used_in_runs) > 0 else None + ) + conjunction = " and " if analysis_usage_text and runs_usage_text else "" + message = ( + f"Cannot remove file {data_file_id} as it is being used in" + f" existing{analysis_usage_text or ''}{conjunction}{runs_usage_text or ''}." + ) super().__init__( message=message, detail={"dataFileId": data_file_id}, diff --git a/robot-server/robot_server/protocols/completed_analysis_store.py b/robot-server/robot_server/protocols/completed_analysis_store.py index bf8cca74871..eb473c7692d 100644 --- a/robot-server/robot_server/protocols/completed_analysis_store.py +++ b/robot-server/robot_server/protocols/completed_analysis_store.py @@ -280,10 +280,13 @@ def get_primitive_rtps_by_analysis_id( with self._sql_engine.begin() as transaction: results = transaction.execute(statement).all() - rtps: Dict[str, PrimitiveAllowedTypes] = {} - for row in results: - param = PrimitiveParameterResource.from_sql_row(row) - rtps.update({param.parameter_variable_name: param.parameter_value}) + param_resources = [ + PrimitiveParameterResource.from_sql_row(row) for row in results + ] + rtps = { + param.parameter_variable_name: param.parameter_value + for param in param_resources + } return rtps def get_csv_rtps_by_analysis_id( diff --git a/robot-server/robot_server/protocols/protocol_store.py b/robot-server/robot_server/protocols/protocol_store.py index 13676a798eb..a3a4a954961 100644 --- a/robot-server/robot_server/protocols/protocol_store.py +++ b/robot-server/robot_server/protocols/protocol_store.py @@ -308,9 +308,11 @@ def get_usage_info(self) -> List[ProtocolUsageInfo]: return usage_info - # TODO (spp, 2024-07-22): get files referenced in runs as well async def get_referenced_data_files(self, protocol_id: str) -> List[DataFile]: - """Get a list of data files referenced in specified protocol's analyses and runs.""" + """Return a list of data files referenced in specified protocol's analyses and runs. + + List returned is in the order in which the data files were uploaded to the server. + """ # Get analyses and runs of protocol_id select_referencing_analysis_ids = sqlalchemy.select(analysis_table.c.id).where( analysis_table.c.protocol_id == protocol_id @@ -318,39 +320,34 @@ async def get_referenced_data_files(self, protocol_id: str) -> List[DataFile]: select_referencing_run_ids = sqlalchemy.select(run_table.c.id).where( run_table.c.protocol_id == protocol_id ) - # Get all entries in csv table that match the analyses - analysis_csv_file_ids = sqlalchemy.select( + # Get all entries in analysis_csv_table that match the analysis IDs above + select_analysis_csv_file_ids = sqlalchemy.select( analysis_csv_rtp_table.c.file_id ).where( analysis_csv_rtp_table.c.analysis_id.in_(select_referencing_analysis_ids) ) - run_csv_file_ids = sqlalchemy.select(run_csv_rtp_table.c.file_id).where( + # Get all entries in run_csv_table that match the run IDs above + select_run_csv_file_ids = sqlalchemy.select(run_csv_rtp_table.c.file_id).where( run_csv_rtp_table.c.run_id.in_(select_referencing_run_ids) ) - # Get list of data file IDs from the entries - select_analysis_data_file_rows_statement = data_files_table.select().where( - data_files_table.c.id.in_(analysis_csv_file_ids) - ) - select_run_data_file_rows_statement = data_files_table.select().where( - data_files_table.c.id.in_(run_csv_file_ids) - ) + with self._sql_engine.begin() as transaction: - analysis_data_files_rows = transaction.execute( - select_analysis_data_file_rows_statement - ).all() - run_data_files_rows = transaction.execute( - select_run_data_file_rows_statement + data_files_rows = transaction.execute( + data_files_table.select() + .where( + data_files_table.c.id.in_(select_analysis_csv_file_ids) + | data_files_table.c.id.in_(select_run_csv_file_ids) + ) + .order_by(sqlite_rowid) ).all() - combine_data_file_rows = set(analysis_data_files_rows + run_data_files_rows) - return [ DataFile( id=sql_row.id, name=sql_row.name, createdAt=sql_row.created_at, ) - for sql_row in combine_data_file_rows + for sql_row in data_files_rows ] def get_referencing_run_ids(self, protocol_id: str) -> List[str]: diff --git a/robot-server/robot_server/runs/run_data_manager.py b/robot-server/robot_server/runs/run_data_manager.py index 62b491e6617..7996d5c5237 100644 --- a/robot-server/robot_server/runs/run_data_manager.py +++ b/robot-server/robot_server/runs/run_data_manager.py @@ -207,6 +207,10 @@ async def create( created_at=created_at, protocol_id=protocol.protocol_id if protocol is not None else None, ) + run_time_parameters = self._run_orchestrator_store.get_run_time_parameters() + self._run_store.insert_csv_rtp( + run_id=run_id, run_time_parameters=run_time_parameters + ) await self._runs_publisher.start_publishing_for_run( get_current_command=self.get_current_command, get_recovery_target_command=self.get_recovery_target_command, @@ -218,7 +222,7 @@ async def create( run_resource=run_resource, state_summary=state_summary, current=True, - run_time_parameters=self._run_orchestrator_store.get_run_time_parameters(), + run_time_parameters=run_time_parameters, ) def get(self, run_id: str) -> Union[Run, BadRun]: diff --git a/robot-server/robot_server/runs/run_store.py b/robot-server/robot_server/runs/run_store.py index bbd50b1f713..0de7d08bac6 100644 --- a/robot-server/robot_server/runs/run_store.py +++ b/robot-server/robot_server/runs/run_store.py @@ -217,7 +217,7 @@ def get_all_csv_rtp(self) -> List[CSVParameterRunResource]: with self._sql_engine.begin() as transaction: csv_rtps = transaction.execute(select_all_csv_rtp).all() - return [_covert_row_to_csv_rtp(row) for row in csv_rtps] + return [_convert_row_to_csv_rtp(row) for row in csv_rtps] def insert_csv_rtp( self, run_id: str, run_time_parameters: List[RunTimeParameter] @@ -543,9 +543,13 @@ def remove(self, run_id: str) -> None: delete_commands = sqlalchemy.delete(run_command_table).where( run_command_table.c.run_id == run_id ) + delete_csv_rtps = sqlalchemy.delete(run_csv_rtp_table).where( + run_csv_rtp_table.c.run_id == run_id + ) with self._sql_engine.begin() as transaction: transaction.execute(delete_actions) transaction.execute(delete_commands) + transaction.execute(delete_csv_rtps) result = transaction.execute(delete_run) if result.rowcount < 1: @@ -574,7 +578,7 @@ def _clear_caches(self) -> None: _run_columns = [run_table.c.id, run_table.c.protocol_id, run_table.c.created_at] -def _covert_row_to_csv_rtp( +def _convert_row_to_csv_rtp( row: sqlalchemy.engine.Row, ) -> CSVParameterRunResource: run_id = row.run_id diff --git a/robot-server/tests/data_files/test_data_files_store.py b/robot-server/tests/data_files/test_data_files_store.py index 33cb31e2621..caef1599961 100644 --- a/robot-server/tests/data_files/test_data_files_store.py +++ b/robot-server/tests/data_files/test_data_files_store.py @@ -265,3 +265,9 @@ async def test_remove_raises_in_file_in_use( expected_error_message = "Cannot remove file file-id as it is being used in existing analyses: {'analysis-id'}." with pytest.raises(FileInUseError, match=expected_error_message): subject.remove(file_id="file-id") + + +def test_remove_raise_for_nonexistent_id(subject: DataFilesStore) -> None: + """It should raise FileIdNotFound error.""" + with pytest.raises(FileIdNotFoundError, match="Data file file-id was not found."): + subject.remove(file_id="file-id") diff --git a/robot-server/tests/integration/http_api/protocols/test_analyses_with_csv_file_parameters.tavern.yaml b/robot-server/tests/integration/http_api/protocols/test_analyses_with_csv_file_parameters.tavern.yaml index 399fc6e445c..9c9980b9608 100644 --- a/robot-server/tests/integration/http_api/protocols/test_analyses_with_csv_file_parameters.tavern.yaml +++ b/robot-server/tests/integration/http_api/protocols/test_analyses_with_csv_file_parameters.tavern.yaml @@ -96,6 +96,7 @@ stages: - displayName: Liquid handling CSV file variableName: liq_handling_csv_file description: A CSV file that contains wells to use for pipetting + type: csv_file file: id: '{csv_file_id}' name: 'sample_record.csv' diff --git a/robot-server/tests/integration/http_api/protocols/test_get_csv_files_used_with_protocol.tavern.yaml b/robot-server/tests/integration/http_api/protocols/test_get_csv_files_used_with_protocol.tavern.yaml new file mode 100644 index 00000000000..7868ece724f --- /dev/null +++ b/robot-server/tests/integration/http_api/protocols/test_get_csv_files_used_with_protocol.tavern.yaml @@ -0,0 +1,250 @@ +test_name: Test the /protocols/{protocolID}/dataFiles endpoint + +marks: + - usefixtures: + - ot2_server_base_url + +stages: + # The order of these data file uploads is important for this test, + # since the list of data files returned for the specified protocol is in upload order. + # The order in which the files are uploaded in this test is the same as the order in which + # these files are uploaded in the overall integration tests suite. + # Until we add data file cleanup after each test, maintaining this order within the suite + # will be important. + + # sample_record -> test + # sample_plates -> sample_record + # test -> sample_plates + - name: Upload data file 1 + request: + url: '{ot2_server_base_url}/dataFiles' + method: POST + files: + file: 'tests/integration/data_files/test.csv' + response: + save: + json: + data_file_1_id: data.id + data_file_1_name: data.name + status_code: + - 201 + - 200 + + - name: Upload data file 2 + request: + url: '{ot2_server_base_url}/dataFiles' + method: POST + files: + file: 'tests/integration/data_files/sample_record.csv' + response: + save: + json: + data_file_2_id: data.id + data_file_2_name: data.name + status_code: + - 201 + - 200 + + - name: Upload data file 3 + request: + url: '{ot2_server_base_url}/dataFiles' + method: POST + files: + file: 'tests/integration/data_files/sample_plates.csv' + response: + save: + json: + data_file_3_id: data.id + data_file_3_name: data.name + status_code: + - 201 + - 200 + + - name: Upload protocol with CSV file ID + request: + url: '{ot2_server_base_url}/protocols' + method: POST + data: + runTimeParameterFiles: '{{"liq_handling_csv_file": "{data_file_1_id}"}}' + files: + files: 'tests/integration/protocols/basic_transfer_with_run_time_parameters.py' + response: + save: + json: + protocol_id: data.id + analysis_id: data.analysisSummaries[0].id + run_time_parameters_data1: data.analysisSummaries[0].runTimeParameters + strict: + json:off + status_code: 201 + json: + data: + analysisSummaries: + - id: !anystr + status: pending + runTimeParameters: + - displayName: Liquid handling CSV file + variableName: liq_handling_csv_file + description: A CSV file that contains wells to use for pipetting + type: csv_file + file: + id: '{data_file_1_id}' + name: 'test.csv' + + - name: Wait until analysis is completed + max_retries: 5 + delay_after: 1 + request: + url: '{ot2_server_base_url}/protocols/{protocol_id}' + response: + status_code: 200 + json: + data: + analyses: [] + analysisSummaries: + - id: '{analysis_id}' + status: completed + id: !anything + protocolType: !anything + files: !anything + createdAt: !anything + robotType: !anything + protocolKind: !anything + metadata: !anything + links: !anything + + - name: Start a new analysis with a different CSV file + request: + url: '{ot2_server_base_url}/protocols/{protocol_id}/analyses' + method: POST + json: + data: + forceReAnalyze: true + runTimeParameterFiles: + liq_handling_csv_file: '{data_file_3_id}' + response: + strict: + - json:off + status_code: 201 + json: + data: + - id: '{analysis_id}' + status: completed + - id: !anystr + status: pending + runTimeParameters: + - displayName: Liquid handling CSV file + variableName: liq_handling_csv_file + description: A CSV file that contains wells to use for pipetting + type: csv_file + file: + id: '{data_file_3_id}' + name: 'sample_plates.csv' + + - name: Wait until analysis is completed + max_retries: 5 + delay_after: 1 + request: + url: '{ot2_server_base_url}/protocols/{protocol_id}' + response: + status_code: 200 + json: + data: + analyses: [] + analysisSummaries: + - id: '{analysis_id}' + status: completed + - id: !anystr + status: completed + id: !anything + protocolType: !anything + files: !anything + createdAt: !anything + robotType: !anything + protocolKind: !anything + metadata: !anything + links: !anything + + - name: Create a run from the protocol and a CSV file + request: + url: '{ot2_server_base_url}/runs' + method: POST + json: + data: + protocolId: '{protocol_id}' + runTimeParameterFiles: + liq_handling_csv_file: '{data_file_1_id}' + response: + status_code: 201 + save: + json: + run_id1: data.id + run_time_parameters_data2: data.runTimeParameters + strict: + json:off + json: + data: + id: !anystr + ok: True + createdAt: !re_fullmatch "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+(Z|([+-]\\d{2}:\\d{2}))" + status: idle + runTimeParameters: + - displayName: Liquid handling CSV file + variableName: liq_handling_csv_file + description: A CSV file that contains wells to use for pipetting + type: csv_file + file: + id: '{data_file_1_id}' + name: 'test.csv' + + - name: Create another run from the protocol and a different CSV file + request: + url: '{ot2_server_base_url}/runs' + method: POST + json: + data: + protocolId: '{protocol_id}' + runTimeParameterFiles: + liq_handling_csv_file: '{data_file_2_id}' + response: + status_code: 201 + save: + json: + run_id2: data.id + run_time_parameters_data3: data.runTimeParameters + strict: + json:off + json: + data: + id: !anystr + ok: True + createdAt: !re_fullmatch "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+(Z|([+-]\\d{2}:\\d{2}))" + status: idle + runTimeParameters: + - displayName: Liquid handling CSV file + variableName: liq_handling_csv_file + description: A CSV file that contains wells to use for pipetting + type: csv_file + file: + id: '{data_file_2_id}' + name: 'sample_record.csv' + + - name: Fetch data files used with the protocol so far + request: + url: '{ot2_server_base_url}/protocols/{protocol_id}/dataFiles' + response: + status_code: 200 + json: + meta: + cursor: 0 + totalLength: 3 + data: + - id: '{data_file_1_id}' + name: "test.csv" + createdAt: !re_fullmatch "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+(Z|([+-]\\d{2}:\\d{2}))" + - id: '{data_file_2_id}' + name: "sample_record.csv" + createdAt: !re_fullmatch "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+(Z|([+-]\\d{2}:\\d{2}))" + - id: '{data_file_3_id}' + name: "sample_plates.csv" + createdAt: !re_fullmatch "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+(Z|([+-]\\d{2}:\\d{2}))" diff --git a/robot-server/tests/integration/http_api/runs/test_run_with_run_time_parameters.tavern.yaml b/robot-server/tests/integration/http_api/runs/test_run_with_run_time_parameters.tavern.yaml index f029e945e20..0f729e62d8d 100644 --- a/robot-server/tests/integration/http_api/runs/test_run_with_run_time_parameters.tavern.yaml +++ b/robot-server/tests/integration/http_api/runs/test_run_with_run_time_parameters.tavern.yaml @@ -27,7 +27,9 @@ stages: save: json: data_file_id: data.id - status_code: 201 + status_code: + - 201 + - 200 json: data: id: !anystr diff --git a/robot-server/tests/protocols/test_protocol_store.py b/robot-server/tests/protocols/test_protocol_store.py index ff6d4ce7b49..5d413ad7fa3 100644 --- a/robot-server/tests/protocols/test_protocol_store.py +++ b/robot-server/tests/protocols/test_protocol_store.py @@ -585,7 +585,7 @@ async def test_get_referenced_data_files( subject.insert(protocol_resource_1) await data_files_store.insert( DataFileInfo( - id="data-file-id", + id="data-file-id-1", name="file-name", file_hash="abc123", created_at=datetime(year=2021, month=1, day=1, tzinfo=timezone.utc), @@ -632,7 +632,7 @@ async def test_get_referenced_data_files( CSVParameterResource( analysis_id="analysis-id-1", parameter_variable_name="csv-var", - file_id="data-file-id", + file_id="data-file-id-1", ), CSVParameterResource( analysis_id="analysis-id-1", @@ -648,23 +648,20 @@ async def test_get_referenced_data_files( ) result = await subject.get_referenced_data_files("protocol-id") - for data_file in result: - assert data_file in [ - DataFile( - id="data-file-id", - name="file-name", - createdAt=datetime(year=2021, month=1, day=1, tzinfo=timezone.utc), - ), - DataFile( - id="data-file-id-2", - name="file-name", - createdAt=datetime(year=2021, month=1, day=1, tzinfo=timezone.utc), - ), - DataFile( - id="data-file-id-3", - name="file-name", - createdAt=datetime(year=2021, month=1, day=1, tzinfo=timezone.utc), - ), - ] - - assert len(result) == 3 + assert result == [ + DataFile( + id="data-file-id-1", + name="file-name", + createdAt=datetime(year=2021, month=1, day=1, tzinfo=timezone.utc), + ), + DataFile( + id="data-file-id-2", + name="file-name", + createdAt=datetime(year=2021, month=1, day=1, tzinfo=timezone.utc), + ), + DataFile( + id="data-file-id-3", + name="file-name", + createdAt=datetime(year=2021, month=1, day=1, tzinfo=timezone.utc), + ), + ] diff --git a/robot-server/tests/runs/test_run_data_manager.py b/robot-server/tests/runs/test_run_data_manager.py index dc19c8b4abc..ff1f70da399 100644 --- a/robot-server/tests/runs/test_run_data_manager.py +++ b/robot-server/tests/runs/test_run_data_manager.py @@ -214,6 +214,7 @@ async def test_create( modules=engine_state_summary.modules, liquids=engine_state_summary.liquids, ) + decoy.verify(mock_run_store.insert_csv_rtp(run_id=run_id, run_time_parameters=[])) async def test_create_with_options( @@ -299,6 +300,11 @@ async def test_create_with_options( liquids=engine_state_summary.liquids, runTimeParameters=[bool_parameter, file_parameter], ) + decoy.verify( + mock_run_store.insert_csv_rtp( + run_id=run_id, run_time_parameters=[bool_parameter, file_parameter] + ) + ) async def test_create_engine_error( diff --git a/robot-server/tests/runs/test_run_store.py b/robot-server/tests/runs/test_run_store.py index f4b2b8e154f..74dcffac14f 100644 --- a/robot-server/tests/runs/test_run_store.py +++ b/robot-server/tests/runs/test_run_store.py @@ -484,7 +484,12 @@ def test_get_all_runs( assert result == expected_result -def test_remove_run(subject: RunStore, mock_runs_publisher: mock.Mock) -> None: +async def test_remove_run( + subject: RunStore, + mock_runs_publisher: mock.Mock, + data_files_store: DataFilesStore, + run_time_parameters: List[pe_types.RunTimeParameter], +) -> None: """It can remove a previously stored run entry.""" action = RunAction( actionType=RunActionType.PLAY, @@ -498,6 +503,15 @@ def test_remove_run(subject: RunStore, mock_runs_publisher: mock.Mock) -> None: created_at=datetime(year=2021, month=1, day=1, tzinfo=timezone.utc), ) subject.insert_action(run_id="run-id", action=action) + await data_files_store.insert( + DataFileInfo( + id="file-id", + name="my_csv_file.csv", + file_hash="file-hash", + created_at=datetime(year=2024, month=1, day=1, tzinfo=timezone.utc), + ) + ) + subject.insert_csv_rtp(run_id="run-id", run_time_parameters=run_time_parameters) subject.remove(run_id="run-id") assert subject.get_all(length=20) == [] diff --git a/shared-data/python/opentrons_shared_data/errors/exceptions.py b/shared-data/python/opentrons_shared_data/errors/exceptions.py index 888dc7f6763..e033ee144f7 100644 --- a/shared-data/python/opentrons_shared_data/errors/exceptions.py +++ b/shared-data/python/opentrons_shared_data/errors/exceptions.py @@ -964,7 +964,7 @@ def __init__( f"{api_element} is not yet available in the API version in use." ) if message: - checked_message = checked_message + message + checked_message = checked_message + " " + message checked_message = ( checked_message or "This feature is not yet available in the API version in use."