Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(api): remove legacy from names of variables and classes that are not actually legacy #15075

Merged
merged 4 commits into from
May 6, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

from opentrons_shared_data.robot.dev_types import RobotType

from .legacy_wrappers import LegacySimulatingContextCreator
from .python_protocol_wrappers import SimulatingContextCreator
from .protocol_runner import create_protocol_runner, AbstractRunner


Expand Down Expand Up @@ -62,7 +62,7 @@ async def create_simulating_runner(
load_fixed_trash=should_load_fixed_trash(protocol_config),
)

simulating_legacy_context_creator = LegacySimulatingContextCreator(
simulating_context_creator = SimulatingContextCreator(
hardware_api=simulating_hardware_api,
protocol_engine=protocol_engine,
)
Expand All @@ -71,7 +71,7 @@ async def create_simulating_runner(
protocol_config=protocol_config,
protocol_engine=protocol_engine,
hardware_api=simulating_hardware_api,
legacy_context_creator=simulating_legacy_context_creator,
protocol_context_creator=simulating_context_creator,
)


Expand Down
72 changes: 37 additions & 35 deletions api/src/opentrons/protocol_runner/legacy_command_mapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,24 @@
from datetime import datetime
from typing import Dict, List, Optional, Tuple, Union

from opentrons.hardware_control.modules.types import (
ModuleModel as HardwareModuleModel,
TemperatureModuleModel,
MagneticModuleModel,
ThermocyclerModuleModel,
HeaterShakerModuleModel,
)
from opentrons_shared_data.pipette.dev_types import PipetteNameType
from opentrons.types import MountType, DeckSlotName, Location
from opentrons.legacy_commands import types as legacy_command_types
from opentrons.protocol_api import InstrumentContext
from opentrons.protocol_api.core.legacy.deck import FIXED_TRASH_ID
from opentrons.protocol_api.core.legacy.load_info import (
LoadInfo,
LabwareLoadInfo,
InstrumentLoadInfo,
ModuleLoadInfo,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do prefer these to be aliased as LegacyLoadInfo, etc because they actually are classes from/for the legacy system.

)
from opentrons.protocol_engine import (
ProtocolEngineError,
actions as pe_actions,
Expand All @@ -19,22 +34,9 @@
ModuleDataProvider,
pipette_data_provider,
)

from opentrons_shared_data.labware.labware_definition import LabwareDefinition
from opentrons_shared_data.errors import ErrorCodes, EnumeratedError, PythonException
from opentrons.protocol_api.core.legacy.deck import FIXED_TRASH_ID

from .legacy_wrappers import (
LegacyLoadInfo,
LegacyInstrumentLoadInfo,
LegacyLabwareLoadInfo,
LegacyModuleLoadInfo,
LegacyPipetteContext,
LegacyModuleModel,
LegacyMagneticModuleModel,
LegacyTemperatureModuleModel,
LegacyThermocyclerModuleModel,
LegacyHeaterShakerModuleModel,
)


class LegacyCommandParams(pe_commands.CustomParams):
Expand Down Expand Up @@ -63,14 +65,14 @@ def __init__(self, wrapping_exc: BaseException) -> None:
)


