fix(api): Support simulating PAPIv≥2.14 (#13709)

Notable changes:

* Rework `simulate.py` to support PAPIv≥2.14 protocols, via Protocol Engine. This rework mirrors what we did to `execute.py` in #12970.
* Rework the `adapt_protocol_source()` helper to cope with protocols for which we don't know the filename. This is necessary to support `opentrons.simulate.simulate()`, whose `filename` argument is optional.

  Unfortunately, this requires modifications to `opentrons.protocols.parse` to make the filename more pass-through, so `adapt_protocol_source()` can tell when it wasn't provided.

Plus various smaller refactors for type safety, deduplication, and consistency.

api/release-notes.md

@@ -36,7 +36,7 @@ Flex touchscreen
 - Manage instruments: View information about connected pipettes and the gripper. Attach, detach, or recalibrate instruments.
 - Robot settings: Customize the behavior of your Flex, including the LED and touchscreen displays.
 
-Flex features 
+Flex features
 
 - Analyze and run protocols that use the Flex robot, Flex pipettes, and Flex tip racks.
 - Move labware around the deck automatically with the Flex Gripper.
@@ -65,7 +65,6 @@ Python API features
 Some protocols can't be simulated with the `opentrons_simulate` command-line tool:
 
 - JSON protocols created or modified with Protocol Designer v6.0.0 or higher.
-- Python protocols specifying an `apiLevel` of 2.14 or higher.
 
 ---
 

api/src/opentrons/execute.py

@@ -11,15 +11,12 @@
 import contextlib
 import logging
 import os
-from pathlib import Path
 import sys
-import tempfile
 from typing import (
     TYPE_CHECKING,
     BinaryIO,
     Callable,
     Dict,
-    Generator,
     List,
     Optional,
     TextIO,
@@ -35,7 +32,6 @@
 
 from opentrons.hardware_control import (
     API as OT2API,
-    HardwareControlAPI,
     ThreadManagedHardware,
     ThreadManager,
 )
@@ -59,23 +55,16 @@
     Config,
     DeckType,
     EngineStatus,
-    ErrorOccurrence as ProtocolEngineErrorOccurrence,
     create_protocol_engine,
     create_protocol_engine_in_thread,
 )
 from opentrons.protocol_engine.types import PostRunHardwareState
 
-from opentrons.protocol_reader import ProtocolReader, ProtocolSource
+from opentrons.protocol_reader import ProtocolSource
 
 from opentrons.protocol_runner import create_protocol_runner
 
-from .util.entrypoint_util import (
-    FoundLabware,
-    find_jupyter_labware,
-    labware_from_paths,
-    datafiles_from_paths,
-    copy_file_like,
-)
+from .util import entrypoint_util
 
 if TYPE_CHECKING:
     from opentrons_shared_data.labware.dev_types import (
@@ -172,7 +161,7 @@ def get_protocol_api(
     if extra_labware is None:
         extra_labware = {
             uri: details.definition
-            for uri, details in (find_jupyter_labware() or {}).items()
+            for uri, details in (entrypoint_util.find_jupyter_labware() or {}).items()
         }
 
     robot_type = _get_robot_type()
@@ -191,6 +180,8 @@ def get_protocol_api(
         )
     else:
         if bundled_labware is not None:
+            # Protocol Engine has a deep assumption that standard labware definitions are always
+            # implicitly loadable.
             raise NotImplementedError(
                 f"The bundled_labware argument is not currently supported for Python protocols"
                 f" with apiLevel {ENGINE_CORE_API_VERSION} or newer."
@@ -363,21 +354,20 @@ def execute(  # noqa: C901
     stack_logger.propagate = propagate_logs
     stack_logger.setLevel(getattr(logging, log_level.upper(), logging.WARNING))
 
-    contents = protocol_file.read()
-
     # TODO(mm, 2023-10-02): Switch this truthy check to `is not None`
     # to match documented behavior.
     # See notes in https://github.com/Opentrons/opentrons/pull/13107
     if custom_labware_paths:
-        extra_labware = labware_from_paths(custom_labware_paths)
+        extra_labware = entrypoint_util.labware_from_paths(custom_labware_paths)
     else:
-        extra_labware = find_jupyter_labware() or {}
+        extra_labware = entrypoint_util.find_jupyter_labware() or {}
 
     if custom_data_paths:
-        extra_data = datafiles_from_paths(custom_data_paths)
+        extra_data = entrypoint_util.datafiles_from_paths(custom_data_paths)
     else:
         extra_data = {}
 
+    contents = protocol_file.read()
     try:
         protocol = parse.parse(
             contents,
@@ -422,10 +412,8 @@ def execute(  # noqa: C901
             )
         protocol_file.seek(0)
         _run_file_pe(
-            protocol_file=protocol_file,
-            protocol_name=protocol_name,
-            extra_labware=extra_labware,
-            hardware_api=_get_global_hardware_controller(_get_robot_type()).wrapped(),
+            protocol=protocol,
+            hardware_api=_get_global_hardware_controller(_get_robot_type()),
             emit_runlog=emit_runlog,
         )
 
@@ -493,49 +481,15 @@ def main() -> int:
             emit_runlog=printer,
         )
         return 0
-    except _ProtocolEngineExecuteError as error:
-        # _ProtocolEngineExecuteError is a wrapper that's meaningless to the CLI user.
+    except entrypoint_util.ProtocolEngineExecuteError as error:
+        # This exception is a wrapper that's meaningless to the CLI user.
         # Take the actual protocol problem out of it and just print that.
         print(error.to_stderr_string(), file=sys.stderr)
         return 1
     # execute() might raise other exceptions, but we don't have a nice way to print those.
     # Just let Python show a traceback.
 
 
-class _ProtocolEngineExecuteError(Exception):
-    def __init__(self, errors: List[ProtocolEngineErrorOccurrence]) -> None:
-        """Raised when there was any fatal error running a protocol through Protocol Engine.
-
-        Protocol Engine reports errors as data, not as exceptions.
-        But the only way for `execute()` to signal problems to its caller is to raise something.
-        So we need this class to wrap them.
-
-        Params:
-            errors: The errors that Protocol Engine reported.
-        """
-        # Show the full error details if this is part of a traceback. Don't try to summarize.
-        super().__init__(errors)
-        self._error_occurrences = errors
-
-    def to_stderr_string(self) -> str:
-        """Return a string suitable as the stderr output of the `opentrons_execute` CLI.
-
-        This summarizes from the full error details.
-        """
-        # It's unclear what exactly we should extract here.
-        #
-        # First, do we print the first element, or the last, or all of them?
-        #
-        # Second, do we print the .detail? .errorCode? .errorInfo? .wrappedErrors?
-        # By contract, .detail seems like it would be insufficient, but experimentally,
-        # it includes a lot, like:
-        #
-        #     ProtocolEngineError [line 3]: Error 4000 GENERAL_ERROR (ProtocolEngineError):
-        #     UnexpectedProtocolError: Labware "fixture_12_trough" not found with version 1
-        #     in namespace "fixture".
-        return self._error_occurrences[0].detail
-
-
 def _create_live_context_non_pe(
     api_version: APIVersion,
     hardware_api: ThreadManagedHardware,
@@ -640,24 +594,22 @@ def _run_file_non_pe(
 
 
 def _run_file_pe(
-    protocol_file: Union[BinaryIO, TextIO],
-    protocol_name: str,
-    extra_labware: Dict[str, FoundLabware],
-    hardware_api: HardwareControlAPI,
+    protocol: Protocol,
+    hardware_api: ThreadManagedHardware,
     emit_runlog: Optional[_EmitRunlogCallable],
 ) -> None:
     """Run a protocol file with Protocol Engine."""
 
     async def run(protocol_source: ProtocolSource) -> None:
         protocol_engine = await create_protocol_engine(
-            hardware_api=hardware_api,
+            hardware_api=hardware_api.wrapped(),
             config=_get_protocol_engine_config(),
         )
 
         protocol_runner = create_protocol_runner(
             protocol_config=protocol_source.config,
             protocol_engine=protocol_engine,
-            hardware_api=hardware_api,
+            hardware_api=hardware_api.wrapped(),
         )
 
         unsubscribe = protocol_runner.broker.subscribe(
@@ -671,13 +623,11 @@ async def run(protocol_source: ProtocolSource) -> None:
             unsubscribe()
 
         if result.state_summary.status != EngineStatus.SUCCEEDED:
-            raise _ProtocolEngineExecuteError(result.state_summary.errors)
+            raise entrypoint_util.ProtocolEngineExecuteError(
+                result.state_summary.errors
+            )
 
-    with _adapt_protocol_source(
-        protocol_file=protocol_file,
-        protocol_name=protocol_name,
-        extra_labware=extra_labware,
-    ) as protocol_source:
+    with entrypoint_util.adapt_protocol_source(protocol) as protocol_source:
         asyncio.run(run(protocol_source))
 
 
@@ -697,39 +647,6 @@ def _get_protocol_engine_config() -> Config:
     )
 
 
-@contextlib.contextmanager
-def _adapt_protocol_source(
-    protocol_file: Union[BinaryIO, TextIO],
-    protocol_name: str,
-    extra_labware: Dict[str, FoundLabware],
-) -> Generator[ProtocolSource, None, None]:
-    """Create a `ProtocolSource` representing input protocol files."""
-    with tempfile.TemporaryDirectory() as temporary_directory:
-        # It's not well-defined in our customer-facing interfaces whether the supplied protocol_name
-        # should be just the filename part, or a path with separators. In case it contains stuff
-        # like "../", sanitize it to just the filename part so we don't save files somewhere bad.
-        safe_protocol_name = Path(protocol_name).name
-
-        temp_protocol_file = Path(temporary_directory) / safe_protocol_name
-
-        # FIXME(mm, 2023-06-26): Copying this file is pure overhead, and it introduces encoding
-        # hazards. Remove this when we can parse JSONv6+ and PAPIv2.14+ protocols without going
-        # through the filesystem. https://opentrons.atlassian.net/browse/RSS-281
-        copy_file_like(source=protocol_file, destination=temp_protocol_file)
-
-        custom_labware_files = [labware.path for labware in extra_labware.values()]
-
-        protocol_source = asyncio.run(
-            ProtocolReader().read_saved(
-                files=[temp_protocol_file] + custom_labware_files,
-                directory=None,
-                files_are_prevalidated=False,
-            )
-        )
-
-        yield protocol_source
-
-
 def _get_global_hardware_controller(robot_type: RobotType) -> ThreadManagedHardware:
     # Build a hardware controller in a worker thread, which is necessary
     # because ipython runs its notebook in asyncio but the notebook

api/src/opentrons/protocols/execution/execute_python.py

@@ -47,10 +47,19 @@ def run_python(proto: PythonProtocol, context: ProtocolContext):
     # If the protocol is written correctly, it will have defined a function
     # like run(context: ProtocolContext). If so, that function is now in the
     # current scope.
+
+    # TODO(mm, 2023-10-11): This coupling to opentrons.protocols.parse is fragile.
+    # Can we get the correct filename directly from proto.contents?
     if proto.filename and proto.filename.endswith("zip"):
+        # The ".zip" extension needs to match what opentrons.protocols.parse recognizes as a bundle,
+        # and the "protocol.ot2.py" fallback needs to match what opentrons.protocol.py sets as the
+        # AST filename.
         filename = "protocol.ot2.py"
     else:
+        # "<protocol>" needs to match what opentrons.protocols.parse sets as the fallback
+        # AST filename.
         filename = proto.filename or "<protocol>"
+
     try:
         _runfunc_ok(new_globs.get("run"))
     except SyntaxError as se:

api/src/opentrons/protocols/parse.py

@@ -217,11 +217,16 @@ def _parse_python(
     extra_labware: Optional[Dict[str, "LabwareDefinition"]] = None,
 ) -> PythonProtocol:
     """Parse a protocol known or at least suspected to be python"""
-    filename_checked = filename or "<protocol>"
-    if filename_checked.endswith(".zip"):
+    if filename is None:
+        # The fallback "<protocol>" needs to match what opentrons.protocols.execution.execute_python
+        # looks for when it extracts tracebacks.
+        ast_filename = "<protocol>"
+    elif filename.endswith(".zip"):
+        # The extension ".zip" and the fallback "protocol.ot2.py" need to match what
+        # opentrons.protocols.execution.execute_python looks for when it extracts tracebacks.
         ast_filename = "protocol.ot2.py"
     else:
-        ast_filename = filename_checked
+        ast_filename = filename
 
     # todo(mm, 2021-09-13): By default, ast.parse will inherit compiler options
     # and future features from this module. This may not be appropriate.
@@ -244,7 +249,7 @@ def _parse_python(
 
     static_info = _extract_static_python_info(parsed)
     protocol = compile(parsed, filename=ast_filename, mode="exec")
-    version = _get_version(static_info, parsed, filename_checked)
+    version = _get_version(static_info, parsed, ast_filename)
     robot_type = _robot_type_from_static_python_info(static_info)
 
     if version >= APIVersion(2, 0):
@@ -257,7 +262,7 @@ def _parse_python(
 
     result = PythonProtocol(
         text=protocol_contents,
-        filename=getattr(protocol, "co_filename", "<protocol>"),
+        filename=filename,
         contents=protocol,
         metadata=static_info.metadata,
         api_level=version,

api/src/opentrons/protocols/types.py

@@ -32,11 +32,22 @@ class StaticPythonInfo:
 @dataclass(frozen=True)
 class _ProtocolCommon:
     text: str
+
     filename: Optional[str]
+    """The original name of the main protocol file, if it had a name.
+
+    For JSON protocols, this will be the name of the .json file.
+    For Python protocols, this will be the name of the .py file.
+    For bundled protocols, this will be the name of the .zip file.
+
+    This can be `None` if, for example, we've parsed the protocol from an in-memory text stream.
+    """
+
     # TODO(mm, 2023-06-22): Move api_level out of _ProtocolCommon and into PythonProtocol.
     # JSON protocols do not have an intrinsic api_level, especially since JSONv6,
     # where they are no longer executed via the Python Protocol API.
     api_level: "APIVersion"
+
     robot_type: RobotType