feat(api): add liquid sensing functionality #11926

caila-marashaj · 2022-12-22T20:59:57Z

This pr implements a liquid-level sensing function in ot3api.py. This function takes in a mount, and a set of LiquidProbeSettings, which can be used to adjust various details of the process.

The mount first moves down to the starting_mount_height, to move the pipette tip close enough to the surface of the liquid to start reading. Then, it will set the pressure sensor's threshold for considering liquid to be found, and use the bind_output context manager to trigger the sync line the first time that pressure threshold is detected by the sensor. After this, the specified mount will move down toward the liquid's surface, while the pressure sensor is continuously reading, until the pipette tip touches the liquid. The pipette plunger needs to be moving simultaneously while the z motor is moving to maintain the internal pressure of the pipette and avoid triggering the sync line prematurely. This is done in the "blow-out" direction by default, but can be reversed with the aspirate_while_sensing parameter. The velocities of the mount and plunger during this move can also be specified in the LiquidProbeSettings.

To avoid crashing, only a given range of distances in the z direction will be accepted for liquid sensing. If the given max_z_distance is exceeded or min_z_distance not reached before the sync line is triggered, a LiquidNotFound or EarlyLiquidSenseTrigger error will be thrown after the move is completed.

The creation of the LogListener class allows all of the sensor readings received to be plotted in a csv file against time, with relevant metadata being stored in the first row. For liquid sensing, this option is represented by the log_pressure parameter.

codecov · 2022-12-22T21:05:18Z

Codecov Report

Merging #11926 (48691db) into edge (60efb43) will increase coverage by 0.01%.
The diff coverage is 83.58%.

@@            Coverage Diff             @@
##             edge   #11926      +/-   ##
==========================================
+ Coverage   74.03%   74.05%   +0.01%     
==========================================
  Files        2172     2172              
  Lines       59831    59890      +59     
  Branches     6145     6145              
==========================================
+ Hits        44297    44351      +54     
- Misses      14151    14156       +5     
  Partials     1383     1383

Flag	Coverage Δ
app	`72.51% <ø> (ø)`
hardware	`59.20% <83.58%> (+0.35%)`	⬆️
notify-server	`89.13% <ø> (ø)`
react-api-client	`75.61% <ø> (ø)`

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

Impacted Files	Coverage Δ
api/src/opentrons/config/defaults_ot3.py	`88.23% <ø> (ø)`
api/src/opentrons/config/types.py	`100.00% <ø> (ø)`
...entrons/hardware_control/backends/ot3controller.py	`64.61% <ø> (ø)`
...pentrons/hardware_control/backends/ot3simulator.py	`88.48% <ø> (-0.09%)`	⬇️
api/src/opentrons/hardware_control/ot3api.py	`80.63% <ø> (ø)`
api/src/opentrons/hardware_control/types.py	`96.51% <ø> (ø)`
...rdware/opentrons_hardware/sensors/sensor_driver.py	`84.94% <71.79%> (-9.60%)`	⬇️
...pentrons_hardware/hardware_control/tool_sensors.py	`98.50% <100.00%> (+0.72%)`	⬆️
...rons_hardware/firmware_bindings/messages/fields.py	`64.40% <0.00%> (+2.25%)`	⬆️
...ardware/opentrons_hardware/sensors/sensor_types.py	`95.94% <0.00%> (+2.70%)`	⬆️

api/src/opentrons/hardware_control/ot3api.py

sfoster1

I think that we should take a step back and consider what we want out of this API.
The goal is that

Assuming
- the pipette is already in the correct XY position
Given
- a suspected nominal location for the top of the liquid
- a range of possibilities for the height of the liquid
- a pipette (with a tip) to use to check the height of the liquid
Determine
- either the height of the liquid
- or that the height of the liquid cannot be found

To me, that means that we need to change the signature of this function somewhat. For instance

It should return the height of the liquid in deck coordinates
It should take a range of possibilities for the height of the liquid
it should perform hardware state safety checks

