feat(api): Add protocol api support for partial tip configurations #13838

Laura-Danielle · 2023-10-24T23:32:22Z

Overview

This pull request adds support for partial tip configuration in the python protocol api. It handles and closes three separate tickets (RSS-302, RSS-354, RSS-355). After this work is complete, a user should be able to configure their pipette in a partial tip mode. The TODO list needs to be addressed before this PR can be tested, but the general structure is ready for review.

TODO

Finish cleaning up/adding tests
Add better handling around column/row use-cases in choosing the ending pipette
Expose NozzleMap better in the hardware API

Test Plan

It's another big 'un but we can strategize and choose to only test the MVP use-cases for single tip pick up and full column pick up. When this PR is finished, it should be able to

Perform single tip pick up on 8 channel using 'A1'
Perform single tip pick up on 8 channel using 'H1'
Perform column pick up on 96 channel using 'A1'
Perform column pick up on 96 channel using 'A12' (or 'H12'?)
Perform single tip pick up on 96 channel using 'A1'
Perform single tip pick up on 96 channel using 'H12'

All of the above configurations should also be run in a small protocol re-verify everything is still moving to the right place.
We should also test the 8 channel on the OT-2.

Changelog

Add a new protocol engine command to configure the pipette nozzle layout
Add a new state field which saves the current nozzle map configuration by pipette id
Add a new protocol api function which takes in dataclasses described in _nozzle_map.py to configure a nozzle state

Review requests

Originally wanted to separate out the commits better, but it ended up being kind of a mess. Devs please review all. You can split it up into protocol_engine files and protocol_api files to more easily review.

@ecormany you only need to review instrument_context.py and the docstrings there. I am definitely sure there are a bunch of edits we'll need to make. Ping me if you have questions.

Risk assessment

Low. Adding a new feature (yay).

codecov · 2023-10-24T23:34:10Z

Codecov Report

Merging #13838 (cab5309) into feat-partial-tip-configuration (0302955) will not change coverage.
Report is 4 commits behind head on feat-partial-tip-configuration.
The diff coverage is n/a.

Additional details and impacted files

@@                       Coverage Diff                       @@
##           feat-partial-tip-configuration   #13838   +/-   ##
===============================================================
  Coverage                           71.29%   71.29%           
===============================================================
  Files                                2423     2423           
  Lines                               68195    68195           
  Branches                             7972     7972           
===============================================================
  Hits                                48621    48621           
  Misses                              17702    17702           
  Partials                             1872     1872

Flag	Coverage Δ
notify-server	`89.13% <ø> (ø)`

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

Files	Coverage Δ
api/src/opentrons/hardware_control/api.py	`82.71% <ø> (ø)`
api/src/opentrons/hardware_control/dev_types.py	`100.00% <ø> (ø)`
api/src/opentrons/hardware_control/ot3api.py	`79.59% <ø> (ø)`
...trons/hardware_control/protocols/liquid_handler.py	`64.70% <ø> (ø)`
api/src/opentrons/protocol_api/__init__.py	`100.00% <ø> (ø)`
...i/src/opentrons/protocol_api/instrument_context.py	`88.95% <ø> (ø)`
api/src/opentrons/protocol_engine/__init__.py	`100.00% <ø> (ø)`
...c/opentrons/protocol_engine/clients/sync_client.py	`100.00% <ø> (ø)`
...src/opentrons/protocol_engine/commands/__init__.py	`100.00% <ø> (ø)`
...entrons/protocol_engine/commands/command_unions.py	`100.00% <ø> (ø)`
... and 5 more

sfoster1

I really think we should change the protocol API interface, but the big thing for me is that we gotta think about where those NozzleMap and NozzleConfigurationType types live if they're going to be part of the hardware controller's external interface. They shouldn't be defined so deep in the tree, they should be at the top level. That's true just for good practice reasons, but it's also true because we want to move a lot of hardware controller to a different package that the api can run without, so we'll need it to be in a typing stub kind of place, and we should put it there now. Even just hardware_control.types would be a good place.

api/src/opentrons/protocol_api/instrument_context.py

sfoster1 · 2023-10-25T14:51:00Z