_LEGACY_TO_PE_MODULE: Dict[LegacyModuleModel, pe_types.ModuleModel] = {
LegacyMagneticModuleModel.MAGNETIC_V1: pe_types.ModuleModel.MAGNETIC_MODULE_V1,
LegacyMagneticModuleModel.MAGNETIC_V2: pe_types.ModuleModel.MAGNETIC_MODULE_V2,
LegacyTemperatureModuleModel.TEMPERATURE_V1: pe_types.ModuleModel.TEMPERATURE_MODULE_V1,
LegacyTemperatureModuleModel.TEMPERATURE_V2: pe_types.ModuleModel.TEMPERATURE_MODULE_V2,
LegacyThermocyclerModuleModel.THERMOCYCLER_V1: pe_types.ModuleModel.THERMOCYCLER_MODULE_V1,
LegacyThermocyclerModuleModel.THERMOCYCLER_V2: pe_types.ModuleModel.THERMOCYCLER_MODULE_V2,
LegacyHeaterShakerModuleModel.HEATER_SHAKER_V1: pe_types.ModuleModel.HEATER_SHAKER_MODULE_V1,
_HARDWARE_TO_PE_MODULE: Dict[HardwareModuleModel, pe_types.ModuleModel] = {
MagneticModuleModel.MAGNETIC_V1: pe_types.ModuleModel.MAGNETIC_MODULE_V1,
MagneticModuleModel.MAGNETIC_V2: pe_types.ModuleModel.MAGNETIC_MODULE_V2,
TemperatureModuleModel.TEMPERATURE_V1: pe_types.ModuleModel.TEMPERATURE_MODULE_V1,
TemperatureModuleModel.TEMPERATURE_V2: pe_types.ModuleModel.TEMPERATURE_MODULE_V2,
ThermocyclerModuleModel.THERMOCYCLER_V1: pe_types.ModuleModel.THERMOCYCLER_MODULE_V1,
ThermocyclerModuleModel.THERMOCYCLER_V2: pe_types.ModuleModel.THERMOCYCLER_MODULE_V2,
HeaterShakerModuleModel.HEATER_SHAKER_V1: pe_types.ModuleModel.HEATER_SHAKER_MODULE_V1,
}

_HIGHER_ORDER_COMMAND_TYPES = {
Expand Down Expand Up @@ -281,13 +283,13 @@ def map_command( # noqa: C901

return results

def map_equipment_load(self, load_info: LegacyLoadInfo) -> List[pe_actions.Action]:
def map_equipment_load(self, load_info: LoadInfo) -> List[pe_actions.Action]:
"""Map a labware, instrument (pipette), or module load to a PE command."""
if isinstance(load_info, LegacyLabwareLoadInfo):
if isinstance(load_info, LabwareLoadInfo):
return self._map_labware_load(load_info)
elif isinstance(load_info, LegacyInstrumentLoadInfo):
elif isinstance(load_info, InstrumentLoadInfo):
return self._map_instrument_load(load_info)
elif isinstance(load_info, LegacyModuleLoadInfo):
elif isinstance(load_info, ModuleLoadInfo):
return self._map_module_load(load_info)

def _build_initial_command(
Expand Down Expand Up @@ -354,7 +356,7 @@ def _build_drop_tip(
command_id: str,
now: datetime,
) -> Tuple[pe_commands.CommandCreate, pe_commands.Command]:
pipette: LegacyPipetteContext = command["payload"]["instrument"]
pipette: InstrumentContext = command["payload"]["instrument"]
well = command["payload"]["location"]
mount = MountType(pipette.mount)
# the following type checking suppression assumes the tiprack is not loaded on top of a module
Expand Down Expand Up @@ -387,7 +389,7 @@ def _build_pick_up_tip(
command_id: str,
now: datetime,
) -> Tuple[pe_commands.CommandCreate, pe_commands.Command]:
pipette: LegacyPipetteContext = command["payload"]["instrument"]
pipette: InstrumentContext = command["payload"]["instrument"]
location = command["payload"]["location"]
well = location
mount = MountType(pipette.mount)
Expand Down Expand Up @@ -422,7 +424,7 @@ def _build_liquid_handling(
command_id: str,
now: datetime,
) -> Tuple[pe_commands.CommandCreate, pe_commands.Command]:
pipette: LegacyPipetteContext = command["payload"]["instrument"]
pipette: InstrumentContext = command["payload"]["instrument"]
location = command["payload"]["location"]
volume = command["payload"]["volume"]
# TODO:(jr, 15.08.2022): aspirate and dispense commands with no specified labware
Expand Down Expand Up @@ -533,7 +535,7 @@ def _build_blow_out(
command_id: str,
now: datetime,
) -> Tuple[pe_commands.CommandCreate, pe_commands.Command]:
pipette: LegacyPipetteContext = command["payload"]["instrument"]
pipette: InstrumentContext = command["payload"]["instrument"]
location = command["payload"]["location"]
flow_rate = pipette.flow_rate.blow_out
# TODO:(jr, 15.08.2022): blow_out commands with no specified labware get filtered
Expand Down Expand Up @@ -590,7 +592,7 @@ def _build_blow_out(
return custom_create, custom_running

def _map_labware_load(
self, labware_load_info: LegacyLabwareLoadInfo
self, labware_load_info: LabwareLoadInfo
) -> List[pe_actions.Action]:
"""Map a legacy labware load to a ProtocolEngine command."""
now = ModelUtils.get_timestamp()
Expand Down Expand Up @@ -658,7 +660,7 @@ def _map_labware_load(

def _map_instrument_load(
self,
instrument_load_info: LegacyInstrumentLoadInfo,
instrument_load_info: InstrumentLoadInfo,
) -> List[pe_actions.Action]:
"""Map a legacy instrument (pipette) load to a ProtocolEngine command.

Expand Down Expand Up @@ -717,16 +719,16 @@ def _map_instrument_load(
return [queue_action, run_action, succeed_action]

def _map_module_load(
self, module_load_info: LegacyModuleLoadInfo
self, module_load_info: ModuleLoadInfo
) -> List[pe_actions.Action]:
"""Map a legacy module load to a Protocol Engine command."""
now = ModelUtils.get_timestamp()

count = self._command_count["LOAD_MODULE"]
command_id = f"commands.LOAD_MODULE-{count}"
module_id = f"module-{count}"
requested_model = _LEGACY_TO_PE_MODULE[module_load_info.requested_model]
loaded_model = _LEGACY_TO_PE_MODULE[module_load_info.loaded_model]
requested_model = _HARDWARE_TO_PE_MODULE[module_load_info.requested_model]
loaded_model = _HARDWARE_TO_PE_MODULE[module_load_info.loaded_model]

# This will fetch a V2 definition only. PAPI < v2.3 use V1 definitions.
# When running a < v2.3 protocol, there will be a mismatch of definitions used
Expand Down
6 changes: 3 additions & 3 deletions api/src/opentrons/protocol_runner/legacy_context_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@

from opentrons.legacy_commands.types import CommandMessage as LegacyCommand
from opentrons.legacy_broker import LegacyBroker
from opentrons.protocol_api.core.legacy.load_info import LoadInfo
from opentrons.protocol_engine import AbstractPlugin, actions as pe_actions
from opentrons.util.broker import ReadOnlyBroker

from .legacy_wrappers import LegacyLoadInfo
from .legacy_command_mapper import LegacyCommandMapper
from .thread_async_queue import ThreadAsyncQueue

Expand All @@ -37,7 +37,7 @@ class LegacyContextPlugin(AbstractPlugin):
def __init__(
self,
broker: LegacyBroker,
equipment_broker: ReadOnlyBroker[LegacyLoadInfo],
equipment_broker: ReadOnlyBroker[LoadInfo],
legacy_command_mapper: Optional[LegacyCommandMapper] = None,
) -> None:
"""Initialize the plugin with its dependencies."""
Expand Down Expand Up @@ -129,7 +129,7 @@ def _handle_legacy_command(self, command: LegacyCommand) -> None:
pe_actions = self._legacy_command_mapper.map_command(command=command)
self._actions_to_dispatch.put(pe_actions)

def _handle_equipment_loaded(self, load_info: LegacyLoadInfo) -> None:
def _handle_equipment_loaded(self, load_info: LoadInfo) -> None:
"""Handle an equipment load reported by the APIv2 protocol.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can update these docstrings to be clearer too.

Suggested change
"""Handle an equipment load reported by the APIv2 protocol.
"""Handle an equipment load reported by the legacy APIv2 protocol.


Used as a broker callback, so this will run in the APIv2 protocol's thread.
Expand Down
51 changes: 28 additions & 23 deletions api/src/opentrons/protocol_runner/protocol_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from opentrons import protocol_reader
from opentrons.legacy_broker import LegacyBroker
from opentrons.protocol_api import ParameterContext
from opentrons.protocol_api.core.legacy.load_info import LoadInfo
from opentrons.protocol_reader import (
ProtocolSource,
JsonProtocolConfig,
Expand All @@ -28,13 +29,12 @@
from .json_file_reader import JsonFileReader
from .json_translator import JsonTranslator
from .legacy_context_plugin import LegacyContextPlugin
from .legacy_wrappers import (
from .python_protocol_wrappers import (
LEGACY_PYTHON_API_VERSION_CUTOFF,
LEGACY_JSON_SCHEMA_VERSION_CUTOFF,
LegacyFileReader,
LegacyContextCreator,
LegacyExecutor,
LegacyLoadInfo,
PythonProtocolFileReader,
ProtocolContextCreator,
PythonProtocolExecutor,
)
from ..protocol_engine.errors import ProtocolCommandFailedError
from ..protocol_engine.types import (
Expand Down Expand Up @@ -136,21 +136,26 @@ def __init__(
protocol_engine: ProtocolEngine,
hardware_api: HardwareControlAPI,
task_queue: Optional[TaskQueue] = None,
legacy_file_reader: Optional[LegacyFileReader] = None,
legacy_context_creator: Optional[LegacyContextCreator] = None,
legacy_executor: Optional[LegacyExecutor] = None,
python_protocol_file_reader: Optional[PythonProtocolFileReader] = None,
protocol_context_creator: Optional[ProtocolContextCreator] = None,
python_protocol_executor: Optional[PythonProtocolExecutor] = None,
post_run_hardware_state: PostRunHardwareState = PostRunHardwareState.HOME_AND_STAY_ENGAGED,
drop_tips_after_run: bool = True,
) -> None:
"""Initialize the PythonAndLegacyRunner with its dependencies."""
super().__init__(protocol_engine)
self._hardware_api = hardware_api
self._legacy_file_reader = legacy_file_reader or LegacyFileReader()
self._legacy_context_creator = legacy_context_creator or LegacyContextCreator(
hardware_api=hardware_api,
protocol_engine=protocol_engine,
self._protocol_file_reader = (
python_protocol_file_reader or PythonProtocolFileReader()
)
self._legacy_executor = legacy_executor or LegacyExecutor()
self._protocol_context_creator = (
protocol_context_creator
or ProtocolContextCreator(
hardware_api=hardware_api,
protocol_engine=protocol_engine,
)
)
self._protocol_executor = python_protocol_executor or PythonProtocolExecutor()
# TODO(mc, 2022-01-11): replace task queue with specific implementations
# of runner interface
self._task_queue = task_queue or TaskQueue()
Expand Down Expand Up @@ -185,14 +190,14 @@ async def load(

# fixme(mm, 2022-12-23): This does I/O and compute-bound parsing that will block
# the event loop. Jira RSS-165.
protocol = self._legacy_file_reader.read(
protocol = self._protocol_file_reader.read(
protocol_source, labware_definitions, python_parse_mode
)
self._parameter_context = ParameterContext(api_version=protocol.api_level)
equipment_broker = None

if protocol.api_level < LEGACY_PYTHON_API_VERSION_CUTOFF:
equipment_broker = Broker[LegacyLoadInfo]()
equipment_broker = Broker[LoadInfo]()
self._protocol_engine.add_plugin(
LegacyContextPlugin(
broker=self._broker, equipment_broker=equipment_broker
Expand All @@ -202,7 +207,7 @@ async def load(
else:
self._hardware_api.should_taskify_movement_execution(taskify=False)

context = self._legacy_context_creator.create(
context = self._protocol_context_creator.create(
protocol=protocol,
broker=self._broker,
equipment_broker=equipment_broker,
Expand All @@ -216,7 +221,7 @@ async def run_func() -> None:
await self._protocol_engine.add_and_execute_command(
request=initial_home_command
)
await self._legacy_executor.execute(
await self._protocol_executor.execute(
protocol=protocol,
context=context,
parameter_context=self._parameter_context,
Expand Down Expand Up @@ -417,9 +422,9 @@ def create_protocol_runner(
task_queue: Optional[TaskQueue] = None,
json_file_reader: Optional[JsonFileReader] = None,
json_translator: Optional[JsonTranslator] = None,
legacy_file_reader: Optional[LegacyFileReader] = None,
legacy_context_creator: Optional[LegacyContextCreator] = None,
legacy_executor: Optional[LegacyExecutor] = None,
python_protocol_file_reader: Optional[PythonProtocolFileReader] = None,
protocol_context_creator: Optional[ProtocolContextCreator] = None,
python_protocol_executor: Optional[PythonProtocolExecutor] = None,
post_run_hardware_state: PostRunHardwareState = PostRunHardwareState.HOME_AND_STAY_ENGAGED,
drop_tips_after_run: bool = True,
) -> AnyRunner:
Expand All @@ -443,9 +448,9 @@ def create_protocol_runner(
protocol_engine=protocol_engine,
hardware_api=hardware_api,
task_queue=task_queue,
legacy_file_reader=legacy_file_reader,
legacy_context_creator=legacy_context_creator,
legacy_executor=legacy_executor,
python_protocol_file_reader=python_protocol_file_reader,
protocol_context_creator=protocol_context_creator,
python_protocol_executor=python_protocol_executor,
post_run_hardware_state=post_run_hardware_state,
drop_tips_after_run=drop_tips_after_run,
)
Expand Down
Loading
Loading