It should figure out on its own where to move the pipette in z to determine this. We don't want the hardware control layer to have to understand things about labware, so we should rely on higher layers to tell us information about where the liquid will probably be found. For instance, the caller can give us maximum and minimum absolute heights in deck coordinates corresponding to the top and bottom of the well being tested. This function can then use its configuration to move to an appropriate position above the top of the range and move to an appropriate position around the bottom of the range. That probably means we'll have to change the config to contain things like a margin above the top of the liquid that is required for the move to spool up, and a margin below the bottom of the liquid that is required to safely stop, rather than absolute positions. The hardware control layer in ot3api can then determine the distance to move the pipette and the mount.

It should also perform its own safety checks. For instance, it should raise an exception if a tip isn't present, because when it moves around it will need to do so based on the length of the tip. It should check that the aspirated volume is 0, since it'll need to raise the plunger to prepare, which would be dangerous if the tip contained liquid.

We probably may also want this configuration to live in pipette settings, since pressures at least will likely be very different based on pressure chamber and probably tip geometry, but that could be in a follow up PR.

sfoster1 · 2023-01-05T18:58:20Z

api/src/opentrons/hardware_control/backends/ot3controller.py

@@ -628,6 +632,16 @@ async def watch(self, loop: asyncio.AbstractEventLoop) -> None:
 while can_watch and (not self._event_watcher.closed):
 await self._handle_watch_event()

+ def get_slot_center_pos(self, slot_num: int) -> OT3AxisMap[float]:


this doesn't seem to be used and doesn't really belong in this layer anyway, maybe get rid of it?

^ bump here, this needs to be removed.

sfoster1 · 2023-01-05T19:02:02Z

hardware/opentrons_hardware/hardware_control/tool_sensors.py

+ if NodeId.pipette_left in movers or NodeId.pipette_right in movers:
+ primary_mover = [ax for ax in movers if ax in [NodeId.head_l, NodeId.head_r]][0]
+ else:
+ primary_mover = movers[0]


the time should be the same for all the movers, right? so we could always just do this

sfoster1

This is awesome and getting there! Couple more things we should change in the hardware control layer.

api/src/opentrons/hardware_control/backends/ot3simulator.py

api/src/opentrons/hardware_control/ot3api.py

sfoster1 · 2023-01-24T14:33:40Z

api/src/opentrons/hardware_control/ot3api.py

+ )
+ except MoveConditionNotMet:
+ self._log.exception("Liquid Sensing failed- threshold never reached.")
+ self._current_position.clear()


do we need to clear the position? the liquid sensing part failed, sure, we don't know where the liquid is - but we certainly should still know where the pipette is.

sfoster1 · 2023-01-24T14:34:43Z

api/src/opentrons/hardware_control/ot3api.py

+ self._log.exception(
+ "Liquid Sensing failed- threshold reached too early."
+ )
+ self._current_position.clear()


same question with this clear position - we still know where the pipette is, even if we don't know where the liquid is

sfoster1 · 2023-01-24T14:37:56Z

api/src/opentrons/hardware_control/ot3api.py

+ )
+ self._encoder_current_position.update(encoder_position)
+
+ await self.home_plunger(mount)