api/src/opentrons/protocol_api/instrument_context.py

+            # Resets the pipette configuration to default
+            instr.configure_nozzle_layout()
+        """
+        if self._core.has_tip():


Organizationally I'm pretty sure this stuff should be in the engine if it's this important to check. I don't think we can enforce it with schemas or typechecking so the engine will have to make sure json-formatted protocols get it right also

and by "this stuff" i mean

checking if we already have a tip, definitely something the engine has to do

checking that we're only requesting columns on 8s and 96s

checking that we're only requesting quadrants or rows on 96s

sfoster1 · 2023-10-25T14:53:01Z

api/src/opentrons/protocol_engine/commands/configure_nozzle_layout.py

+        self, params: ConfigureNozzleLayoutParams
+    ) -> Tuple[ConfigureNozzleLayoutResult, ConfigureNozzleLayoutPrivateResult]:
+        """Check that requested pipette can support the requested nozzle layout."""
+


like, here is where we should do the validation probably

api/src/opentrons/protocol_engine/commands/configure_nozzle_layout.py

api/src/opentrons/protocol_engine/commands/configuring_common.py

sanni-t · 2023-10-25T22:18:13Z

api/src/opentrons/protocol_engine/commands/configure_nozzle_layout.py

+        )
+
+        return ConfigureNozzleLayoutResult(), ConfigureNozzleLayoutPrivateResult(
+            pipette_id=params.pipetteId,


I don't think pipette_id is needed in the result

We need it to save the layout so that it is keyed by pipette id and so that we can look it up later.

sanni-t · 2023-11-01T20:54:09Z

api/src/opentrons/protocol_engine/state/pipettes.py

@@ -143,6 +152,10 @@ def _handle_command(  # noqa: C901
                nozzle_offset_z=config.nozzle_offset_z,
            )
            self._state.flow_rates_by_id[private_result.pipette_id] = config.flow_rates
+        elif isinstance(private_result, PipetteNozzleLayoutResultMixin):
+            self._state.nozzle_configuration_by_id[
+                private_result.pipette_id


You can use command.params.pipetteId to get the pipette ID for this command. So you won't need to include pipettte_id in the results

CaseyBatten

Ran this on RSS flex with most recent changes and was able to successfully do partial tip pickup, perform aspirations and dispenses, drop tips, reconfigure for whole 96 channel pickup and repeat.
However important note:
Under the shared-data pipette json definitions for the 96 channel, the "presses" parameter under the "pickUpTipConfigurations" has to be changed from a 0.0 to a 1.0 in order to successfully pick up the tips, otherwise the 96-channel would not make contact with the tips.

sfoster1

Sure, sounds good to merge I guess. We really need to fix the gcode tests.

jwwojak

@Laura-Danielle Added a small suggestion for your consideration. Don't let me be a blocker to moving forward. We can always edit docstrings at a later date.

jwwojak · 2023-11-02T14:20:50Z

api/src/opentrons/protocol_api/instrument_context.py

+        """Configure a pipette to pick up less than the maximum tip capacity. The pipette
+        will remain in its partial state until this function is called again without any inputs. All subsequent
+        pipetting calls will execute with the new nozzle layout meaning that the pipette will perform
+        robot moves in the set nozzle layout.


Here's a suggestion for the start of the description:

Configure a pipette to pick up less than the tip's maximum rated capacity. The pipette will continue to aspirate in this reduced capacity until configure_nozzle_layout is called again with no arguments.

I don't know enough about what this method does to understand part of the last sentence in italics:

All subsequent pipetting calls will execute with the new nozzle layout meaning that the pipette will perform robot moves in the set nozzle layout.

Just a reminder, even though this is a python file with a .py file extension, our docs and docstrings are rendered by Sphinx. This means to get code type font, you need to put two single quotes `` around the code snippet. For example:

Certain pipettes are restricted to a subset of ``NozzleLayout`` types.

…13838)

Laura-Danielle added 5 commits October 25, 2023 02:21

add configure nozzle manager in state

414193f

add configure nozzle command

04e0e6c

add virtual nozzle manager handler

e8c5c63

