refactor(hardware): give options for sensor data output during probe #14673

caila-marashaj · 2024-03-15T16:16:58Z

Overview

These are some small changes that help accommodate liquid-level detection testing in the hardware layer.
EXEC-337

Changelog

add AddSensorLinearMoveRequest, SendAccumulatedPressureDataRequest CAN messages
add sensor_report Move Condition to allow us to request that the firmware accumulate data in the necessary manner for hardware testing.
modify tool_sensors.py to allow its users to choose between reporting sensor data to the CAN bus only, writing sensor data to a csv file, or not reporting data at all during a pass.

Review requests

The CAN messages mentioned above will probably be temporary additions to edge, but I think merging them and potentially later on removing them may be the best way to go about this. Let me know if not.

codecov · 2024-03-15T21:01:06Z

Codecov Report

Attention: Patch coverage is 64.17910% with 24 lines in your changes are missing coverage. Please review.

Project coverage is 67.17%. Comparing base (2857419) to head (500f2d0).
Report is 4 commits behind head on edge.

Additional details and impacted files

@@            Coverage Diff             @@
##             edge   #14673      +/-   ##
==========================================
- Coverage   67.19%   67.17%   -0.03%     
==========================================
  Files        2495     2495              
  Lines       71563    71612      +49     
  Branches     9076     9076              
==========================================
+ Hits        48088    48105      +17     
- Misses      21330    21362      +32     
  Partials     2145     2145

Flag	Coverage Δ
g-code-testing	`92.43% <ø> (ø)`
hardware	`55.43% <64.17%> (-0.11%)`	⬇️
hardware-testing	`∅ <ø> (∅)`

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

Files	Coverage Δ
api/src/opentrons/config/defaults_ot3.py	`100.00% <ø> (ø)`
api/src/opentrons/config/types.py	`100.00% <ø> (ø)`
...entrons/hardware_control/backends/ot3controller.py	`68.48% <ø> (ø)`
...pentrons/hardware_control/backends/ot3simulator.py	`86.11% <ø> (-0.13%)`	⬇️
api/src/opentrons/hardware_control/ot3api.py	`79.55% <ø> (ø)`
.../opentrons_hardware/firmware_bindings/constants.py	`99.26% <100.00%> (+<0.01%)`	⬆️
.../firmware_bindings/messages/message_definitions.py	`97.57% <100.00%> (+0.04%)`	⬆️
...ns_hardware/firmware_bindings/messages/messages.py	`91.66% <ø> (ø)`
...ns_hardware/firmware_bindings/messages/payloads.py	`95.95% <100.00%> (+0.07%)`	⬆️
...ware/opentrons_hardware/hardware_control/motion.py	`82.75% <100.00%> (+0.20%)`	⬆️
... and 2 more

... and 1 file with indirect coverage changes

sfoster1

Couple small things to change like removing prints and making a function invocation kwargs. I think I would have done the split of the CSV stuff slightly differently but I think this way will work and I'm interested to see how it turns out.

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

sfoster1 · 2024-03-18T13:46:04Z

hardware/opentrons_hardware/firmware_bindings/messages/message_definitions.py

+
+
+@dataclass
+class SendAccumulatedPressureDataRequest(BaseMessage):


what response message does this use?

this prompts all of the ReadFromSensorResponse messages to be dumped from the array the firmware accumulates during liquid probe

It sends the same ReadFromSensorResponse as we did before

hardware/opentrons_hardware/hardware_control/tool_sensors.py

ryanthecoder · 2024-03-18T20:38:37Z

hardware/opentrons_hardware/hardware_control/tool_sensors.py

+    return move_group
+
+
+async def run_pass_output_to_csv(


We still need a way to gate the behavior here to see if we want to use the stream can messages to csv behavior we had before, or the new buffer-on-pipette then write we need.
the way we run the move group runner is different with the custom firmware.

the new custom firmware needs this logic

messenger.add_listener(sensor_capturer, None) async with sensor_capturer: print("starting move group runner") positions = await sensor_runner.run(can_messenger=messenger) await messenger.send( node_id=tool, message=SendAccumulatedPressureDataRequest( payload=SendAccumulatedPressureDataPayload( sensor_id=SensorIdField(sensor_id) ) ), ) await asyncio.sleep(10) messenger.remove_listener(sensor_capturer) await messenger.send( node_id=tool, message=BindSensorOutputRequest( payload=BindSensorOutputRequestPayload( sensor=SensorTypeField(SensorType.pressure), sensor_id=SensorIdField(sensor_id), binding=SensorOutputBindingField(SensorOutputBinding.none), ) ), )

and the normal firmware needs what you have here

ryanthecoder · 2024-03-18T21:02:06Z

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

+            plunger_speed=plunger_speed,
+            mount_speed=mount_speed,
+            threshold_pascals=threshold_pascals,
+            output_format=OutputOptions.none,


we need to hook this up one more layer to ot3_api,
and if the Output Options chosen is the one that needs the custom firm we should assert here that
self._subsystem_manager.device_info[SubSystem.of_mount(mount)].revision.tertiary == "1"

ryanthecoder · 2024-03-19T15:28:12Z

hardware/opentrons_hardware/hardware_control/tool_sensors.py

+        #   will be the same
+        duration=float64(abs(distance[movers[0]] / speed[movers[0]])),
+        present_nodes=pipette_nodes,
+        stop_condition=MoveStopCondition.sensor_report,


This also needs to be gated on using the custom firmware or not, the normal firmware cannot process this stop condition
And we'll need to grab the changes from move_group_runner to handle this stop condition too

…ferentiate buffered on pipette moves and regular firmware moves

sfoster1

Okay, looks good to me!

…14673)

