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): express stalls in a recoverable way #16861

Merged
merged 5 commits into from
Nov 19, 2024

Conversation

sfoster1
Copy link
Member

@sfoster1 sfoster1 commented Nov 15, 2024

Commands that use move_to_well, move_to_coordinates, move_to_addressable_area, and move_relative now will return stalls as DefinedErrors, which means they can be hooked into error recovery en masse.

This just leaves move labware.

Closes EXEC-831

Reviews

  • Feel like it paid off?

Testing

  • Do some stalls and make sure they end up with defined errors

Further work and questions

  • I think we should probably handle errors during aspirate's automatic prepare for aspirate at the beginning and probably the same with drop tip but we didn't handle it at all so far so I don't know

Copy link
Contributor

@TamarZanzouri TamarZanzouri left a comment

Choose a reason for hiding this comment

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

That was quick! Thank you!

@TamarZanzouri
Copy link
Contributor

one small note, I worry that the error construction will now be scattered in a bunch of places, but it makes sense to add this logic in one place so I think its ok.

Copy link
Contributor

@SyntaxColoring SyntaxColoring left a comment

Choose a reason for hiding this comment

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

Nice.

Definitely paid off—way better than a global try/catch. Thank you!

Comment on lines +95 to +108
class StallOrCollisionError(ErrorOccurrence):
"""Returned when the machine detects that axis encoders are reading a different position than expected.

All axes are stopped at the point where the error was encountered.

The next thing to move the machine must account for the robot not having a valid estimate
of its position. It should be a `home` or `unsafe/updatePositionEstimators`.
"""

isDefined: bool = True
errorType: Literal["stallOrCollision"] = "stallOrCollision"

errorCode: str = ErrorCodes.STALL_OR_COLLISION_DETECTED.value.code
detail: str = ErrorCodes.STALL_OR_COLLISION_DETECTED.value.detail
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the docstring.

Base automatically changed from exec-830-movement to edge November 18, 2024 19:12
sfoster1 and others added 5 commits November 18, 2024 14:14
Commands that use move_to_well now will return stalls as DefinedErrors,
which means they can be hooked into error recovery en masse.

Closes EXEC-831
Both move to addressable are and move to addressable area for drop tip
handle stalls now.
@sfoster1
Copy link
Member Author

one small note, I worry that the error construction will now be scattered in a bunch of places, but it makes sense to add this logic in one place so I think its ok.

Well, that's the goal with the micro-operations - now they're not scattered, they're all in one place for each kind of micro-operation

@sfoster1 sfoster1 merged commit 05eeed7 into edge Nov 19, 2024
49 checks passed
@sfoster1 sfoster1 deleted the exec-831-stall-defined-error branch November 19, 2024 15:15
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.

3 participants