feat(hardware): add acceleration to pick up tip for 96 channel #12944

caila-marashaj · 2023-06-21T18:00:20Z

This pr adds acceleration to pick up tip and drop tip moves for the gear motors on the 96 channel pipette. The ot3api signature's for pick_up_tip and drop_tip haven't changed, but the hardware controller's tip_action function will now optionally take in a list of moves. For the purpose of homing the gear motors, the backend tip_action function can also just take in a distance and velocity.

When pick_up_tip or drop_tip is called, the api function will now:

check to see if the gear motor's position is known- if not, it will home the gear motors before starting to move
perform the downward part of the tip action movement as normal, with acceleration
move most of the way back up also with acceleration
move the last 5 mm at a constant speed while looking for the limit switch to be pressed

Changelog

changed default speed for Q axis to 10
created separate homing functions in ot3controller and ot3utils for the gear motors
made backoff optional for homing the gear motors
added a gear_motor_position property in ot3controller
tweaked get_system_constraints so we can grab default acceleration for the Q axis
gave Axis.Q its own enum value, rather than having it be part of OTHER
added parsing for gear motor positions in move_group_runner

codecov · 2023-06-21T18:05:21Z

Codecov Report

Merging #12944 (42777ac) into internal-release_0.14.0 (9f29c9d) will decrease coverage by 7.34%.
Report is 1 commits behind head on internal-release_0.14.0.
The diff coverage is 92.30%.

❗ Current head 42777ac differs from pull request most recent head 6eb76e6. Consider uploading reports for the commit 6eb76e6 to get more accurate results

@@                     Coverage Diff                     @@
##           internal-release_0.14.0   #12944      +/-   ##
===========================================================
- Coverage                    72.53%   65.19%   -7.34%     
===========================================================
  Files                         2394     1599     -795     
  Lines                        66062    37028   -29034     
  Branches                      7342     6590     -752     
===========================================================
- Hits                         47915    24139   -23776     
+ Misses                       16401    11259    -5142     
+ Partials                      1746     1630     -116

Flag	Coverage Δ
api	`?`
app	`71.34% <ø> (ø)`
components	`?`
g-code-testing	`96.44% <ø> (ø)`
hardware	`56.55% <90.90%> (+0.06%)`	⬆️
hardware-testing	`∅ <ø> (∅)`
labware-library	`?`
notify-server	`89.13% <ø> (ø)`
ot3-gravimetric-test	`?`
protocol-designer	`47.30% <ø> (ø)`
react-api-client	`64.25% <ø> (ø)`
robot-server	`?`
shared-data	`78.80% <100.00%> (ø)`
step-generation	`87.18% <ø> (ø)`
system-server	`?`
update-server	`?`
usb-bridge	`?`

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

Files Changed	Coverage Δ
...p/src/organisms/LabwarePositionCheck/CheckItem.tsx	`65.62% <ø> (ø)`
...p/src/organisms/LabwarePositionCheck/PickUpTip.tsx	`62.06% <ø> (ø)`
...hardware/hardware_control/motion_planning/types.py	`89.74% <ø> (-1.71%)`	⬇️
...ns_hardware/scripts/high_throughput_diagnostics.py	`0.00% <ø> (ø)`
...s_hardware/scripts/high_throughput_tip_handling.py	`0.00% <ø> (ø)`
...ware/opentrons_hardware/hardware_control/motion.py	`84.21% <66.66%> (+0.21%)`	⬆️
...ns_hardware/firmware_bindings/messages/payloads.py	`95.47% <100.00%> (+0.01%)`	⬆️
...ons_hardware/hardware_control/move_group_runner.py	`93.18% <100.00%> (+0.07%)`	⬆️
...data/python/opentrons_shared_data/pipette/types.py	`96.11% <100.00%> (ø)`

... and 797 files with indirect coverage changes

Laura-Danielle

We talked on slack about it, but you need to make sure you're essentially mimicking what ot3_utils::create_move_group does for the tip action steps. Otherwise you won't be moving the motors correctly.

Laura-Danielle · 2023-06-23T14:54:25Z

api/src/opentrons/hardware_control/ot3api.py

+ moves = self._build_moves(gear_origin_dict, gear_target_dict)
+ blocks = moves[0][0].blocks
+
+ for block in blocks:


we should pass moves into the backend tip action

Laura-Danielle

Initial comments.

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

Laura-Danielle · 2023-07-07T12:31:38Z

hardware/opentrons_hardware/firmware_bindings/messages/payloads.py

+class GearMotorPositionResponse(EmptyPayload):
+ """Read Gear Motor Position Estimation."""
+
+ current_position: utils.UInt32Field


We can still have position flags here imo, it would just be related to boot-up only.