here, do we need to home (meaning that we don't know the position) or is this more of a park (just getting everything out of the way)? I think it might be a good idea to not home the pipette, because frequently you're gonna want to

sense liquid height

move the pipette clear of the liquid

prep the plunger for aspiration

move the pipette down to the liquid

aspirate
Homing or parking the mount isn't really necessary and just sort of wastes time.

What might be a good idea instead is recovering up to the prep position specified by the liquid sense settings and then homing the plunger.

sfoster1 · 2023-01-24T14:39:11Z

api/src/opentrons/hardware_control/ot3api.py

+ a ThresholdReachedTooEarly error will be thrown.
+
+ Otherwise, the function will stop moving once the threshold is triggered,
+ home the mount and plunger to avoid collision, and return the height at


let's avoid using words like "height" here - this is the z position, in deck coordinates, in mm, of the position of the top of the liquid.

sfoster1 · 2023-01-24T14:41:36Z

api/src/opentrons/hardware_control/ot3api.py

+ self._current_position.clear()
+ raise
+ else:
+ final_mount_pos_um: float = positions[head_node_for_mount(mount)][0]


I think this section has some unit confusion. Is this value really in micrometers? or is it converted to millimeters by opentrons_hardware? if it is in um, then you definitely can't directly subtract the starting mount height from it, since that's in mm, and you should convert it back into mm before returning it. If it's in mm, then you should change the variable name.

Laura-Danielle

Mostly cleanup requests. Looking good in general.

Laura-Danielle · 2023-02-09T15:14:16Z

api/src/opentrons/config/defaults_ot3.py

+ log_pressure=True,
+ home_plunger_at_start=False,
+ aspirate_while_sensing=False,
+ read_only=False,


what's the difference between log_pressure and read_only? To me they mean essentially the same thing. We should pick only one config to keep here.

read_only sets the sensor to report instead of sync, so someone who uses this can just take a bunch of readings without necessarily stopping or having an error thrown. I put this in here so the testing team can use it, I think it can be restructured to be clearer though

Laura-Danielle · 2023-02-09T15:19:45Z

api/src/opentrons/config/defaults_ot3.py

+DEFAULT_LIQUID_PROBE_SETTINGS: Final[LiquidProbeSettings] = LiquidProbeSettings(
+ starting_mount_height=100,
+ prep_move_speed=10,
+ max_z_distance=40,


Let's keep the max/min z for now, but can we put a comment that we may want to think a little more about how we can cleanly handle triggering "early" or triggering "late"

Laura-Danielle · 2023-02-09T15:20:12Z

api/src/opentrons/hardware_control/backends/ot3controller.py

@@ -347,7 +351,7 @@ def _build_home_pipettes_runner(
 )

 distances_pipette = {
- ax: -1 * self.axis_bounds[ax][1] - self.axis_bounds[ax][0]
+ ax: -1 * self.phony_bounds[ax][1] - self.phony_bounds[ax][0]


Why do we have this change?

Laura-Danielle · 2023-02-09T15:20:39Z

api/src/opentrons/hardware_control/backends/ot3controller.py

@@ -628,6 +632,16 @@ async def watch(self, loop: asyncio.AbstractEventLoop) -> None:
 while can_watch and (not self._event_watcher.closed):
 await self._handle_watch_event()

+ def get_slot_center_pos(self, slot_num: int) -> OT3AxisMap[float]:


^ bump here, this needs to be removed.

Laura-Danielle · 2023-02-09T15:21:27Z

api/src/opentrons/hardware_control/backends/ot3controller.py

 @property
 def axis_bounds(self) -> OT3AxisMap[Tuple[float, float]]:
+ """Get the axis bounds."""
+ # TODO (CM): gripper axis bounds need to be defined
+ return {


What is the purpose of this change?

Can we put it in a separate PR if it's unrelated to liquid sensing?

also wonder this

Laura-Danielle · 2023-02-09T15:53:51Z

hardware/opentrons_hardware/hardware_control/tool_sensors.py

@@ -27,16 +43,167 @@
 ProbeTarget = Union[Literal[NodeId.pipette_left, NodeId.pipette_right, NodeId.gripper]]


-def _build_pass_step(mover: NodeId, distance: float, speed: float) -> MoveGroupStep:
+class SensorLog:
+ """Capture incoming sensor messages."""


Can we put this class in a better place, like a utils.py or something?

hardware/opentrons_hardware/hardware_control/tool_sensors.py

Laura-Danielle · 2023-02-09T16:00:56Z

hardware/opentrons_hardware/hardware_control/tool_sensors.py

+ )
+ binding.append(SensorOutputBinding.report)
+
+ async with sensor_driver.bind_output(messenger, pressure_sensor, binding):


nice, much cleaner :D

Laura-Danielle · 2023-02-09T16:02:01Z

hardware/opentrons_hardware/hardware_control/tool_sensors.py

+ stop_threshold=threshold_fixed_point,
+ )
+
+ if read_only:


yea not quite sure what this read_only stuff is for? If they want standalone pressure readings, they should use the capture_sensor script or whatever it's called rather than this function.

hardware/tests/opentrons_hardware/hardware_control/test_tool_sensors.py

sfoster1

This is looking really strong! There's some stuff to fix up still but it's all details that don't affect higher level functionality.

sfoster1 · 2023-02-10T20:01:16Z

api/src/opentrons/hardware_control/backends/ot3controller.py

@@ -628,6 +632,16 @@ async def watch(self, loop: asyncio.AbstractEventLoop) -> None:
 while can_watch and (not self._event_watcher.closed):
 await self._handle_watch_event()

+ def get_slot_center_pos(self, slot_num: int) -> OT3AxisMap[float]:


sfoster1 · 2023-02-10T20:01:27Z

api/src/opentrons/hardware_control/backends/ot3controller.py

 @property
 def axis_bounds(self) -> OT3AxisMap[Tuple[float, float]]:
+ """Get the axis bounds."""
+ # TODO (CM): gripper axis bounds need to be defined
+ return {


also wonder this

sfoster1 · 2023-02-10T20:02:10Z

api/src/opentrons/hardware_control/backends/ot3controller.py

+ plunger_speed: float,
+ threshold_pascals: float,
+ starting_mount_height: float,
+ prep_move_speed: float,


agree with the starting_mount_height thing, and this is part of that too right? We should pull that out

sfoster1 · 2023-02-10T20:04:06Z

api/src/opentrons/hardware_control/backends/ot3simulator.py

@@ -437,24 +454,47 @@ async def watch(self, loop: asyncio.AbstractEventLoop) -> None:
 ]
 await self.module_controls.register_modules(new_mods_at_ports=new_mods_at_ports)

+ def get_slot_center_pos(self, slot_num: int) -> OT3AxisMap[float]:


This should go away as it should in ot3controller

sfoster1 · 2023-02-10T20:04:26Z

api/src/opentrons/hardware_control/backends/ot3simulator.py

 @property
 def axis_bounds(self) -> OT3AxisMap[Tuple[float, float]]:
+ """Get the axis bounds."""


Still don't quite get the point of this change

hardware/opentrons_hardware/hardware_control/tool_sensors.py

hardware/opentrons_hardware/sensors/sensor_driver.py

caila-marashaj · 2023-02-17T20:50:58Z

api/src/opentrons/hardware_control/ot3api.py

+ probe_settings.sensor_threshold_pascals,
+ probe_settings.log_pressure,
+ )
+ position = deck_from_machine(


I'm adding a call to axis_convert in here since deck_from_machine wants an AxisType dict and not NodeId

Laura-Danielle

BEAUTIFUL! Excited to get this in.

Changes addressed

caila-marashaj changed the title ~~Rliq 69 hardware control liquid sense~~ feat(api): add liquid sensing function to api Dec 22, 2022

caila-marashaj changed the title ~~feat(api): add liquid sensing function to api~~ feat(api): add liquid sensing functionality Dec 22, 2022

caila-marashaj commented Jan 4, 2023

View reviewed changes

api/src/opentrons/hardware_control/ot3api.py Outdated Show resolved Hide resolved

caila-marashaj marked this pull request as ready for review January 4, 2023 23:57

caila-marashaj requested a review from a team as a code owner January 4, 2023 23:57

caila-marashaj force-pushed the RLIQ-69-hardware-control-liquid-sense branch from 23e5648 to a632aca Compare January 5, 2023 15:45

caila-marashaj requested review from a team and removed request for a team January 5, 2023 16:03

sfoster1 requested changes Jan 5, 2023

View reviewed changes

caila-marashaj marked this pull request as draft January 9, 2023 20:23

caila-marashaj force-pushed the RLIQ-69-hardware-control-liquid-sense branch from 4e8abd1 to 864756e Compare January 20, 2023 20:20

caila-marashaj marked this pull request as ready for review January 20, 2023 21:52

caila-marashaj requested a review from a team January 20, 2023 21:53

sfoster1 requested changes Jan 24, 2023

View reviewed changes

caila-marashaj force-pushed the RLIQ-69-hardware-control-liquid-sense branch from 362610b to 768be59 Compare January 25, 2023 17:52

caila-marashaj added 12 commits January 25, 2023 13:51

add liquidsenseparams class

199f542

added liquid_probe ot3api function

bf44b76

changed head node in build_pass_step

bf222dd

fixed primary_mover bug

981cbfb

removing unneeded file

6be190a

fixed robot defaults test

f4d86e7

move threshold to settings, get rid of slot param

5124c46

changed threshold param from fixed pt to float

2cc0393

get rid of axis_bounds change in ot3simulator

b022804

remove extra function from ot3controller

c536ee0

remove math imports

921be27

moving intial move to tool_sensors

5b6155a

caila-marashaj and others added 2 commits February 8, 2023 16:42

update default values, remove stray comment

46ae9f3

Merge branch 'edge' into RLIQ-69-hardware-control-liquid-sense

4ec0ebc

Laura-Danielle requested changes Feb 9, 2023

View reviewed changes

caila-marashaj added 3 commits February 9, 2023 11:29

rebase on edge

bb73515

updating branch

fe8b132

updating branch

47ba406

caila-marashaj force-pushed the RLIQ-69-hardware-control-liquid-sense branch from c3715bf to 47ba406 Compare February 13, 2023 19:23

test sensor bind_output calls

18986b3

caila-marashaj force-pushed the RLIQ-69-hardware-control-liquid-sense branch from 07c68bc to 18986b3 Compare February 14, 2023 15:19

caila-marashaj added 5 commits February 14, 2023 10:53

some cleanup

792d744

move prep move to ot3api

4d77fa8

move max_z check to ot3api

3979ecc

format

139335c

updating docstring

a86fb9b

caila-marashaj force-pushed the RLIQ-69-hardware-control-liquid-sense branch from 6758857 to a86fb9b Compare February 16, 2023 14:22

sfoster1 previously requested changes Feb 16, 2023

View reviewed changes

generalize LogListener

90ebf12

caila-marashaj force-pushed the RLIQ-69-hardware-control-liquid-sense branch from 30fed4b to 90ebf12 Compare February 17, 2023 20:34

caila-marashaj commented Feb 17, 2023

View reviewed changes

Laura-Danielle approved these changes Feb 17, 2023

View reviewed changes

update unit tests

daea942

caila-marashaj force-pushed the RLIQ-69-hardware-control-liquid-sense branch from cd41b39 to daea942 Compare February 17, 2023 23:13

take OT3Axis into liquid sensing errors

2b3522c

caila-marashaj force-pushed the RLIQ-69-hardware-control-liquid-sense branch from a12cad7 to 2b3522c Compare February 17, 2023 23:34

add new settings to robot config

58e821a

caila-marashaj force-pushed the RLIQ-69-hardware-control-liquid-sense branch from 468a34b to 58e821a Compare February 17, 2023 23:51

Merge branch 'edge' into RLIQ-69-hardware-control-liquid-sense

48691db

caila-marashaj merged commit b58e61b into edge Feb 21, 2023

caila-marashaj deleted the RLIQ-69-hardware-control-liquid-sense branch February 21, 2023 17:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(api): add liquid sensing functionality #11926

feat(api): add liquid sensing functionality #11926

caila-marashaj commented Dec 22, 2022 •

edited

Loading

codecov bot commented Dec 22, 2022 •

edited

Loading

sfoster1 left a comment

sfoster1 Jan 5, 2023

Laura-Danielle Feb 9, 2023

sfoster1 Feb 10, 2023

sfoster1 Jan 5, 2023

sfoster1 left a comment

sfoster1 Jan 24, 2023

sfoster1 Jan 24, 2023

sfoster1 Jan 24, 2023

sfoster1 Jan 24, 2023

sfoster1 Jan 24, 2023

Laura-Danielle left a comment

Laura-Danielle Feb 9, 2023

caila-marashaj Feb 9, 2023

Laura-Danielle Feb 9, 2023

Laura-Danielle Feb 9, 2023

Laura-Danielle Feb 9, 2023

Laura-Danielle Feb 9, 2023

sfoster1 Feb 10, 2023

Laura-Danielle Feb 9, 2023

Laura-Danielle Feb 9, 2023

Laura-Danielle Feb 9, 2023

sfoster1 left a comment

sfoster1 Feb 10, 2023

sfoster1 Feb 10, 2023

sfoster1 Feb 10, 2023

sfoster1 Feb 10, 2023

sfoster1 Feb 10, 2023

caila-marashaj Feb 17, 2023

Laura-Danielle left a comment

feat(api): add liquid sensing functionality #11926

feat(api): add liquid sensing functionality #11926

Conversation

caila-marashaj commented Dec 22, 2022 • edited Loading

codecov bot commented Dec 22, 2022 • edited Loading

Codecov Report

sfoster1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfoster1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Laura-Danielle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfoster1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Laura-Danielle left a comment

Choose a reason for hiding this comment

caila-marashaj commented Dec 22, 2022 •

edited

Loading

codecov bot commented Dec 22, 2022 •

edited

Loading