additional cleanup to protocol engine command

695b4b4

feat(api): add api function for partial tip configurations

7353369

Laura-Danielle requested a review from ecormany October 24, 2023 23:32

sfoster1 requested changes Oct 25, 2023

View reviewed changes

sanni-t reviewed Oct 25, 2023

View reviewed changes

Laura-Danielle and others added 10 commits October 26, 2023 23:16

better handle resetting config and getting access to nozzle map

e2e025a

move nozzle manager

0864bc5

update protocol api and command engine to new contract

9524095

fix syntax errors for testing

ff6efaa

formatting and linter errors

32df8e3

add error handling and more tests

162db87

fixed primary nozzle value check, refactored a bit

30e5da8

fix reset logic in hardware controller

184d07c

fix logic problem when starting nozzle isn't A1

a84f00b

finish adding tests and linter

b3056a8

Laura-Danielle marked this pull request as ready for review November 1, 2023 20:44

Laura-Danielle requested a review from a team as a code owner November 1, 2023 20:44

Laura-Danielle requested a review from sfoster1 November 1, 2023 20:44

finish docstring thoughts

cab5309

sanni-t reviewed Nov 1, 2023

View reviewed changes

CaseyBatten approved these changes Nov 1, 2023

View reviewed changes

sfoster1 approved these changes Nov 2, 2023

View reviewed changes

Laura-Danielle merged commit dc07bf0 into feat-partial-tip-configuration Nov 2, 2023
16 of 25 checks passed

Laura-Danielle deleted the RSS-302-track-nozzle-configurations-pe branch November 2, 2023 14:13

jwwojak reviewed Nov 2, 2023

View reviewed changes

Laura-Danielle added a commit that referenced this pull request Nov 2, 2023

feat(api): Add protocol api support for partial tip configurations (#…

f7cec25

…13838)

sanni-t mentioned this pull request Nov 7, 2023

feat(api): Partial tip configuration #13936

Merged

sfoster1 pushed a commit that referenced this pull request Nov 7, 2023

feat(api): Add protocol api support for partial tip configurations (#…

a9cd7f9

…13838)

ecormany mentioned this pull request Nov 16, 2023

docs(api): edit docstring for configure_nozzle_layout #13997

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(api): Add protocol api support for partial tip configurations #13838

feat(api): Add protocol api support for partial tip configurations #13838

Laura-Danielle commented Oct 24, 2023 •

edited

Loading

codecov bot commented Oct 24, 2023 •

edited

Loading

sfoster1 left a comment

sfoster1 Oct 25, 2023

sfoster1 Oct 25, 2023

sfoster1 Oct 25, 2023

sanni-t Oct 25, 2023

Laura-Danielle Oct 26, 2023

sanni-t Nov 1, 2023

CaseyBatten left a comment

sfoster1 left a comment

jwwojak left a comment

jwwojak Nov 2, 2023

feat(api): Add protocol api support for partial tip configurations #13838

feat(api): Add protocol api support for partial tip configurations #13838

Conversation

Laura-Danielle commented Oct 24, 2023 • edited Loading

Overview

TODO

Test Plan

Changelog

Review requests

Risk assessment

codecov bot commented Oct 24, 2023 • edited Loading

Codecov Report

sfoster1 left a comment

Choose a reason for hiding this comment

sfoster1 Oct 25, 2023

Choose a reason for hiding this comment

sfoster1 Oct 25, 2023

Choose a reason for hiding this comment

sfoster1 Oct 25, 2023

Choose a reason for hiding this comment

sanni-t Oct 25, 2023

Choose a reason for hiding this comment

Laura-Danielle Oct 26, 2023

Choose a reason for hiding this comment

sanni-t Nov 1, 2023

Choose a reason for hiding this comment

CaseyBatten left a comment

Choose a reason for hiding this comment

sfoster1 left a comment

Choose a reason for hiding this comment

jwwojak left a comment

Choose a reason for hiding this comment

jwwojak Nov 2, 2023

Choose a reason for hiding this comment

Laura-Danielle commented Oct 24, 2023 •

edited

Loading

codecov bot commented Oct 24, 2023 •

edited

Loading