Skip to content

Commit

Permalink
feat(hardware-testing): enable multi sensor processing in liquid probe (
Browse files Browse the repository at this point in the history
#14883)

<!--
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 allows the liquid level detection script to tell the pipette to
buffer the data from both pipettes and fetch them afterwards,
it will now spit out seprate CSVs for each sensor. post processing not
yet updated so the final report just grabs one from each trial, will
implement in EXEC-268

<!--
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 74cf56e commit b3b65df
Show file tree
Hide file tree
Showing 21 changed files with 277 additions and 138 deletions.
64 changes: 59 additions & 5 deletions api/src/opentrons/config/defaults_ot3.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
from typing import Any, Dict, cast, List, Iterable, Tuple
from typing import Any, Dict, cast, List, Iterable, Tuple, Optional
from typing_extensions import Final
from dataclasses import asdict

from opentrons.hardware_control.types import OT3AxisKind
from opentrons.hardware_control.types import OT3AxisKind, InstrumentProbeType
from .types import (
OT3Config,
ByGantryLoad,
Expand Down Expand Up @@ -33,7 +33,7 @@
aspirate_while_sensing=False,
auto_zero_sensor=True,
num_baseline_reads=10,
data_file="/var/pressure_sensor_data.csv",
data_files={InstrumentProbeType.PRIMARY: "/data/pressure_sensor_data.csv"},
)

DEFAULT_CALIBRATION_SETTINGS: Final[OT3CalibrationSettings] = OT3CalibrationSettings(
Expand Down Expand Up @@ -193,6 +193,49 @@
)


def _build_output_option_with_default(
from_conf: Any, default: OutputOptions
) -> OutputOptions:
if from_conf is None:
return default
else:
if isinstance(from_conf, OutputOptions):
return from_conf
else:
try:
enumval = OutputOptions[from_conf]
except KeyError: # not an enum entry
return default
else:
return enumval


def _build_log_files_with_default(
from_conf: Any,
default: Optional[Dict[InstrumentProbeType, str]],
) -> Optional[Dict[InstrumentProbeType, str]]:
print(f"from_conf {from_conf} default {default}")
if not isinstance(from_conf, dict):
if default is None:
return None
else:
return {k: v for k, v in default.items()}
else:
validated: Dict[InstrumentProbeType, str] = {}
for k, v in from_conf.items():
if isinstance(k, InstrumentProbeType):
validated[k] = v
else:
try:
enumval = InstrumentProbeType[k]
except KeyError: # not an enum entry
pass
else:
validated[enumval] = v
print(f"result {validated}")
return validated


def _build_dict_with_default(
from_conf: Any,
default: Dict[OT3AxisKind, float],
Expand Down Expand Up @@ -277,6 +320,17 @@ def _build_default_cap_pass(
def _build_default_liquid_probe(
from_conf: Any, default: LiquidProbeSettings
) -> LiquidProbeSettings:
output_option = _build_output_option_with_default(
from_conf.get("output_option", None), default.output_option
)
data_files: Optional[Dict[InstrumentProbeType, str]] = None
if (
output_option is OutputOptions.sync_buffer_to_csv
or output_option is OutputOptions.stream_to_csv
):
data_files = _build_log_files_with_default(
from_conf.get("data_files", {}), default.data_files
)
return LiquidProbeSettings(
max_z_distance=from_conf.get("max_z_distance", default.max_z_distance),
min_z_distance=from_conf.get("min_z_distance", default.min_z_distance),
Expand All @@ -298,7 +352,7 @@ def _build_default_liquid_probe(
num_baseline_reads=from_conf.get(
"num_baseline_reads", default.num_baseline_reads
),
data_file=from_conf.get("data_file", default.data_file),
data_files=data_files,
)


Expand Down Expand Up @@ -408,7 +462,7 @@ def build_with_defaults(robot_settings: Dict[str, Any]) -> OT3Config:
def serialize(config: OT3Config) -> Dict[str, Any]:
def _build_dict(pairs: Iterable[Tuple[Any, Any]]) -> Dict[str, Any]:
def _normalize_key(key: Any) -> Any:
if isinstance(key, OT3AxisKind):
if isinstance(key, OT3AxisKind) or isinstance(key, InstrumentProbeType):
return key.name
return key

Expand Down
4 changes: 2 additions & 2 deletions api/src/opentrons/config/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from dataclasses import dataclass, asdict, fields
from typing import Dict, Tuple, TypeVar, Generic, List, cast, Optional
from typing_extensions import TypedDict, Literal
from opentrons.hardware_control.types import OT3AxisKind
from opentrons.hardware_control.types import OT3AxisKind, InstrumentProbeType


class AxisDict(TypedDict):
Expand Down Expand Up @@ -138,7 +138,7 @@ class LiquidProbeSettings:
aspirate_while_sensing: bool
auto_zero_sensor: bool
num_baseline_reads: int
data_file: Optional[str]
data_files: Optional[Dict[InstrumentProbeType, str]]


@dataclass(frozen=True)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ async def liquid_probe(
plunger_speed: float,
threshold_pascals: float,
output_format: OutputOptions = OutputOptions.can_bus_only,
data_file: Optional[str] = None,
data_files: Optional[Dict[InstrumentProbeType, str]] = None,
auto_zero_sensor: bool = True,
num_baseline_reads: int = 10,
probe: InstrumentProbeType = InstrumentProbeType.PRIMARY,
Expand Down
12 changes: 10 additions & 2 deletions api/src/opentrons/hardware_control/backends/ot3controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -1357,7 +1357,7 @@ async def liquid_probe(
plunger_speed: float,
threshold_pascals: float,
output_option: OutputOptions = OutputOptions.can_bus_only,
data_file: Optional[str] = None,
data_files: Optional[Dict[InstrumentProbeType, str]] = None,
auto_zero_sensor: bool = True,
num_baseline_reads: int = 10,
probe: InstrumentProbeType = InstrumentProbeType.PRIMARY,
Expand All @@ -1378,6 +1378,14 @@ async def liquid_probe(
can_bus_only_output = bool(
output_option.value & OutputOptions.can_bus_only.value
)
data_files_transposed = (
None
if data_files is None
else {
sensor_id_for_instrument(probe): data_files[probe]
for probe in data_files.keys()
}
)
positions = await liquid_probe(
messenger=self._messenger,
tool=tool,
Expand All @@ -1389,7 +1397,7 @@ async def liquid_probe(
csv_output=csv_output,
sync_buffer_output=sync_buffer_output,
can_bus_only_output=can_bus_only_output,
data_file=data_file,
data_files=data_files_transposed,
auto_zero_sensor=auto_zero_sensor,
num_baseline_reads=num_baseline_reads,
sensor_id=sensor_id_for_instrument(probe),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ async def liquid_probe(
plunger_speed: float,
threshold_pascals: float,
output_format: OutputOptions = OutputOptions.can_bus_only,
data_file: Optional[str] = None,
data_files: Optional[Dict[InstrumentProbeType, str]] = None,
auto_zero_sensor: bool = True,
num_baseline_reads: int = 10,
probe: InstrumentProbeType = InstrumentProbeType.PRIMARY,
Expand Down
1 change: 1 addition & 0 deletions api/src/opentrons/hardware_control/backends/ot3utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,7 @@ def sensor_node_for_pipette(mount: OT3Mount) -> PipetteProbeTarget:
_instr_sensor_id_lookup: Dict[InstrumentProbeType, SensorId] = {
InstrumentProbeType.PRIMARY: SensorId.S0,
InstrumentProbeType.SECONDARY: SensorId.S1,
InstrumentProbeType.BOTH: SensorId.BOTH,
}


Expand Down
2 changes: 1 addition & 1 deletion api/src/opentrons/hardware_control/ot3api.py
Original file line number Diff line number Diff line change
Expand Up @@ -2623,7 +2623,7 @@ async def liquid_probe(
(probe_settings.plunger_speed * plunger_direction),
probe_settings.sensor_threshold_pascals,
probe_settings.output_option,
probe_settings.data_file,
probe_settings.data_files,
probe_settings.auto_zero_sensor,
probe_settings.num_baseline_reads,
probe=probe if probe else InstrumentProbeType.PRIMARY,
Expand Down
1 change: 1 addition & 0 deletions api/src/opentrons/hardware_control/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -624,6 +624,7 @@ class GripperJawState(enum.Enum):
class InstrumentProbeType(enum.Enum):
PRIMARY = enum.auto()
SECONDARY = enum.auto()
BOTH = enum.auto()


class GripperProbe(enum.Enum):
Expand Down
2 changes: 1 addition & 1 deletion api/tests/opentrons/config/ot3_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@
"aspirate_while_sensing": False,
"auto_zero_sensor": True,
"num_baseline_reads": 10,
"data_file": "/var/pressure_sensor_data.csv",
"data_files": {"PRIMARY": "/data/pressure_sensor_data.csv"},
},
"calibration": {
"z_offset": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
UpdateState,
EstopState,
CurrentConfig,
InstrumentProbeType,
)
from opentrons.hardware_control.errors import (
InvalidPipetteName,
Expand Down Expand Up @@ -185,7 +186,7 @@ def fake_liquid_settings() -> LiquidProbeSettings:
aspirate_while_sensing=False,
auto_zero_sensor=False,
num_baseline_reads=8,
data_file="fake_data_file",
data_files={InstrumentProbeType.PRIMARY: "fake_file_name"},
)


Expand Down
6 changes: 3 additions & 3 deletions api/tests/opentrons/hardware_control/test_ot3_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ def fake_liquid_settings() -> LiquidProbeSettings:
aspirate_while_sensing=False,
auto_zero_sensor=False,
num_baseline_reads=10,
data_file="fake_file_name",
data_files={InstrumentProbeType.PRIMARY: "fake_file_name"},
)


Expand Down Expand Up @@ -809,7 +809,7 @@ async def test_liquid_probe(
aspirate_while_sensing=True,
auto_zero_sensor=False,
num_baseline_reads=10,
data_file="fake_file_name",
data_files={InstrumentProbeType.PRIMARY: "fake_file_name"},
)
await ot3_hardware.liquid_probe(mount, fake_settings_aspirate)
mock_move_to_plunger_bottom.assert_called_once()
Expand All @@ -820,7 +820,7 @@ async def test_liquid_probe(
(fake_settings_aspirate.plunger_speed * -1),
fake_settings_aspirate.sensor_threshold_pascals,
fake_settings_aspirate.output_option,
fake_settings_aspirate.data_file,
fake_settings_aspirate.data_files,
fake_settings_aspirate.auto_zero_sensor,
fake_settings_aspirate.num_baseline_reads,
probe=InstrumentProbeType.PRIMARY,
Expand Down
8 changes: 5 additions & 3 deletions hardware-testing/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -257,9 +257,11 @@ scp $(ssh_helper_ot3) $(4) root@$(1):/tmp/
ssh $(ssh_helper_ot3) root@$(1) \
"function cleanup () { (rm -rf /tmp/$(4) || true) && mount -o remount,ro / ; } ;\
mount -o remount,rw / &&\
(unzip -o /tmp/$(4) -d /usr/lib/firmware || cleanup) &&\
(unzip -o /tmp/$(5) -d /usr/lib/firmware || cleanup) &&\
python3 -m json.tool /usr/lib/firmware/opentrons-firmware.json &&\
cleanup"
cleanup &&\
echo "Restarting robot server" &&\
systemctl restart opentrons-robot-server"
endef

.PHONY: sync-sw-ot3
Expand All @@ -284,7 +286,7 @@ remove-patches-fixture:

.PHONY: sync-fw-ot3
sync-fw-ot3:
$(call push-and-update-fw,$(host),$(ssh_key),$(ssh_opts),$(zip))
$(call push-and-update-fw,$(host),$(ssh_key),$(ssh_opts),$(zip),$(notdir $(zip)))

.PHONY: sync-ot3
sync-ot3: sync-sw-ot3 sync-fw-ot3
Expand Down
3 changes: 2 additions & 1 deletion hardware-testing/hardware_testing/gravimetric/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from enum import Enum
from opentrons.config.types import LiquidProbeSettings, OutputOptions
from opentrons.protocol_api.labware import Well
from opentrons.hardware_control.types import InstrumentProbeType


class ConfigType(Enum):
Expand Down Expand Up @@ -197,7 +198,7 @@ def _get_liquid_probe_settings(
aspirate_while_sensing=False,
auto_zero_sensor=True,
num_baseline_reads=10,
data_file="/data/testing_data/pressure.csv",
data_files={InstrumentProbeType.PRIMARY: "/data/testing_data/pressure.csv"},
)


Expand Down
4 changes: 3 additions & 1 deletion hardware-testing/hardware_testing/liquid_sense/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ def build_run_args(cls, args: argparse.Namespace) -> "RunArgs":

args = parser.parse_args()
run_args = RunArgs.build_run_args(args)
exit_error = os.EX_OK
try:
if not run_args.ctx.is_simulating():
data_dir = get_testing_data_directory()
Expand All @@ -292,6 +293,7 @@ def build_run_args(cls, args: argparse.Namespace) -> "RunArgs":
except Exception as e:
ui.print_info(f"got error {e}")
ui.print_info(traceback.format_exc())
exit_error = 1
finally:
if run_args.recorder is not None:
ui.print_info("ending recording")
Expand All @@ -314,4 +316,4 @@ def build_run_args(cls, args: argparse.Namespace) -> "RunArgs":
run_args.ctx.cleanup()
if not args.simulate:
helpers_ot3.restart_server_ot3()
os._exit(os.EX_OK)
os._exit(exit_error)
26 changes: 18 additions & 8 deletions hardware-testing/hardware_testing/liquid_sense/execute.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,14 +177,15 @@ def run(tip: int, run_args: RunArgs) -> None:
run_args.pipette._retract()

def _get_baseline() -> float:
run_args.pipette.pick_up_tip(tips.pop(0))
run_args.pipette.pick_up_tip(tips[0])
del tips[: run_args.pipette_channels]
liquid_height = _jog_to_find_liquid_height(
run_args.ctx, run_args.pipette, test_well
)
target_height = test_well.bottom(liquid_height).point.z

run_args.pipette._retract()
# tip_offset = 0.0
tip_offset = 0.0
if run_args.dial_indicator is not None:
run_args.pipette.move_to(dial_well.top())
tip_offset = run_args.dial_indicator.read_stable()
Expand Down Expand Up @@ -214,7 +215,8 @@ def _get_baseline() -> float:
tip_offset = _get_baseline()

ui.print_info(f"Picking up {tip}ul tip")
run_args.pipette.pick_up_tip(tips.pop(0))
run_args.pipette.pick_up_tip(tips[0])
del tips[: run_args.pipette_channels]
run_args.pipette.move_to(test_well.top())

start_pos = hw_api.current_position_ot3(OT3Mount.LEFT)
Expand Down Expand Up @@ -274,9 +276,17 @@ def _run_trial(run_args: RunArgs, tip: int, well: Well, trial: int) -> float:
run_args.pipette_channels
][tip]
data_dir = get_testing_data_directory()
data_filename = f"pressure_sensor_data-trial{trial}-tip{tip}.csv"
data_file = f"{data_dir}/{run_args.name}/{run_args.run_id}/{data_filename}"
ui.print_info(f"logging pressure data to {data_file}")
probes: List[InstrumentProbeType] = [InstrumentProbeType.PRIMARY]
probe_target: InstrumentProbeType = InstrumentProbeType.PRIMARY
if run_args.pipette_channels > 1:
probes.append(InstrumentProbeType.SECONDARY)
probe_target = InstrumentProbeType.BOTH
data_files: Dict[InstrumentProbeType, str] = {}
for probe in probes:
data_filename = f"pressure_sensor_data-trial{trial}-tip{tip}-{probe.name}.csv"
data_file = f"{data_dir}/{run_args.name}/{run_args.run_id}/{data_filename}"
ui.print_info(f"logging pressure data to {data_file}")
data_files[probe] = data_file

plunger_speed = (
lqid_cfg["plunger_speed"]
Expand All @@ -295,13 +305,13 @@ def _run_trial(run_args: RunArgs, tip: int, well: Well, trial: int) -> float:
aspirate_while_sensing=run_args.aspirate,
auto_zero_sensor=True,
num_baseline_reads=10,
data_file=data_file,
data_files=data_files,
)

hw_mount = OT3Mount.LEFT if run_args.pipette.mount == "left" else OT3Mount.RIGHT
run_args.recorder.set_sample_tag(f"trial-{trial}-{tip}ul")
# TODO add in stuff for secondary probe
height = hw_api.liquid_probe(hw_mount, lps, InstrumentProbeType.PRIMARY)
height = hw_api.liquid_probe(hw_mount, lps, probe_target)
ui.print_info(f"Trial {trial} complete")
run_args.recorder.clear_sample_tag()
return height
Original file line number Diff line number Diff line change
Expand Up @@ -1386,7 +1386,7 @@ async def _test_liquid_probe(
aspirate_while_sensing=False, # FIXME: I heard this doesn't work
auto_zero_sensor=True, # TODO: when would we want to adjust this?
num_baseline_reads=10, # TODO: when would we want to adjust this?
data_file="", # FIXME: remove
data_files=None,
)
end_z = await api.liquid_probe(mount, probe_settings, probe=probe)
if probe == InstrumentProbeType.PRIMARY:
Expand Down
2 changes: 2 additions & 0 deletions hardware/opentrons_hardware/firmware_bindings/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,8 @@ class SensorId(int, Enum):

S0 = 0x0
S1 = 0x1
UNUSED = 0x2
BOTH = 0x3


@unique
Expand Down
Loading

0 comments on commit b3b65df

Please sign in to comment.