feat(engine): use grip force and height from definitions in pe #13233
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## internal-release_0.14.0 #13233 +/- ##
===========================================================
- Coverage 72.52% 72.10% -0.43%
===========================================================
Files 2392 1571 -821
Lines 66072 51965 -14107
Branches 7338 3281 -4057
===========================================================
- Hits 47921 37470 -10451
+ Misses 16404 13979 -2425
+ Partials 1747 516 -1231
Flags with carried forward coverage won't be shown. Click here to find out more.
|
SyntaxColoring
approved these changes
Aug 4, 2023
Comment on lines
767
to
775
|
|
||
| def get_grip_force(self, labware_id: str) -> float: | ||
| """Get the recommended grip force for gripping labware using gripper.""" | ||
| return self.get_definition(labware_id).gripForce or LABWARE_GRIP_FORCE | ||
|
|
||
| def get_grip_height_from_labware_bottom(self, labware_id: str) -> float: | ||
| """Get the recommended grip height from labware bottom, if present.""" | ||
| recommended_height = self.get_definition(labware_id).gripHeightFromLabwareBottom | ||
| return recommended_height or self.get_dimensions(labware_id).z / 2 |
There was a problem hiding this comment.
I think this is probably fine in practice because nobody will do this, but beware that this will do the wrong thing if someone does "gripForce": 0 or "gripHeightFromLabwareBottom": 0 for some reason.
There was a problem hiding this comment.
Good point. Although it really doesn't make sense to have a force or height of 0. But it makes sense to correct this.
sfoster1
deleted the
RSS-279-use-grip-force-and-height-from-definitions-in-pe
branch
8 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes RSS-279
Overview
Updates the labware movement code in PE to use the grip height & force specified by the labware definition, or fallback to default if those are not specified.
Test Plan
(Currently testing with @andySigler )
Changelog
gripHeightFromLabwareBottomproperty if present in definition, otherwise use default we've been using so fargripForcespecified in definition if present, otherwise fallback to the default force we've been using so far.Review requests
Risk assessment
Medium. Code is pretty isolated and well-tested so it should not cause any issues as long as tests are passing.