hardware/opentrons_hardware/firmware_bindings/messages/payloads.py

hardware/opentrons_hardware/hardware_control/motion.py

Laura-Danielle · 2023-07-07T12:57:39Z

hardware/opentrons_hardware/hardware_control/move_group_runner.py

- if len(self._expected_tip_action_motors) == 0:
+ seq_id = message.payload.seq_id.value
+ self._expected_tip_action_motors[seq_id].remove(gear_id)
+ print(f"expected tip motors = {self._expected_tip_action_motors}")


print statement here.

hardware/opentrons_hardware/hardware_control/move_group_runner.py

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

Laura-Danielle · 2023-07-07T15:36:25Z

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

@@ -563,14 +581,6 @@ async def home(
 if runner
 ]
 positions = await asyncio.gather(*coros)
- if OT3Axis.Q in checked_axes:


We should be updating the position here and also why did we remove the tip action from the home command? We need to handle the use-case where a home command is called without any parameters.

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

Laura-Danielle

Some more comments.

Mainly, it looks like you aren't handling the multiple types of responses you can now get from the 96 channel and there is some general code structure cleanup requests.

You should also make this PR not a draft and add a PR description please once you've finished those requests.

Laura-Danielle · 2023-07-19T10:52:01Z

hardware/tests/opentrons_hardware/hardware_control/test_move_group_runner.py

@@ -770,10 +771,10 @@ async def test_home_timeout(
 await subject.run(can_messenger=mock_can_messenger)


-async def test_tip_action_move_runner_receives_two_responses(
+async def test_tip_action_move_runner_receives_response(


We are still receiving multiple responses from the tip action request. We should have a test that captures that.

In fact, we should expand on this test to mimic all the varying responses you can get from a tip action (i.e. if it's an acceleration move, then 3 responses for each motor) or if it's a regular home then just two responses.

hardware/opentrons_hardware/hardware_control/move_group_runner.py

Laura-Danielle · 2023-07-19T10:56:31Z

hardware/opentrons_hardware/hardware_control/motor_position_status.py

+
+ data = []
+
+ with WaitableCallback(can_messenger, _listener_filter) as reader:


Bump on this comment.

Laura-Danielle · 2023-07-19T10:57:12Z

hardware/opentrons_hardware/hardware_control/motor_position_status.py

+ assert data[0] == data[1]
+ if not data[0][1] or not data[1][1]:
+ # If the stepper_ok flag isn't set, that means the node didn't update position.
+ raise RuntimeError(


Bump on this comment.

api/tests/opentrons/hardware_control/test_ot3_api.py

api/src/opentrons/hardware_control/ot3api.py

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

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

fsinapi

We should definitely rename the UpdateGearMotorPositionEstimation command and make sure the implementation of it just pulls the current position.

hardware/opentrons_hardware/firmware_bindings/messages/message_definitions.py

sfoster1

A couple nitpicks but really solid!

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

Laura-Danielle requested changes Jun 23, 2023

View reviewed changes

caila-marashaj force-pushed the accelerate-tip-action branch from 15c5190 to 9070819 Compare June 27, 2023 14:39

ahiuchingau force-pushed the accelerate-tip-action branch from 5856a53 to ffa0d23 Compare June 28, 2023 16:21

sfoster1 changed the title ~~feat(harwdare): add acceleration to pick up tip for 96 channel~~ feat(hardware): add acceleration to pick up tip for 96 channel Jun 30, 2023

caila-marashaj force-pushed the accelerate-tip-action branch from ffa0d23 to 6181560 Compare July 6, 2023 15:25

Laura-Danielle requested changes Jul 7, 2023

View reviewed changes

caila-marashaj force-pushed the accelerate-tip-action branch from e217855 to 79384d0 Compare July 18, 2023 15:47

Laura-Danielle reviewed Jul 19, 2023

View reviewed changes

caila-marashaj force-pushed the accelerate-tip-action branch from 0a0ed2b to e80089c Compare July 24, 2023 15:29

caila-marashaj marked this pull request as ready for review July 26, 2023 15:19

caila-marashaj requested a review from a team as a code owner July 26, 2023 15:19

caila-marashaj mentioned this pull request Jul 26, 2023

feat(pipettes): add acceleration to TipActionRequest message Opentrons/ot3-firmware#653

Merged

ahiuchingau requested changes Jul 26, 2023

View reviewed changes

api/src/opentrons/hardware_control/backends/ot3controller.py Show resolved Hide resolved

caila-marashaj requested review from ahiuchingau and a team July 26, 2023 18:10

fsinapi suggested changes Jul 26, 2023

View reviewed changes

hardware/opentrons_hardware/firmware_bindings/messages/message_definitions.py Outdated Show resolved Hide resolved

caila-marashaj changed the base branch from edge to internal-release_0.14.0 July 26, 2023 20:23

caila-marashaj requested review from a team as code owners July 26, 2023 20:23

caila-marashaj requested review from brenthagen and removed request for a team July 26, 2023 20:23

ahiuchingau changed the base branch from internal-release_0.14.0 to edge July 26, 2023 20:45

caila-marashaj force-pushed the accelerate-tip-action branch from f955057 to 42777ac Compare July 26, 2023 21:15

caila-marashaj changed the base branch from edge to internal-release_0.14.0 July 26, 2023 21:16

caila-marashaj added 5 commits July 26, 2023 17:37

add acceleration to tip_action

95d068b

pick up tip move happens, but rumbly

b4d4bee

linter stuff cleanup

bdea049

fixed position of acceleration in tipaction msg

782a760

moving but delayed

73d26bf

caila-marashaj added 22 commits July 26, 2023 17:40

modify tip action, debugging code still in there

c3e044a

mostly working but returning position too early

a85074e

need to update tests

1952348

updated unit test args for tip_action

fa194a3

rebase

d2e7927

rough final draft

2b20a20

cleanup

8cf3c15

format

7e5b7ef

remove redundant data check from gear motors

8ace0ef

hardware controller changes

155738b

increase backward distances

0aede03

changed helpers_ot3 function

97e12e9

format

1550c9a

helpers_ot3 function change

735ebe7

got rid of copysign call

af1eefa

check if speed exists

e2280aa

add deafult value to axis convert call

5be4492

update gear motor position handling after move

93ef4ca

test fix

f1a42d1

moved comments

2b71f07

remove updategearmotorpositionestimation msg

57bb955

removed more unneeded code

e201a43

caila-marashaj force-pushed the accelerate-tip-action branch from 42777ac to e201a43 Compare July 26, 2023 21:41

caila-marashaj requested review from fsinapi, Laura-Danielle and sfoster1 July 26, 2023 21:47

sfoster1 approved these changes Jul 27, 2023

View reviewed changes

api/src/opentrons/hardware_control/backends/ot3controller.py Show resolved Hide resolved

api/src/opentrons/hardware_control/backends/ot3controller.py Outdated Show resolved Hide resolved

api/src/opentrons/hardware_control/backends/ot3controller.py Show resolved Hide resolved

address change reqs

6eb76e6

caila-marashaj merged commit 6c38267 into internal-release_0.14.0 Jul 27, 2023
25 checks passed

caila-marashaj deleted the accelerate-tip-action branch July 27, 2023 16:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(hardware): add acceleration to pick up tip for 96 channel #12944

feat(hardware): add acceleration to pick up tip for 96 channel #12944

caila-marashaj commented Jun 21, 2023 •

edited

Loading

codecov bot commented Jun 21, 2023 •

edited

Loading

Laura-Danielle left a comment

Laura-Danielle Jun 23, 2023

Laura-Danielle left a comment

Laura-Danielle Jul 7, 2023

Laura-Danielle Jul 7, 2023

Laura-Danielle Jul 7, 2023

Laura-Danielle left a comment

Laura-Danielle Jul 19, 2023

Laura-Danielle Jul 19, 2023

Laura-Danielle Jul 19, 2023

fsinapi left a comment

sfoster1 left a comment


		data = []

		with WaitableCallback(can_messenger, _listener_filter) as reader:

feat(hardware): add acceleration to pick up tip for 96 channel #12944

feat(hardware): add acceleration to pick up tip for 96 channel #12944

Conversation

caila-marashaj commented Jun 21, 2023 • edited Loading

Changelog

codecov bot commented Jun 21, 2023 • edited Loading

Codecov Report

Laura-Danielle left a comment

Choose a reason for hiding this comment

Laura-Danielle Jun 23, 2023

Choose a reason for hiding this comment

Laura-Danielle left a comment

Choose a reason for hiding this comment

Laura-Danielle Jul 7, 2023

Choose a reason for hiding this comment

Laura-Danielle Jul 7, 2023

Choose a reason for hiding this comment

Laura-Danielle Jul 7, 2023

Choose a reason for hiding this comment

Laura-Danielle left a comment

Choose a reason for hiding this comment

Laura-Danielle Jul 19, 2023

Choose a reason for hiding this comment

Laura-Danielle Jul 19, 2023

Choose a reason for hiding this comment

Laura-Danielle Jul 19, 2023

Choose a reason for hiding this comment

fsinapi left a comment

Choose a reason for hiding this comment

sfoster1 left a comment

Choose a reason for hiding this comment

caila-marashaj commented Jun 21, 2023 •

edited

Loading

codecov bot commented Jun 21, 2023 •

edited

Loading