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

feat(hardware-testing): collected work for upgrading scripts to PE #14245

Merged
merged 32 commits into from
Apr 4, 2024

Conversation

ryanthecoder
Copy link
Contributor

@ryanthecoder ryanthecoder commented Dec 19, 2023

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.

Test Plan

Changelog

Review requests

Risk assessment

Copy link

codecov bot commented Dec 19, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.43%. Comparing base (d803d78) to head (a42ffb4).
Report is 12 commits behind head on edge.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             edge   #14245       +/-   ##
===========================================
+ Coverage   67.19%   92.43%   +25.24%     
===========================================
  Files        2495       77     -2418     
  Lines       71380     1283    -70097     
  Branches     8981        0     -8981     
===========================================
- Hits        47967     1186    -46781     
+ Misses      21301       97    -21204     
+ Partials     2112        0     -2112     
Flag Coverage Δ
api ?
app ?
components ?
g-code-testing 92.43% <ø> (ø)
hardware ?
hardware-testing ?
labware-library ?
notify-server ?
ot3-gravimetric-test ?
protocol-designer ?
react-api-client ?
robot-server ?
shared-data ?
step-generation ?
system-server ?
update-server ?
usb-bridge ?

Flags with carried forward coverage won't be shown. Click here to find out more.

see 2418 files with indirect coverage changes

@ryanthecoder ryanthecoder force-pushed the RH-10-volumetric-epic branch 2 times, most recently from fc5399e to f787e87 Compare December 21, 2023 15:58
@ryanthecoder ryanthecoder marked this pull request as ready for review December 21, 2023 15:58
@ryanthecoder ryanthecoder requested review from a team as code owners December 21, 2023 15:58
@ryanthecoder ryanthecoder force-pushed the RH-10-volumetric-epic branch 2 times, most recently from 961f873 to 3b637a2 Compare January 4, 2024 16:21
@ryanthecoder ryanthecoder force-pushed the RH-10-volumetric-epic branch from aa4fdd2 to a9feef5 Compare January 25, 2024 16:30
@ryanthecoder ryanthecoder force-pushed the RH-10-volumetric-epic branch 2 times, most recently from 397c8f5 to a1e1102 Compare March 4, 2024 21:26
Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

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

Some stuff we should definitely change - we cannot add retract retroactively liks this, and I think we should implement it differently in the engine core

api/src/opentrons/protocol_api/core/engine/instrument.py Outdated Show resolved Hide resolved
api/src/opentrons/protocol_api/instrument_context.py Outdated Show resolved Hide resolved
hardware-testing/Makefile Outdated Show resolved Hide resolved
@ryanthecoder ryanthecoder force-pushed the RH-10-volumetric-epic branch from 42060a4 to 2fc8936 Compare March 13, 2024 16:07
@ryanthecoder ryanthecoder force-pushed the RH-10-volumetric-epic branch from 54145d5 to fb78956 Compare March 21, 2024 20:04
Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

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

Looks good to me, love the change to retract

@ryanthecoder ryanthecoder force-pushed the RH-10-volumetric-epic branch 2 times, most recently from 1e291fa to 650b34a Compare March 25, 2024 20:32
@ryanthecoder ryanthecoder requested a review from a team as a code owner March 25, 2024 20:32
@caila-marashaj caila-marashaj force-pushed the RH-10-volumetric-epic branch from 24dca79 to 435857e Compare March 28, 2024 16:11
* fix some errors in the simulation of the script

* Switch over the api version to 2.15

* switch protocol engine to using the hardware emulation instead of virtual pipettes

* Revert "switch protocol engine to using the hardware emulation instead of virtual pipettes"

This reverts commit 5eaef14.

* gate virutal pipettes between get_protocol_api and simulate

* use push-out and air-gap in the pipetting.py now that we've upgraded to PE.

* fix the pipette loading when switching over to PE

* improve the way we do labware offsets

* fix up a few remaining calls to old functions

* format

* linting fixes

* fix liquid height test script
* override the pipette guards at runtime instead of with a patch

* remove the api patch for gravimetric tests

* format/lint
* remove custom adapters

* use the new load_labware with adapters

* remove unneeded retracts now that it properly knows how to move and avoid adapters

* update LPC protocols to use real tiprack adapters

* format lint

* don't try and load the old custom def

* get lpc protocols passsing analaysis

* workaround a LPC bug (RAUT-897) with moving adapters

* format

* lint
…14163)

We don't need the gravimetric patches anymore so this comments them out
and changes the push-ot3-gravimetric to not push anything besides the
custom labware defs (for the radwag vial) and the hardware-testing subdirectory
this is in an effort to avoid changing the existing software on the bot
…erface. (#14166)

* call instrument context configure

* don't force an ALL tip config, its that by default and breaks the single channel
…urrent (#14200)

* call instrument context configure

* don't force an ALL tip config, its that by default and breaks the single channel

* simplify found model loading
* call instrument context configure

* don't force an ALL tip config, its that by default and breaks the single channel

* remove easy instances of using hw_api

* at least unify all the calls of get_hardware

* call retract less now that we don't need it as much with the tip rack adapter feature

* use some updated protocol api commands

* add a retract command to intrument context

* we have to load the trash manually now

* format

* fix tests
@ryanthecoder ryanthecoder force-pushed the RH-10-volumetric-epic branch from d7165c7 to 68cdd2f Compare April 3, 2024 15:26
@ryanthecoder ryanthecoder force-pushed the RH-10-volumetric-epic branch from 68cdd2f to a42ffb4 Compare April 4, 2024 14:44
@ryanthecoder ryanthecoder merged commit 3c5d160 into edge Apr 4, 2024
23 checks passed
Carlos-fernandez pushed a commit that referenced this pull request May 20, 2024
…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.
-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants