Skip to content

Commit

Permalink
feat(hardware-testing): collected work for upgrading scripts to PE (#…
Browse files Browse the repository at this point in the history
…14245)

<!--
Thanks for taking the time to open a pull request! Please make sure
you've read the "Opening Pull Requests" section of our Contributing
Guide:


https://github.com/Opentrons/opentrons/blob/edge/CONTRIBUTING.md#opening-pull-requests

To ensure your code is reviewed quickly and thoroughly, please fill out
the sections below to the best of your ability!
-->

# Overview
This upgrades our pipette QC scripts to using protocol engine interface,
this simplifies a lot of the code and lets us use new features:
  push out
  labware adapters
  using the normal ot3 deck definitions
  partial tip configuration

With the reduction in the amount of changes to the base repo required,
we now don't need to use patches before running the tests, this means we
only need the hardware-testing directory to be pushed to the bot with no
changes to the base software.

<!--
Use this section to describe your pull-request at a high level. If the
PR addresses any open issues, please tag the issues here.
-->

# Test Plan

<!--
Use this section to describe the steps that you took to test your Pull
Request.
If you did not perform any testing provide justification why.

OT-3 Developers: You should default to testing on actual physical
hardware.
Once again, if you did not perform testing against hardware, justify
why.

Note: It can be helpful to write a test plan before doing development

Example Test Plan (HTTP API Change)

- Verified that new optional argument `dance-party` causes the robot to
flash its lights, move the pipettes,
then home.
- Verified that when you omit the `dance-party` option the robot homes
normally
- Added protocol that uses `dance-party` argument to G-Code Testing
Suite
- Ran protocol that did not use `dance-party` argument and everything
was successful
- Added unit tests to validate that changes to pydantic model are
correct

-->

# Changelog

<!--
List out the changes to the code in this PR. Please try your best to
categorize your changes and describe what has changed and why.

Example changelog:
- Fixed app crash when trying to calibrate an illegal pipette
- Added state to API to track pipette usage
- Updated API docs to mention only two pipettes are supported

IMPORTANT: MAKE SURE ANY BREAKING CHANGES ARE PROPERLY COMMUNICATED
-->

# Review requests

<!--
Describe any requests for your reviewers here.
-->

# Risk assessment

<!--
Carefully go over your pull request and look at the other parts of the
codebase it may affect. Look for the possibility, even if you think it's
small, that your change may affect some other part of the system - for
instance, changing return tip behavior in protocol may also change the
behavior of labware calibration.

Identify the other parts of the system your codebase may affect, so that
in addition to your own review and testing, other people who may not
have the system internalized as much as you can focus their attention
and testing there.
-->
  • Loading branch information
ryanthecoder authored and Carlos-fernandez committed May 20, 2024
1 parent badfe60 commit 6c8fe24
Show file tree
Hide file tree
Showing 30 changed files with 549 additions and 4,312 deletions.
5 changes: 5 additions & 0 deletions api/src/opentrons/hardware_control/backends/ot3controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -1653,3 +1653,8 @@ async def get_hepa_uv_state(self) -> Optional[HepaUVState]:
if res
else None
)

def _update_tip_state(self, mount: OT3Mount, status: bool) -> None:
"""This is something we only use in the simulator.
It is required so that PE simulations using ot3api don't break."""
pass
16 changes: 14 additions & 2 deletions api/src/opentrons/hardware_control/backends/ot3simulator.py
Original file line number Diff line number Diff line change
Expand Up @@ -506,13 +506,20 @@ def _attached_pipette_to_mount(
),
"id": None,
}
if found_model and expected_instr or found_model:
if found_model and init_instr["id"] is not None:
# Instrument detected matches instrument expected (note:
# "instrument detected" means passed as an argument to the
# constructor of this class)

# OR Instrument detected and no expected instrument specified
converted_name = pipette_load_name.convert_pipette_model(found_model)

