refactor(api): hardware controller use error codes #13318
Merged
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
Additional details and impacted files@@ Coverage Diff @@
## edge #13318 +/- ##
==========================================
- Coverage 71.26% 71.08% -0.19%
==========================================
Files 1588 1593 +5
Lines 52824 53026 +202
Branches 3481 3548 +67
==========================================
+ Hits 37646 37693 +47
- Misses 14627 14782 +155
Partials 551 551
Flags with carried forward coverage won't be shown. Click here to find out more.
|
sfoster1
reviewed
Aug 17, 2023
|
|
||
| @wraps(func) | ||
| async def wrapper(self: Any, *args: Any, **kwargs: Any) -> Any: | ||
| if self.update_required and self.initialized: | ||
| raise FirmwareUpdateRequired() | ||
| raise FirmwareUpdateRequiredError |
There was a problem hiding this comment.
the idea with these errors is that you can pack details in them, and we should generally take advantage of that. so here for instance, you could put in the details argument a dict containing like
func.__name__update_requiredandinitialized- maybe even poke into
self.subsystemsand figuring out which subsystems need the update
| def __init__(self, message: str): | ||
| self.message = message | ||
| super().__init__() | ||
| super().__init__(message=message) |
There was a problem hiding this comment.
we should also pass the details in and store them in details
api/src/opentrons/hardware_control/instruments/ot3/gripper_handler.py
Outdated
Show resolved
Hide resolved
ahiuchingau
force-pushed
the
api_use-error-codes
branch
from
aa3095e to
2ec3a3f
Compare
ahiuchingau
force-pushed
the
api_use-error-codes
branch
from
3f62208 to
92ce759
Compare
HardwareMustHomeError -> PositionUnknownError GripperNotAttachedError -> GripperNotPresentError AxisNotPresentError -> InvalidActuator FirmwareUpdateRequired -> FirmwareUpdateRequiredError FirmwareUpdateFailed -> FirmwareUpdateFailedError InvalidMove -> InvalidCriticalPoint UpdateOngoingError as part of RobotInUseError NotSupportedByHardware -> UnsupportedHardwareCommand NoTipAttachedError -> UnexpectedTipRemovalError TipAttachedError -> UnexpectedTipAttachError add detail for error msgs in ot3api.py gripper errors fix tests
ahiuchingau
force-pushed
the
api_use-error-codes
branch
from
cae045d to
306b9d6
Compare
ahiuchingau
requested review from
brenthagen
and removed request for
a team
sfoster1
requested changes
Sep 20, 2023
| @@ -107,7 +107,7 @@ async def get_position( | |||
| critical_point=pipette_location.critical_point, | |||
| fail_on_not_homed=fail_on_not_homed, | |||
| ) | |||
| except HardwareMustHomeError as e: | |||
| except PositionUnknownError as e: | |||
| raise MustHomeError(str(e)) from e | |||
There was a problem hiding this comment.
this should link the lower level error with wraps=[e]
| @@ -167,7 +167,7 @@ async def move_relative( | |||
| critical_point=critical_point, | |||
| fail_on_not_homed=True, | |||
| ) | |||
| except HardwareMustHomeError as e: | |||
| except PositionUnknownError as e: | |||
| raise MustHomeError(str(e)) from e | |||
There was a problem hiding this comment.
should link the error with wraps=[e]
| f"Motor positions for {str(mount)} are missing; must first home motors." | ||
| raise PositionUnknownError( | ||
| message=f"Motor positions for {str(mount)} are missing; must first home motors.", | ||
| detail={"mount": str(mount)}, |
There was a problem hiding this comment.
can we put the exact missing axis positions in the mount?
| raise MustHomeError( | ||
| f"Current position of {str(mount)} pipette is unknown, please home." | ||
| raise PositionUnknownError( | ||
| message=f"Current position of {str(mount)} pipette is unknown, please home." |
There was a problem hiding this comment.
maybe put in the details whether the problem is that the axis isn't homed or that position hasn't been pulled yet
| raise NotSupportedByHardware( | ||
| f"At least one axis in {axes} is not supported on the OT2." | ||
| raise UnsupportedHardwareCommand( | ||
| message=f"At least one axis in {axes} is not supported on the OT2." |
There was a problem hiding this comment.
maybe worth putting in which axis it is, at least in the details
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.
Overview
We are now using the error codes in the hardware controller api. Let me know if i forgot anything!