caila-marashaj force-pushed the add-tool-sensors-option-for-lld branch from f4ca186 to 5c1c040 Compare March 15, 2024 19:49

caila-marashaj changed the title ~~givet tool sensors output options~~ refactor(hardware): give options for sensor data output during probe Mar 15, 2024

caila-marashaj requested a review from a team March 15, 2024 20:31

sfoster1 requested changes Mar 18, 2024

View reviewed changes

ryanthecoder reviewed Mar 18, 2024

View reviewed changes

ryanthecoder reviewed Mar 19, 2024

View reviewed changes

ryanthecoder force-pushed the add-tool-sensors-option-for-lld branch from fa01e9f to fd0aec9 Compare March 21, 2024 19:46

caila-marashaj and others added 12 commits March 25, 2024 10:24

givet tool sensors output options

3b3b802

add can messges for lld testing

086bc77

tool sensors changes

78b9909

api lint fix

e3af5f0

test changes

51a3120

remove prints and add arg keyword

414938b

implement the new move type in tool sensors, and provide a way to dif…

88f33c9

…ferentiate buffered on pipette moves and regular firmware moves

hook up the new output options for liquid probe up through ot3api

6250709

add the new move to move-group-runner

960eddd

format lint

e5c5b61

fix hardware-testing liquid-probes

59d98d2

update dummy config

2864274

caila-marashaj force-pushed the add-tool-sensors-option-for-lld branch from fd0aec9 to 2864274 Compare March 25, 2024 14:39

caila-marashaj added 2 commits March 25, 2024 10:57

move output enum to config types

f65299a

get api tests passing

500f2d0

ryanthecoder approved these changes Mar 25, 2024

View reviewed changes

caila-marashaj marked this pull request as ready for review March 25, 2024 17:39

caila-marashaj requested a review from a team as a code owner March 25, 2024 17:39

sfoster1 approved these changes Mar 25, 2024

View reviewed changes

caila-marashaj merged commit 36de306 into edge Mar 25, 2024
23 of 24 checks passed

caila-marashaj deleted the add-tool-sensors-option-for-lld branch March 25, 2024 20:07

Carlos-fernandez pushed a commit that referenced this pull request May 20, 2024

refactor(hardware): give options for sensor data output during probe (#…

7995d78

…14673)

Carlos-fernandez pushed a commit that referenced this pull request Jun 3, 2024

refactor(hardware): give options for sensor data output during probe (#…

1c6a2eb

…14673)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(hardware): give options for sensor data output during probe #14673

refactor(hardware): give options for sensor data output during probe #14673

caila-marashaj commented Mar 15, 2024 •

edited by ryanthecoder

Loading

codecov bot commented Mar 15, 2024 •

edited

Loading

sfoster1 left a comment

sfoster1 Mar 18, 2024

caila-marashaj Mar 18, 2024

ryanthecoder Mar 18, 2024

ryanthecoder Mar 18, 2024

ryanthecoder Mar 18, 2024

ryanthecoder Mar 19, 2024 •

edited

Loading

sfoster1 left a comment



		@dataclass
		class SendAccumulatedPressureDataRequest(BaseMessage):

refactor(hardware): give options for sensor data output during probe #14673

refactor(hardware): give options for sensor data output during probe #14673

Conversation

caila-marashaj commented Mar 15, 2024 • edited by ryanthecoder Loading

Overview

Changelog

Review requests

codecov bot commented Mar 15, 2024 • edited Loading

Codecov Report

sfoster1 left a comment

Choose a reason for hiding this comment

sfoster1 Mar 18, 2024

Choose a reason for hiding this comment

caila-marashaj Mar 18, 2024

Choose a reason for hiding this comment

ryanthecoder Mar 18, 2024

Choose a reason for hiding this comment

ryanthecoder Mar 18, 2024

Choose a reason for hiding this comment

ryanthecoder Mar 18, 2024

Choose a reason for hiding this comment

ryanthecoder Mar 19, 2024 • edited Loading

Choose a reason for hiding this comment

sfoster1 left a comment

Choose a reason for hiding this comment

caila-marashaj commented Mar 15, 2024 •

edited by ryanthecoder

Loading

codecov bot commented Mar 15, 2024 •

edited

Loading

ryanthecoder Mar 19, 2024 •

edited

Loading