found_model_version = ""
if found_model.find("flex") > -1:
found_model = found_model.replace("_flex", "") # type: ignore
found_model_version = f"{init_instr['id'][4]}.{init_instr['id'][5]}"
converted_name = pipette_load_name.convert_pipette_model(
found_model, found_model_version
)
return {
"config": load_pipette_data.load_definition(
converted_name.pipette_type,
Expand Down Expand Up @@ -843,3 +850,8 @@ async def set_hepa_uv_state(self, light_on: bool, timeout_s: int) -> bool:

async def get_hepa_uv_state(self) -> Optional[HepaUVState]:
return None

def _update_tip_state(self, mount: OT3Mount, status: bool) -> None:
"""This is something we only use in the simulator.
It is required so that PE simulations using ot3api don't break."""
self._sim_tip_state[mount] = status
5 changes: 5 additions & 0 deletions api/src/opentrons/hardware_control/ot3api.py
Original file line number Diff line number Diff line change
Expand Up @@ -2153,6 +2153,8 @@ async def pick_up_tip(
def add_tip_to_instr() -> None:
instrument.add_tip(tip_length=tip_length)
instrument.set_current_volume(0)
if isinstance(self._backend, OT3Simulator):
self._backend._update_tip_state(realmount, True)

await self._move_to_plunger_bottom(realmount, rate=1.0)
if (
Expand Down Expand Up @@ -2255,6 +2257,9 @@ def _remove_tips() -> None:
await self._home([Axis.by_mount(mount)])

_remove_tips()
# call this in case we're simulating
if isinstance(self._backend, OT3Simulator):
self._backend._update_tip_state(realmount, False)

async def clean_up(self) -> None:
"""Get the API ready to stop cleanly."""
Expand Down
5 changes: 5 additions & 0 deletions api/src/opentrons/protocol_api/core/engine/instrument.py
Original file line number Diff line number Diff line change
Expand Up @@ -777,3 +777,8 @@ def configure_nozzle_layout(
self._engine_client.configure_nozzle_layout(
pipette_id=self._pipette_id, configuration_params=configuration_model
)

def retract(self) -> None:
"""Retract this instrument to the top of the gantry."""
z_axis = self._engine_client.state.pipettes.get_z_axis(self._pipette_id)
self._engine_client.home([z_axis])
3 changes: 3 additions & 0 deletions api/src/opentrons/protocol_api/core/instrument.py
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,9 @@ def configure_nozzle_layout(
@abstractmethod
def is_tip_tracking_available(self) -> bool:
"""Return whether auto tip tracking is available for the pipette's current nozzle configuration."""

def retract(self) -> None:
"""Retract this instrument to the top of the gantry."""
...


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -558,3 +558,7 @@ def get_nozzle_map(self) -> NozzleMap:
def is_tip_tracking_available(self) -> bool:
# Tip tracking is always available in legacy context
return True

def retract(self) -> None:
"""Retract this instrument to the top of the gantry."""
self._protocol_interface.get_hardware.retract(self._mount) # type: ignore [attr-defined]
Original file line number Diff line number Diff line change
Expand Up @@ -476,3 +476,7 @@ def get_nozzle_map(self) -> NozzleMap:
def is_tip_tracking_available(self) -> bool:
# Tip tracking is always available in legacy context
return True

def retract(self) -> None:
"""Retract this instrument to the top of the gantry."""
self._protocol_interface.get_hardware.retract(self._mount) # type: ignore [attr-defined]
6 changes: 6 additions & 0 deletions api/src/opentrons/protocol_api/instrument_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -1531,6 +1531,12 @@ def move_to(

return self

@requires_version(2, 18)
def _retract(
self,
) -> None:
self._core.retract()

@property
@requires_version(2, 0)
def mount(self) -> str:
Expand Down
18 changes: 12 additions & 6 deletions api/src/opentrons/simulate.py
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ def get_protocol_api(
# type checking, like Jupyter Notebook.
*,
robot_type: Optional[_UserSpecifiedRobotType] = None,
use_virtual_hardware: bool = True,
) -> protocol_api.ProtocolContext:
"""
Build and return a ``protocol_api.ProtocolContext``
Expand Down Expand Up @@ -260,6 +261,7 @@ def get_protocol_api(
: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.
:return: The protocol context.
"""
if isinstance(version, str):
Expand Down Expand Up @@ -317,6 +319,7 @@ def get_protocol_api(
hardware_api=checked_hardware,
bundled_data=bundled_data,
extra_labware=extra_labware,
use_virtual_hardware=use_virtual_hardware,
)

# Intentional difference from execute.get_protocol_api():
Expand Down Expand Up @@ -790,6 +793,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,
) -> ProtocolContext:
"""Return a live ProtocolContext that controls the robot through ProtocolEngine."""
assert api_version >= ENGINE_CORE_API_VERSION
Expand All @@ -798,7 +802,9 @@ def _create_live_context_pe(
pe, loop = _LIVE_PROTOCOL_ENGINE_CONTEXTS.enter_context(
create_protocol_engine_in_thread(
hardware_api=hardware_api.wrapped(),
config=_get_protocol_engine_config(robot_type),
config=_get_protocol_engine_config(
robot_type, virtual=use_virtual_hardware
),
drop_tips_after_run=False,
post_run_hardware_state=PostRunHardwareState.STAY_ENGAGED_IN_PLACE,
load_fixed_trash=should_load_fixed_trash_labware_for_python_protocol(
Expand Down Expand Up @@ -899,7 +905,7 @@ def _run_file_pe(
async def run(protocol_source: ProtocolSource) -> _SimulateResult:
protocol_engine = await create_protocol_engine(
hardware_api=hardware_api.wrapped(),
config=_get_protocol_engine_config(robot_type),
config=_get_protocol_engine_config(robot_type, virtual=True),
load_fixed_trash=should_load_fixed_trash(protocol_source.config),
)

Expand Down Expand Up @@ -934,15 +940,15 @@ async def run(protocol_source: ProtocolSource) -> _SimulateResult:
return asyncio.run(run(protocol_source))


def _get_protocol_engine_config(robot_type: RobotType) -> Config:
def _get_protocol_engine_config(robot_type: RobotType, virtual: 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=True,
use_virtual_modules=True,
use_virtual_gripper=True,
use_virtual_pipettes=virtual,
use_virtual_modules=virtual,
use_virtual_gripper=virtual,
use_simulated_deck_config=True,
)

Expand Down
20 changes: 4 additions & 16 deletions hardware-testing/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -79,36 +79,26 @@ sdist:

.PHONY: test
test:
-$(MAKE) apply-patches-gravimetric
$(pytest) $(tests) $(test_opts)
-$(MAKE) remove-patches-gravimetric

.PHONY: test-cov
test-cov:
-$(MAKE) apply-patches-gravimetric
$(pytest) $(tests) $(test_opts) $(cov_opts)
-$(MAKE) remove-patches-gravimetric

.PHONY: test-photometric-single
test-photometric-single:
-$(MAKE) apply-patches-gravimetric
$(python) -m hardware_testing.gravimetric --photometric --simulate --pipette 50 --channels 1 --tip 50
$(python) -m hardware_testing.gravimetric --photometric --simulate --pipette 50 --channels 1 --tip 50 --photoplate-col-offset 3
$(python) -m hardware_testing.gravimetric --photometric --simulate --pipette 50 --channels 1 --tip 50 --dye-well-col-offset 3
-$(MAKE) remove-patches-gravimetric

.PHONY: test-photometric-multi
test-photometric-multi:
-$(MAKE) apply-patches-gravimetric
$(python) -m hardware_testing.gravimetric --photometric --simulate --pipette 50 --channels 8 --tip 50
-$(MAKE) remove-patches-gravimetric

.PHONY: test-photometric
test-photometric:
-$(MAKE) apply-patches-gravimetric
$(python) -m hardware_testing.gravimetric --photometric --simulate --pipette 1000 --channels 96 --tip 50 --trials 1
$(python) -m hardware_testing.gravimetric --photometric --simulate --pipette 1000 --channels 96 --tip 200 --trials 1
-$(MAKE) remove-patches-gravimetric

.PHONY: test-gravimetric-single
test-gravimetric-single:
Expand All @@ -134,14 +124,12 @@ test-gravimetric-96:

.PHONY: test-gravimetric
test-gravimetric:
-$(MAKE) apply-patches-gravimetric
$(python) -m hardware_testing.gravimetric.daily_setup --simulate
$(python) -m hardware_testing.gravimetric.daily_setup --simulate --calibrate
$(MAKE) test-gravimetric-single
$(MAKE) test-gravimetric-multi
$(MAKE) test-gravimetric-96
$(MAKE) test-photometric
-$(MAKE) remove-patches-gravimetric

.PHONY: test-production-qc
test-production-qc:
Expand Down Expand Up @@ -172,11 +160,9 @@ test-integration: test-production-qc test-examples test-scripts test-gravimetric

.PHONY: lint
lint:
-$(MAKE) apply-patches-gravimetric
$(python) -m mypy hardware_testing tests
$(python) -m black --check hardware_testing tests setup.py
$(python) -m flake8 hardware_testing tests setup.py
-$(MAKE) remove-patches-gravimetric

.PHONY: format
format:
Expand Down Expand Up @@ -297,9 +283,11 @@ sync-ot3: sync-sw-ot3 sync-fw-ot3

.PHONY: push-ot3-gravimetric
push-ot3-gravimetric:
$(MAKE) push-ot3
ssh $(ssh_helper_ot3) root@$(host) "mkdir -p /data/labware/v2/custom_definitions/custom_beta"
scp $(ssh_helper_ot3) -r hardware_testing/labware/* root@$(host):/data/labware/v2/custom_definitions/custom_beta/
$(MAKE) apply-patches-gravimetric
-$(MAKE) sync-sw-ot3
scp $(ssh_helper_ot3) -r hardware_testing/labware root@$(host):/data/labware/v2/custom_definitions/custom_beta/
cd ../ && $(MAKE) -C shared-data push-ot3
$(MAKE) remove-patches-gravimetric

.PHONY: apply-patches-gravimetric
Expand Down
2 changes: 1 addition & 1 deletion hardware-testing/hardware_testing/drivers/asair_sensor.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def BuildAsairSensor(simulate: bool, autosearch: bool = True) -> AsairSensorBase
ui.print_info(f"Trying to connect to env sensor on port {port}")
sensor = AsairSensor.connect(port)
ser_id = sensor.get_serial()
if len(ser_id) != 0:
if ser_id == " ":
ui.print_info(f"Found env sensor {ser_id} on port {port}")
return sensor
except: # noqa: E722
Expand Down
Loading

0 comments on commit 6c8fe24

Please sign in to comment.