-
Notifications
You must be signed in to change notification settings - Fork 179
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
fix(api, shared-data): Allow labware lids to be disposed in the trash bin #16638
Conversation
if labware_validation.validate_definition_is_lid( | ||
self._state_view.labware.get_definition(params.labwareId) | ||
): | ||
self._state_view.labware.get_labware_gripper_offsets |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is this doing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
secondary validation that the labware we are about to move to the trash is infact a lid. We validate this once at the PAPI call, but we validate it here to catch incase someone tries to do this during a maintenance run or through other direct-engine command implementations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah that i see, im talking specifically about
self._state_view.labware.get_labware_gripper_offsets
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually just got that accidental variable call you highlighted on 178, thats unintentional good catch.
if ( | ||
"lidDisposalOffsets" | ||
in current_labware_definition.gripperOffsets.keys() | ||
): | ||
trash_lid_drop_offset = LabwareOffsetVector( | ||
x=current_labware_definition.gripperOffsets[ | ||
"lidDisposalOffsets" | ||
].dropOffset.x, | ||
y=current_labware_definition.gripperOffsets[ | ||
"lidDisposalOffsets" | ||
].dropOffset.y, | ||
z=current_labware_definition.gripperOffsets[ | ||
"lidDisposalOffsets" | ||
].dropOffset.z, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could change to
lid_disposable_offfets = current_labware_definition.gripperOffsets.get('lidDisposalOffsets')
if lis_disposable_offsets is not None:
trash_lid_drop_offset = LabwareOffsetVector(**lid_disposable_offsets['dropOffset'])
else:
raise ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice,
I left a few comments
I tested on the Flex and everything works as expected.
Overview
Covers PLAT-539
This allows the TC lid to be dropped in the trash bin at a slight offset. Originally we only allowed it to be dropped in the waste chute, but we need to let it be disposed of in some sanitary way in other disposal locations as well.
Test Plan and Hands on Testing
The following protocol should move a TC lids from a stack on the deck into a trash bin:
Changelog
Review requests
Do movements achieve what we want to unblock users without waste chutes? Do those analysis error messages make sense for when people try to do unsupported actions?
Risk assessment
This currently allows any labware with the allowed role of "Lid" to be dropped in the trash bin. This means users could start trashing other lids in the future, so we may want to limit this via a labware quirk instead. On the other hand, giving users a sanitary way to dispose of any and all future lid types even if they don't own a waste chute may be useful. Thoughts?