Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(api): refresh gripper jaw state from firmware #13506

Merged
merged 11 commits into from
Sep 11, 2023

Conversation

ahiuchingau
Copy link
Contributor

@ahiuchingau ahiuchingau commented Sep 8, 2023

Overview

The gripper firmware can now report the active jaw state–this means that we don't need to solely rely on the hardware controller to keep track of what the gripper is doing.

Knowing the currently jaw state, along with the jaw width reported by the brushed motor encoder, the hardware controller can safely assume whether or not a labware is actively being held by the jaw. (We can do this by checking the the minimum jaw width against the current jaw width; if those values are not close enough, it's very likely that the the gripper is holding something.)

We want this because we can use this info to determine whether or not we should home the gripper jaw during protocol cleanup. YAY!

Test Plan

Run a protocol that uses the gripper to move a labware around, and while the gripper is holding on the labware:

  • cancel the protocol
  • cause the protocol to fail somehow, e.g cause a collision

...and make sure that the labware just doesn't get dropped when (1) the protocol gets canceled or (2) when the robot homes all axes during protocol cleanup.

Here's a sample protocol:

from opentrons import protocol_api

requirements = {
    "robotType": "OT-3",
    "apiLevel": "2.15",
}

def run(protocol: protocol_api.ProtocolContext):
    sample_plate = protocol.load_labware('nest_96_wellplate_100ul_pcr_full_skirt','5')

    protocol.move_labware(
        labware=sample_plate,
        new_location='6',
        use_gripper=True
    )

Make sure you're using the firmware changes in this PR: Opentrons/ot3-firmware#719

Review requests

On the protocol engine side, we should handle that GripError differently when we're encountering it during protocol cleanup and when we're about to start a protocol.

@ahiuchingau ahiuchingau marked this pull request as ready for review September 8, 2023 20:58
@ahiuchingau ahiuchingau requested a review from a team as a code owner September 8, 2023 20:58
@codecov
Copy link

codecov bot commented Sep 8, 2023

Codecov Report

Merging #13506 (13cc9ad) into chore_release-7.0.0 (fdf6e22) will decrease coverage by 0.02%.
Report is 16 commits behind head on chore_release-7.0.0.
The diff coverage is 5.00%.

Additional details and impacted files

Impacted file tree graph

@@                   Coverage Diff                   @@
##           chore_release-7.0.0   #13506      +/-   ##
=======================================================
- Coverage                71.32%   71.31%   -0.02%     
=======================================================
  Files                     2427     2427              
  Lines                    67943    67959      +16     
  Branches                  7880     7880              
=======================================================
+ Hits                     48462    48465       +3     
- Misses                   17627    17640      +13     
  Partials                  1854     1854              
Flag Coverage Δ
hardware 56.41% <5.00%> (-0.07%) ⬇️
notify-server 89.13% <ø> (ø)

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

Files Changed Coverage Δ
...entrons/hardware_control/backends/ot3controller.py 68.20% <ø> (ø)
...pentrons/hardware_control/backends/ot3simulator.py 86.44% <ø> (ø)
...rc/opentrons/hardware_control/backends/ot3utils.py 94.23% <ø> (ø)
api/src/opentrons/hardware_control/ot3api.py 79.14% <ø> (-0.12%) ⬇️
api/src/opentrons/hardware_control/types.py 96.57% <ø> (ø)
...i/src/opentrons/protocol_engine/errors/__init__.py 100.00% <ø> (ø)
...src/opentrons/protocol_engine/errors/exceptions.py 100.00% <ø> (ø)
...i/src/opentrons/protocol_runner/protocol_runner.py 100.00% <ø> (ø)
...ns_hardware/firmware_bindings/messages/messages.py 91.66% <ø> (ø)
...rons_hardware/hardware_control/gripper_settings.py 0.00% <0.00%> (ø)
... and 1 more

... and 2 files with indirect coverage changes

Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like checking and returning the state but I don't think we should raise an error if you try and home while we're gripping and the jaws are at least 5mm apart. While it's a reasonable assumption that this means a labware is in the jaws, it's still an assumption, and completely locking out the ability of the jaw to home in that state means it would be stuck until a machine reboot or until you pulled the gripper off.

Let's remove raising that error but otherwise looks good!

@ahiuchingau
Copy link
Contributor Author

I like checking and returning the state but I don't think we should raise an error if you try and home while we're gripping and the jaws are at least 5mm apart. While it's a reasonable assumption that this means a labware is in the jaws, it's still an assumption, and completely locking out the ability of the jaw to home in that state means it would be stuck until a machine reboot or until you pulled the gripper off.

Let's remove raising that error but otherwise looks good!

My intention was to force us to only use ungrip to indicate we want to release the jaw when a gripper is assumed to be actively gripping. I believe we're adding a button on the app so we can ungrip whenever we're outside of a protocol if the gripper is holding something so we wouldn't be stuck in the cannot be homed state.

@sfoster1
Copy link
Member

I guess... I really like keeping home as a thing that always resets the thing whenever possible though. I think the reason to lock out home here is just to prevent programming errors on higher layers, but we're sort of making ourselves vulnerable to the same error.

@ahiuchingau ahiuchingau changed the base branch from edge to chore_release-7.0.0 September 11, 2023 20:02
Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense!

@ahiuchingau ahiuchingau merged commit 6d649ac into chore_release-7.0.0 Sep 11, 2023
14 of 25 checks passed
@ahiuchingau ahiuchingau deleted the api_check-jaw-state-before-home branch September 11, 2023 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants