feat(hardware): refactor tool_sensors to simplify and remove csvs (#1…

…6462)

<!--
Thanks for taking the time to open a Pull Request (PR)! 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

GitHub provides robust markdown to format your PR. Links, diagrams,
pictures, and videos along with text formatting make it possible to
create a rich and informative PR. For more information on GitHub
markdown, see:


https://docs.github.com/en/get-started/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax

To ensure your code is reviewed quickly and thoroughly, please fill out
the sections below to the best of your ability!
-->

# Overview
This PR removes much of the complexity in tool_sensors around the liquid
probe routine.
Instead of having many different options of setting the bindings and
building messages, this consolidates all of the previous options by only
commanding the firmware one way, forwarding the sensor information to a
new log file, and providing a way for the system to optionally grab the
raw sensor data if they wish which is required for the hardware-testing
scripts and possibly for future features.

We no longer write CSV files to the robot through this system and have
removed all of the coded in paths for them. It also removes the
"OutputOptions" data type that previously controlled the behavior
variants.

Before each variant had it's own quirks to behavior so now machine
actions will always be the same regardless of how the tool_sensors
method is called.

This also adds a new can message that sends sensor data in batches
instead of 1 by 1 so we don't choke up the can bus as much.

<!--
Describe your PR at a high level. State acceptance criteria and how this
PR fits into other work. Link issues, PRs, and other relevant resources.
-->

## Test Plan and Hands on Testing

<!--
Describe your testing of the PR. Emphasize testing not reflected in the
code. Attach protocols, logs, screenshots and any other assets that
support your testing.
-->
## Changelog

<!--
List changes introduced by this PR considering future developers and the
end user. Give careful thought and clear documentation to breaking
changes.
-->

## Review requests

<!--
- What do you need from reviewers to feel confident this PR is ready to
merge?
- Ask questions.
-->

## Risk assessment

<!--
- Indicate the level of attention this PR needs.
- Provide context to guide reviewers.
- Discuss trade-offs, coupling, and side effects.
- 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 may also change the
behavior of labware calibration.
- How do your unit tests and on hands on testing mitigate this PR's
risks and the risk of future regressions?
- Especially in high risk PRs, explain how you know your testing is
enough.
-->

api/src/opentrons/config/__init__.py

@@ -202,6 +202,15 @@ class ConfigElement(NamedTuple):
         " absolute path, it will be used directly. If it is a "
         "relative path it will be relative to log_dir",
     ),
+    ConfigElement(
+        "sensor_log_file",
+        "Sensor Log File",
+        Path("logs") / "sensor.log",
+        ConfigElementType.FILE,
+        "The location of the file to save sensor logs to. If this is an"
+        " absolute path, it will be used directly. If it is a "
+        "relative path it will be relative to log_dir",
+    ),
     ConfigElement(
         "serial_log_file",
         "Serial Log File",

api/src/opentrons/config/defaults_ot3.py

@@ -15,7 +15,6 @@
     LiquidProbeSettings,
     ZSenseSettings,
     EdgeSenseSettings,
-    OutputOptions,
 )
 
 
@@ -27,13 +26,11 @@
     plunger_speed=15,
     plunger_impulse_time=0.2,
     sensor_threshold_pascals=15,
-    output_option=OutputOptions.sync_buffer_to_csv,
     aspirate_while_sensing=False,
     z_overlap_between_passes_mm=0.1,
     plunger_reset_offset=2.0,
     samples_for_baselining=20,
     sample_time_sec=0.004,
-    data_files={InstrumentProbeType.PRIMARY: "/data/pressure_sensor_data.csv"},
 )
 
 DEFAULT_CALIBRATION_SETTINGS: Final[OT3CalibrationSettings] = OT3CalibrationSettings(
@@ -43,7 +40,6 @@
             max_overrun_distance_mm=5.0,
             speed_mm_per_s=1.0,
             sensor_threshold_pf=3.0,
-            output_option=OutputOptions.sync_only,
         ),
     ),
     edge_sense=EdgeSenseSettings(
@@ -54,7 +50,6 @@
             max_overrun_distance_mm=0.5,
             speed_mm_per_s=1,
             sensor_threshold_pf=3.0,
-            output_option=OutputOptions.sync_only,
         ),
         search_initial_tolerance_mm=12.0,
         search_iteration_limit=8,
@@ -195,23 +190,6 @@
 )
 
 
-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]],
@@ -316,24 +294,12 @@ def _build_default_cap_pass(
         sensor_threshold_pf=from_conf.get(
             "sensor_threshold_pf", default.sensor_threshold_pf
         ),
-        output_option=from_conf.get("output_option", default.output_option),
     )
 
 
 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", None), default.data_files
-        )
     return LiquidProbeSettings(
         mount_speed=from_conf.get("mount_speed", default.mount_speed),
         plunger_speed=from_conf.get("plunger_speed", default.plunger_speed),
@@ -343,7 +309,6 @@ def _build_default_liquid_probe(
         sensor_threshold_pascals=from_conf.get(
             "sensor_threshold_pascals", default.sensor_threshold_pascals
         ),
-        output_option=from_conf.get("output_option", default.output_option),
         aspirate_while_sensing=from_conf.get(
             "aspirate_while_sensing", default.aspirate_while_sensing
         ),
@@ -357,7 +322,6 @@ def _build_default_liquid_probe(
             "samples_for_baselining", default.samples_for_baselining
         ),
         sample_time_sec=from_conf.get("sample_time_sec", default.sample_time_sec),
-        data_files=data_files,
     )
 
 

api/src/opentrons/config/types.py

@@ -1,8 +1,8 @@
 from enum import Enum
 from dataclasses import dataclass, asdict, fields
-from typing import Dict, Tuple, TypeVar, Generic, List, cast, Optional
+from typing import Dict, Tuple, TypeVar, Generic, List, cast
 from typing_extensions import TypedDict, Literal
-from opentrons.hardware_control.types import OT3AxisKind, InstrumentProbeType
+from opentrons.hardware_control.types import OT3AxisKind
 
 
 class AxisDict(TypedDict):
@@ -103,25 +103,12 @@ def by_gantry_load(
         )
 
 
-class OutputOptions(int, Enum):
-    """Specifies where we should report sensor data to during a sensor pass."""
-
-    stream_to_csv = 0x1  # compile sensor data stream into a csv file, in addition to can_bus_only behavior
-    sync_buffer_to_csv = 0x2  # collect sensor data on pipette mcu, then stream to robot server and compile into a csv file, in addition to can_bus_only behavior
-    can_bus_only = (
-        0x4  # stream sensor data over CAN bus, in addition to sync_only behavior
-    )
-    sync_only = 0x8  # trigger pipette sync line upon sensor's detection of something
-
-
 @dataclass(frozen=True)
 class CapacitivePassSettings:
     prep_distance_mm: float
     max_overrun_distance_mm: float
     speed_mm_per_s: float
     sensor_threshold_pf: float
-    output_option: OutputOptions
-    data_files: Optional[Dict[InstrumentProbeType, str]] = None
 
 
 @dataclass(frozen=True)
@@ -135,13 +122,11 @@ class LiquidProbeSettings:
     plunger_speed: float
     plunger_impulse_time: float
     sensor_threshold_pascals: float
-    output_option: OutputOptions
     aspirate_while_sensing: bool
     z_overlap_between_passes_mm: float
     plunger_reset_offset: float
     samples_for_baselining: int
     sample_time_sec: float
-    data_files: Optional[Dict[InstrumentProbeType, str]]
 
 
 @dataclass(frozen=True)

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

@@ -15,7 +15,7 @@
 from opentrons_shared_data.pipette.types import (
     PipetteName,
 )
-from opentrons.config.types import GantryLoad, OutputOptions
+from opentrons.config.types import GantryLoad
 from opentrons.hardware_control.types import (
     BoardRevision,
     Axis,
@@ -38,6 +38,8 @@
     StatusBarState,
 )
 from opentrons.hardware_control.module_control import AttachedModulesControl
+from opentrons_hardware.firmware_bindings.constants import SensorId
+from opentrons_hardware.sensors.types import SensorDataType
 from ..dev_types import OT3AttachedInstruments
 from .types import HWStopCondition
 
@@ -152,10 +154,11 @@ async def liquid_probe(
         threshold_pascals: float,
         plunger_impulse_time: float,
         num_baseline_reads: int,
-        output_format: OutputOptions = OutputOptions.can_bus_only,
-        data_files: Optional[Dict[InstrumentProbeType, str]] = None,
         probe: InstrumentProbeType = InstrumentProbeType.PRIMARY,
         force_both_sensors: bool = False,
+        response_queue: Optional[
+            asyncio.Queue[Dict[SensorId, List[SensorDataType]]]
+        ] = None,
     ) -> float:
         ...
 
@@ -371,8 +374,6 @@ async def capacitive_probe(
         speed_mm_per_s: float,
         sensor_threshold_pf: float,
         probe: InstrumentProbeType = InstrumentProbeType.PRIMARY,
-        output_format: OutputOptions = OutputOptions.sync_only,
-        data_files: Optional[Dict[InstrumentProbeType, str]] = None,
     ) -> bool:
         ...
 

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

@@ -25,7 +25,7 @@
     Union,
     Mapping,
 )
-from opentrons.config.types import OT3Config, GantryLoad, OutputOptions
+from opentrons.config.types import OT3Config, GantryLoad
 from opentrons.config import gripper_config
 from .ot3utils import (
     axis_convert,
@@ -102,7 +102,9 @@
     NodeId,
     PipetteName as FirmwarePipetteName,
     ErrorCode,
+    SensorId,
 )
