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(app, shared-data): Add liquidProbe command text #15722

Merged
merged 4 commits into from
Jul 19, 2024

Conversation

mjhuff
Copy link
Contributor

@mjhuff mjhuff commented Jul 19, 2024

Closes EXEC-567

Overview

Add new command text for the liquidProbe command. I've looked at some similar commands for inspiration.

Test Plan

  • Covered by new tests.

Changelog

  • Added liquidProbe command text.

Risk assessment

low

@mjhuff mjhuff requested review from ecormany, SyntaxColoring and a team July 19, 2024 17:56
@mjhuff mjhuff requested a review from a team as a code owner July 19, 2024 17:56
@mjhuff mjhuff requested review from TamarZanzouri and removed request for a team and TamarZanzouri July 19, 2024 17:56
Copy link
Contributor

@ecormany ecormany left a comment

Choose a reason for hiding this comment

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

Command text looks good. Is this command text supposed to be emitted when doing an aspirate with liquid_presence_detection=True set for the pipette? Because I'm not seeing that. (I also can't get standalone pipette.detect_liquid_presence() to work but that seems to be a separate bug.)

Copy link
Collaborator

@jerader jerader left a comment

Choose a reason for hiding this comment

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

makes sense to me! liquidProbe and tryLiquidProbe are supposed to be part of schema 9 right?

@mjhuff
Copy link
Contributor Author

mjhuff commented Jul 19, 2024

Command text looks good. Is this command text supposed to be emitted when doing an aspirate with liquid_presence_detection=True set for the pipette? Because I'm not seeing that. (I also can't get standalone pipette.detect_liquid_presence() to work but that seems to be a separate bug.)

My understanding is this is an entirely separate command that is emitted before an aspirate. CC @SyntaxColoring

makes sense to me! liquidProbe and tryLiquidProbe are supposed to be part of schema 9 right?

I'm like 90% sure this is correct but I'll CC @SyntaxColoring just to be safe.

@mjhuff mjhuff requested a review from a team as a code owner July 19, 2024 18:32
@mjhuff mjhuff changed the title feat(app): Add liquidProbe command text feat(app, shared-data): Add liquidProbe command text Jul 19, 2024
@SyntaxColoring
Copy link
Contributor

Is this command text supposed to be emitted when doing an aspirate with liquid_presence_detection=True set for the pipette?

Yes. The aspirate() in the user's Python script should emit a liquidProbe Protocol Engine command, which will get its own command text, and then emit an aspirate Protocol Engine command, which will also get its own command text.

makes sense to me! liquidProbe and tryLiquidProbe are supposed to be part of schema 9 right?

I'm like 90% sure this is correct but I'll CC @SyntaxColoring just to be safe.

Yep.

@mjhuff mjhuff merged commit b46305f into edge Jul 19, 2024
57 checks passed
@mjhuff mjhuff deleted the app_liquid-probe-command-text branch July 19, 2024 20:24
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.

5 participants