Skip to content

Commit

Permalink
Merge branch 'edge' into second_probe_command
Browse files Browse the repository at this point in the history
Resolve conflicts in:
* api/src/opentrons/protocol_engine/commands/liquid_probe.py
* api/tests/opentrons/protocol_engine/commands/test_liquid_probe.py
  • Loading branch information
SyntaxColoring committed Jul 16, 2024
2 parents 2ff9417 + 0ba6c79 commit 3de025e
Show file tree
Hide file tree
Showing 58 changed files with 1,009 additions and 378 deletions.
1 change: 1 addition & 0 deletions .github/workflows/app-test-build-deploy.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ jobs:
needs: [determine-build-type]
if: needs.determine-build-type.outputs.variants != '[]'
strategy:
fail-fast: false
matrix:
os: ['windows-2022', 'ubuntu-22.04', 'macos-latest']
variant: ${{fromJSON(needs.determine-build-type.outputs.variants)}}
Expand Down
2 changes: 1 addition & 1 deletion abr-testing/abr_testing/data_collection/abr_robot_error.py
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,7 @@ def get_run_error_info_from_robot(
email = args.email[0]
board_id = args.board_id[0]
reporter_id = args.reporter_id[0]
file_paths = read_robot_logs.get_logs(storage_directory, ip)
ticket = jira_tool.JiraTicket(url, api_token, email)
ticket.issues_on_board(board_id)
users_file_path = ticket.get_jira_users(storage_directory)
Expand Down Expand Up @@ -496,7 +497,6 @@ def get_run_error_info_from_robot(
saved_file_path_calibration, calibration = read_robot_logs.get_calibration_offsets(
ip, storage_directory
)
file_paths = read_robot_logs.get_logs(storage_directory, ip)

print(f"Making ticket for {summary}.")
# TODO: make argument or see if I can get rid of with using board_id.
Expand Down
21 changes: 15 additions & 6 deletions abr-testing/abr_testing/data_collection/read_robot_logs.py
Original file line number Diff line number Diff line change
Expand Up @@ -569,23 +569,32 @@ def get_calibration_offsets(

def get_logs(storage_directory: str, ip: str) -> List[str]:
"""Get Robot logs."""
log_types = ["api.log", "server.log", "serial.log", "touchscreen.log"]
log_types: List[Dict[str, Any]] = [
{"log type": "api.log", "records": 1000},
{"log type": "server.log", "records": 10000},
{"log type": "serial.log", "records": 10000},
{"log type": "touchscreen.log", "records": 1000},
]
all_paths = []
for log_type in log_types:
try:
log_type_name = log_type["log type"]
print(log_type_name)
log_records = int(log_type["records"])
print(log_records)
response = requests.get(
f"http://{ip}:31950/logs/{log_type}",
headers={"log_identifier": log_type},
params={"records": 5000},
f"http://{ip}:31950/logs/{log_type_name}",
headers={"log_identifier": log_type_name},
params={"records": log_records},
)
response.raise_for_status()
log_data = response.text
log_name = ip + "_" + log_type.split(".")[0] + ".log"
log_name = ip + "_" + log_type_name.split(".")[0] + ".log"
file_path = os.path.join(storage_directory, log_name)
with open(file_path, mode="w", encoding="utf-8") as file:
file.write(log_data)
except RuntimeError:
print(f"Request exception. Did not save {log_type}")
print(f"Request exception. Did not save {log_type_name}")
continue
all_paths.append(file_path)
# Get weston.log using scp
Expand Down
6 changes: 3 additions & 3 deletions api/src/opentrons/config/defaults_ot3.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@
DEFAULT_MODULE_OFFSET = [0.0, 0.0, 0.0]

DEFAULT_LIQUID_PROBE_SETTINGS: Final[LiquidProbeSettings] = LiquidProbeSettings(
mount_speed=10,
plunger_speed=5,
mount_speed=5,
plunger_speed=20,
plunger_impulse_time=0.2,
sensor_threshold_pascals=15,
output_option=OutputOptions.sync_buffer_to_csv,
Expand Down Expand Up @@ -328,7 +328,7 @@ def _build_default_liquid_probe(
or output_option is OutputOptions.stream_to_csv
):
data_files = _build_log_files_with_default(
from_conf.get("data_files", {}), default.data_files
from_conf.get("data_files", None), default.data_files
)
return LiquidProbeSettings(
mount_speed=from_conf.get("mount_speed", default.mount_speed),
Expand Down
12 changes: 0 additions & 12 deletions api/src/opentrons/hardware_control/backends/ot3controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,6 @@
PipetteLiquidNotFoundError,
CommunicationError,
PythonException,
UnsupportedHardwareCommand,
)

from .subsystem_manager import SubsystemManager
Expand Down Expand Up @@ -1363,17 +1362,6 @@ async def liquid_probe(
probe: InstrumentProbeType = InstrumentProbeType.PRIMARY,
force_both_sensors: bool = False,
) -> float:
if output_option == OutputOptions.sync_buffer_to_csv:
if (
self._subsystem_manager.device_info[
SubSystem.of_mount(mount)
].revision.tertiary
!= "1"
):
raise UnsupportedHardwareCommand(
"Liquid Probe not supported on this pipette firmware"
)

head_node = axis_to_node(Axis.by_mount(mount))
tool = sensor_node_for_pipette(OT3Mount(mount.value))
csv_output = bool(output_option.value & OutputOptions.stream_to_csv.value)
Expand Down
11 changes: 7 additions & 4 deletions api/src/opentrons/hardware_control/ot3api.py
Original file line number Diff line number Diff line change
Expand Up @@ -2636,10 +2636,13 @@ async def liquid_probe(
pos = await self.gantry_position(checked_mount, refresh=True)
while (probe_start_pos.z - pos.z) < max_z_dist:
# safe distance so we don't accidentally aspirate liquid if we're already close to liquid
safe_plunger_pos = pos._replace(z=(pos.z + 2))
safe_plunger_pos = top_types.Point(pos.x, pos.y, pos.z + 2)
# overlap amount we want to use between passes
pass_start_pos = pos._replace(z=(pos.z + 0.5))

pass_start_pos = top_types.Point(pos.x, pos.y, pos.z + 0.5)
max_z_time = (
max_z_dist - (probe_start_pos.z - safe_plunger_pos.z)
) / probe_settings.mount_speed
pass_travel = min(max_z_time * probe_settings.plunger_speed, p_travel)
# Prep the plunger
await self.move_to(checked_mount, safe_plunger_pos)
if probe_settings.aspirate_while_sensing:
Expand All @@ -2655,7 +2658,7 @@ async def liquid_probe(
checked_mount,
probe_settings,
probe if probe else InstrumentProbeType.PRIMARY,
p_travel,
pass_travel,
)
# if we made it here without an error we found the liquid
error = None
Expand Down
12 changes: 8 additions & 4 deletions api/src/opentrons/protocol_api/core/engine/instrument.py
Original file line number Diff line number Diff line change
Expand Up @@ -838,13 +838,12 @@ def retract(self) -> None:
z_axis = self._engine_client.state.pipettes.get_z_axis(self._pipette_id)
self._engine_client.execute_command(cmd.HomeParams(axes=[z_axis]))

def liquid_probe_with_recovery(self, well_core: WellCore) -> None:
def liquid_probe_with_recovery(self, well_core: WellCore, loc: Location) -> None:
labware_id = well_core.labware_id
well_name = well_core.get_name()
well_location = WellLocation(
origin=WellOrigin.TOP, offset=WellOffset(x=0, y=0, z=0)
)

self._engine_client.execute_command(
cmd.LiquidProbeParams(
labwareId=labware_id,
Expand All @@ -854,13 +853,16 @@ def liquid_probe_with_recovery(self, well_core: WellCore) -> None:
)
)

def liquid_probe_without_recovery(self, well_core: WellCore) -> float:
self._protocol_core.set_last_location(location=loc, mount=self.get_mount())

def liquid_probe_without_recovery(
self, well_core: WellCore, loc: Location
) -> float:
labware_id = well_core.labware_id
well_name = well_core.get_name()
well_location = WellLocation(
origin=WellOrigin.TOP, offset=WellOffset(x=0, y=0, z=0)
)

result = self._engine_client.execute_command_without_recovery(
cmd.LiquidProbeParams(
labwareId=labware_id,
Expand All @@ -870,5 +872,7 @@ def liquid_probe_without_recovery(self, well_core: WellCore) -> float:
)
)

self._protocol_core.set_last_location(location=loc, mount=self.get_mount())

if result is not None and isinstance(result, LiquidProbeResult):
return result.z_position
8 changes: 6 additions & 2 deletions api/src/opentrons/protocol_api/core/instrument.py
Original file line number Diff line number Diff line change
Expand Up @@ -306,12 +306,16 @@ def retract(self) -> None:
...

@abstractmethod
def liquid_probe_with_recovery(self, well_core: WellCoreType) -> None:
def liquid_probe_with_recovery(
self, well_core: WellCoreType, loc: types.Location
) -> None:
"""Do a liquid probe to detect the presence of liquid in the well."""
...

@abstractmethod
def liquid_probe_without_recovery(self, well_core: WellCoreType) -> float:
def liquid_probe_without_recovery(
self, well_core: WellCoreType, loc: types.Location
) -> float:
"""Do a liquid probe to find the level of the liquid in the well."""
...

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -566,10 +566,14 @@ 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]

def liquid_probe_with_recovery(self, well_core: WellCore) -> None:
def liquid_probe_with_recovery(
self, well_core: WellCore, loc: types.Location
) -> None:
"""This will never be called because it was added in API 2.20."""
assert False, "liquid_probe_with_recovery only supported in API 2.20 & later"

def liquid_probe_without_recovery(self, well_core: WellCore) -> float:
def liquid_probe_without_recovery(
self, well_core: WellCore, loc: types.Location
) -> float:
"""This will never be called because it was added in API 2.20."""
assert False, "liquid_probe_without_recovery only supported in API 2.20 & later"
Original file line number Diff line number Diff line change
Expand Up @@ -484,10 +484,14 @@ 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]

def liquid_probe_with_recovery(self, well_core: WellCore) -> None:
def liquid_probe_with_recovery(
self, well_core: WellCore, loc: types.Location
) -> None:
"""This will never be called because it was added in API 2.20."""
assert False, "liquid_probe_with_recovery only supported in API 2.20 & later"

def liquid_probe_without_recovery(self, well_core: WellCore) -> float:
def liquid_probe_without_recovery(
self, well_core: WellCore, loc: types.Location
) -> float:
"""This will never be called because it was added in API 2.20."""
assert False, "liquid_probe_without_recovery only supported in API 2.20 & later"
28 changes: 15 additions & 13 deletions api/src/opentrons/protocol_api/instrument_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
CommandParameterLimitViolated,
UnexpectedTipRemovalError,
)
from opentrons.protocol_engine.errors.exceptions import WellDoesNotExistError
from opentrons.legacy_broker import LegacyBroker
from opentrons.hardware_control.dev_types import PipetteDict
from opentrons import types
Expand Down Expand Up @@ -226,7 +225,6 @@ def aspirate(

well: Optional[labware.Well] = None
move_to_location: types.Location

last_location = self._get_last_location_by_api_version()
try:
target = validation.validate_location(
Expand Down Expand Up @@ -264,6 +262,14 @@ def aspirate(
c_vol = self._core.get_available_volume() if not volume else volume
flow_rate = self._core.get_aspirate_flow_rate(rate)

if (
self.api_version >= APIVersion(2, 20)
and well is not None
and self.liquid_presence_detection
):
self.require_liquid_presence(well=well)
self.prepare_to_aspirate()

with publisher.publish_context(
broker=self.broker,
command=cmds.aspirate(
Expand Down Expand Up @@ -2105,11 +2111,11 @@ def detect_liquid_presence(self, well: labware.Well) -> bool:
:returns: A boolean.
"""
if not isinstance(well, labware.Well):
raise WellDoesNotExistError("You must provide a valid well to check.")
loc = well.top()
try:
self._core.liquid_probe_without_recovery(well._core)
self._core.liquid_probe_without_recovery(well._core, loc)
except ProtocolCommandFailedError as e:
# if we handle the error, we change the protocl state from error to valid
if isinstance(e.original_error, LiquidNotFoundError):
return False
raise e
Expand All @@ -2122,10 +2128,8 @@ def require_liquid_presence(self, well: labware.Well) -> None:
:returns: None.
"""
if not isinstance(well, labware.Well):
raise WellDoesNotExistError("You must provide a valid well to check.")

self._core.liquid_probe_with_recovery(well._core)
loc = well.top()
self._core.liquid_probe_with_recovery(well._core, loc)

@requires_version(2, 20)
def measure_liquid_height(self, well: labware.Well) -> float:
Expand All @@ -2137,8 +2141,6 @@ def measure_liquid_height(self, well: labware.Well) -> float:
This is intended for Opentrons internal use only and is not a guaranteed API.
"""
if not isinstance(well, labware.Well):
raise WellDoesNotExistError("You must provide a valid well to check.")

height = self._core.liquid_probe_without_recovery(well._core)
loc = well.top()
height = self._core.liquid_probe_without_recovery(well._core, loc)
return height
11 changes: 3 additions & 8 deletions api/src/opentrons/protocol_engine/commands/liquid_probe.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

from pydantic import Field

from ..types import CurrentWell, DeckPoint
from ..types import DeckPoint
from .pipetting_common import (
LiquidNotFoundError,
LiquidNotFoundErrorInternalData,
Expand Down Expand Up @@ -97,6 +97,8 @@ async def execute(self, params: LiquidProbeParams) -> _LiquidProbeExecuteReturn:
Raises:
TipNotAttachedError: as an undefined error, if there is not tip attached to
the pipette.
TipNotEmptyError: as an undefined error, if the tip starts with liquid
in it.
MustHomeError: as an undefined error, if the plunger is not in a valid
position.
"""
Expand All @@ -118,19 +120,12 @@ async def execute(self, params: LiquidProbeParams) -> _LiquidProbeExecuteReturn:
message="Current position of pipette is invalid. Please home."
)

current_well = CurrentWell(
pipette_id=pipette_id,
labware_id=labware_id,
well_name=well_name,
)

# liquid_probe process start position
position = await self._movement.move_to_well(
pipette_id=pipette_id,
labware_id=labware_id,
well_name=well_name,
well_location=params.wellLocation,
current_well=current_well,
)

try:
Expand Down
10 changes: 5 additions & 5 deletions api/src/opentrons/protocol_engine/execution/pipetting.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class PipettingHandler(TypingProtocol):
"""Liquid handling commands."""

def get_is_empty(self, pipette_id: str) -> bool:
"""Get whether a pipette has a working volume equal to 0."""
"""Get whether a pipette has an aspirated volume equal to 0."""

def get_is_ready_to_aspirate(self, pipette_id: str) -> bool:
"""Get whether a pipette is ready to aspirate."""
Expand Down Expand Up @@ -81,8 +81,8 @@ def __init__(self, state_view: StateView, hardware_api: HardwareControlAPI) -> N
self._hardware_api = hardware_api

def get_is_empty(self, pipette_id: str) -> bool:
"""Get whether a pipette has a working volume equal to 0."""
return self._state_view.pipettes.get_working_volume(pipette_id) == 0
"""Get whether a pipette has an aspirated volume equal to 0."""
return self._state_view.pipettes.get_aspirated_volume(pipette_id) == 0

def get_is_ready_to_aspirate(self, pipette_id: str) -> bool:
"""Get whether a pipette is ready to aspirate."""
Expand Down Expand Up @@ -234,8 +234,8 @@ def __init__(
self._state_view = state_view

def get_is_empty(self, pipette_id: str) -> bool:
"""Get whether a pipette has a working volume equal to 0."""
return self._state_view.pipettes.get_working_volume(pipette_id) == 0
"""Get whether a pipette has an aspirated volume equal to 0."""
return self._state_view.pipettes.get_aspirated_volume(pipette_id) == 0

def get_is_ready_to_aspirate(self, pipette_id: str) -> bool:
"""Get whether a pipette is ready to aspirate."""
Expand Down
2 changes: 1 addition & 1 deletion api/src/opentrons/protocol_engine/state/pipettes.py
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,7 @@ def get_current_tip_lld_settings(self, pipette_id: str) -> float:
if attached_tip is None or attached_tip.volume is None:
return 0
lld_settings = self.get_pipette_lld_settings(pipette_id)
tipVolume = str(attached_tip.volume)
tipVolume = "t" + str(int(attached_tip.volume))
if (
lld_settings is None
or lld_settings[tipVolume] is None
Expand Down
5 changes: 1 addition & 4 deletions api/tests/opentrons/hardware_control/test_ot3_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -806,9 +806,6 @@ async def test_liquid_probe(
)
await ot3_hardware.cache_pipette(mount, instr_data, None)
pipette = ot3_hardware.hardware_pipettes[mount.to_mount()]
plunger_positions = ot3_hardware._pipette_handler.get_pipette(
mount
).plunger_positions

assert pipette
await ot3_hardware.add_tip(mount, 100)
Expand Down Expand Up @@ -841,7 +838,7 @@ async def test_liquid_probe(
mock_move_to_plunger_bottom.assert_called_once()
mock_liquid_probe.assert_called_once_with(
mount,
plunger_positions.bottom - plunger_positions.top,
3.0,
fake_settings_aspirate.mount_speed,
(fake_settings_aspirate.plunger_speed * -1),
fake_settings_aspirate.sensor_threshold_pascals,
Expand Down
Loading

0 comments on commit 3de025e

Please sign in to comment.