+from opentrons_hardware.sensors.types import SensorDataType
 from opentrons_hardware.firmware_bindings.messages.message_definitions import (
     StopRequest,
 )
@@ -1368,28 +1370,14 @@ async def liquid_probe(
         threshold_pascals: float,
         plunger_impulse_time: float,
         num_baseline_reads: int,
-        output_option: OutputOptions = OutputOptions.can_bus_only,
-        data_files: Optional[Dict[InstrumentProbeType, str]] = None,
         probe: InstrumentProbeType = InstrumentProbeType.PRIMARY,
         force_both_sensors: bool = False,
+        response_queue: Optional[
+            asyncio.Queue[Dict[SensorId, List[SensorDataType]]]
+        ] = None,
     ) -> float:
         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)
-        sync_buffer_output = bool(
-            output_option.value & OutputOptions.sync_buffer_to_csv.value
-        )
-        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,
@@ -1400,12 +1388,9 @@ async def liquid_probe(
             threshold_pascals=threshold_pascals,
             plunger_impulse_time=plunger_impulse_time,
             num_baseline_reads=num_baseline_reads,
-            csv_output=csv_output,
-            sync_buffer_output=sync_buffer_output,
-            can_bus_only_output=can_bus_only_output,
-            data_files=data_files_transposed,
             sensor_id=sensor_id_for_instrument(probe),
             force_both_sensors=force_both_sensors,
+            response_queue=response_queue,
         )
         for node, point in positions.items():
             self._position.update({node: point.motor_position})
@@ -1432,41 +1417,13 @@ async def capacitive_probe(
         speed_mm_per_s: float,
         sensor_threshold_pf: float,
         probe: InstrumentProbeType = InstrumentProbeType.PRIMARY,
-        output_option: OutputOptions = OutputOptions.sync_only,
-        data_files: Optional[Dict[InstrumentProbeType, str]] = None,
     ) -> bool:
-        if output_option == OutputOptions.sync_buffer_to_csv:
-            assert (
-                self._subsystem_manager.device_info[
-                    SubSystem.of_mount(mount)
-                ].revision.tertiary
-                == "1"
-            )
-        csv_output = bool(output_option.value & OutputOptions.stream_to_csv.value)
-        sync_buffer_output = bool(
-            output_option.value & OutputOptions.sync_buffer_to_csv.value
-        )
-        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()
-            }
-        )
         status = await capacitive_probe(
             messenger=self._messenger,
             tool=sensor_node_for_mount(mount),
             mover=axis_to_node(moving),
             distance=distance_mm,
             mount_speed=speed_mm_per_s,
-            csv_output=csv_output,
-            sync_buffer_output=sync_buffer_output,
-            can_bus_only_output=can_bus_only_output,
-            data_files=data_files_transposed,
             sensor_id=sensor_id_for_instrument(probe),
             relative_threshold_pf=sensor_threshold_pf,
         )

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

@@ -17,7 +17,7 @@
     Mapping,
 )
 
-from opentrons.config.types import OT3Config, GantryLoad, OutputOptions
+from opentrons.config.types import OT3Config, GantryLoad
 from opentrons.config import gripper_config
 
 from opentrons.hardware_control.module_control import AttachedModulesControl
@@ -63,7 +63,8 @@
 from opentrons.util.async_helpers import ensure_yield
 from .types import HWStopCondition
 from .flex_protocol import FlexBackend
-
+from opentrons_hardware.firmware_bindings.constants import SensorId
+from opentrons_hardware.sensors.types import SensorDataType
 
 log = logging.getLogger(__name__)
 
@@ -347,10 +348,11 @@ async def liquid_probe(
         threshold_pascals: float,
         plunger_impulse_time: float,
         num_baseline_reads: int,
-        output_format: OutputOptions = OutputOptions.can_bus_only,
-        data_files: Optional[Dict[InstrumentProbeType, str]] = None,
         probe: InstrumentProbeType = InstrumentProbeType.PRIMARY,
         force_both_sensors: bool = False,
+        response_queue: Optional[
+            asyncio.Queue[Dict[SensorId, List[SensorDataType]]]
+        ] = None,
     ) -> float:
         z_axis = Axis.by_mount(mount)
         pos = self._position
@@ -750,8 +752,6 @@ async def capacitive_probe(
         speed_mm_per_s: float,
         sensor_threshold_pf: float,
         probe: InstrumentProbeType = InstrumentProbeType.PRIMARY,
-        output_format: OutputOptions = OutputOptions.sync_only,
-        data_files: Optional[Dict[InstrumentProbeType, str]] = None,
     ) -> bool:
         self._position[moving] += distance_mm
         return True