-
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): raise error when using gripper in ot2 protocols #13208
fix(api, shared-data): raise error when using gripper in ot2 protocols #13208
Conversation
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.
Looks good to me!
@@ -120,6 +120,10 @@ async def execute(self, params: MoveLabwareParams) -> MoveLabwareResult: | |||
) | |||
|
|||
if params.strategy == LabwareMovementStrategy.USING_GRIPPER: | |||
if self._state_view.config.robot_type == "OT-2 Standard": | |||
raise NotSupportedOnRobotType( |
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.
maybe add like "action": "gripper-move-labware"
or something to details?
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's the difference between that and putting "Labware movement using a gripper is not supported on the OT2"
in message
?
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.
nothing :) it's just nice to have both
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.
Can't use the estop error code for this, let's make a new one for hardware not supported
wrapping: Optional[Sequence[EnumeratedError]] = None, | ||
) -> None: | ||
"""Build a NotSupportedOnRobotType exception.""" | ||
super().__init__(ErrorCodes.E_STOP_ACTIVATED, message, details, wrapping) |
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.
Wait no this is not the right code for this. Let's add a 4000 series "not supported on hardware" error code in shared-data/errors/definitions/1.json
, a python binding for the code, and maybe an exception in shared-data (don't necessarily need it if you're building a protocol engine exception instead) for it.
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.
🤦🏻♀️ you can tell where I copy pasted that from.
Will make the changes
Codecov Report
@@ Coverage Diff @@
## internal-release_0.14.0 #13208 +/- ##
===========================================================
+ Coverage 72.14% 72.59% +0.45%
===========================================================
Files 1571 2391 +820
Lines 51948 65968 +14020
Branches 3281 7321 +4040
===========================================================
+ Hits 37476 47892 +10416
- Misses 13956 16328 +2372
- Partials 516 1748 +1232
Flags with carried forward coverage won't be shown. Click here to find out more.
|
class NotSupportedOnRobotType(ProtocolEngineError): | ||
"""Raised when attempting to perform an action that is not supported for the given robot type.""" |
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's the difference between HardwareNotSupportedError
and NotSupportedOnRobotType
?
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.
HardwareNotSupportedError
is raised after checking the specific hardware API type and will only be able to be checked when running on an actual robot (or emulator), while NotSupportedOnRobotType
checks only the robot type specified for the particular protocol/ engine. Subtle difference but an analysis saying 'not supported on hardware' sounds kinda misleading.
Co-authored-by: Ed Cormany <[email protected]>
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.
Great! Thank you!
@@ -120,6 +120,10 @@ async def execute(self, params: MoveLabwareParams) -> MoveLabwareResult: | |||
) | |||
|
|||
if params.strategy == LabwareMovementStrategy.USING_GRIPPER: | |||
if self._state_view.config.robot_type == "OT-2 Standard": | |||
raise NotSupportedOnRobotType( |
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.
nothing :) it's just nice to have both
Overview
Closes RSS-297
Test Plan
Check if an OT2 protocol fails analysis when there's a
move_labware
step using gripper.Changelog
NotSupportedOnRobotType
errorReview requests
Risk assessment
None.