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): add get tip presence command in protocol engine #14095

Merged

Conversation

ahiuchingau
Copy link
Contributor

@ahiuchingau ahiuchingau commented Dec 5, 2023

Overview

This PR adds the get_tip_presence command in protocol engine so we can actually get the realtime tip presence response from the pipette, instead of having to rely on the instrument endpoint which only returns the cached tip detected value.
Closes RQA-1992

@ahiuchingau ahiuchingau requested review from a team as code owners December 5, 2023 17:57
Copy link

codecov bot commented Dec 5, 2023

Codecov Report

Merging #14095 (86f5ab6) into chore_release-7.1.0 (3a2eae7) will decrease coverage by 0.01%.
Report is 1 commits behind head on chore_release-7.1.0.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@                   Coverage Diff                   @@
##           chore_release-7.1.0   #14095      +/-   ##
=======================================================
- Coverage                70.44%   70.43%   -0.01%     
=======================================================
  Files                     2483     2512      +29     
  Lines                    70327    71289     +962     
  Branches                  8767     8996     +229     
=======================================================
+ Hits                     49539    50215     +676     
- Misses                   18637    18872     +235     
- Partials                  2151     2202      +51     
Flag Coverage Δ
app 67.67% <ø> (ø)
components 60.14% <ø> (ø)
g-code-testing 96.44% <ø> (ø)
labware-library 51.50% <ø> (ø)
notify-server 89.13% <ø> (ø)
protocol-designer 45.03% <ø> (+0.04%) ⬆️
react-api-client 65.73% <ø> (ø)
shared-data 75.05% <ø> (-0.80%) ⬇️
step-generation 86.84% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
api/src/opentrons/hardware_control/ot3api.py 79.58% <ø> (ø)
...src/opentrons/protocol_engine/commands/__init__.py 100.00% <ø> (ø)
...entrons/protocol_engine/commands/command_unions.py 100.00% <ø> (ø)
api/src/opentrons/protocol_engine/types.py 97.50% <ø> (ø)

... and 32 files with indirect coverage changes

Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

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

I think it actually makes more sense for the verify to raise, and for verify to be something that's really very rarely called and only intentionally when you want it to raise.

pipette_id = params.pipetteId
expected_state = params.expectedState

result = await self._tip_handler.verify_tip_presence(
Copy link
Member

Choose a reason for hiding this comment

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

I think I'd expect this to raise, because the only reason that I would call this instead of get tip presence is to simplify a status check that should fail.

Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

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

Okay, looks good to me! Should fix the tip presence error too!

@ahiuchingau ahiuchingau force-pushed the api_check-tip-presence-in-protocol-engine branch from bb43975 to 241f9cd Compare December 5, 2023 22:28
format

typo

add command to schema 8

made prettier

add get tip presence to shared-data

add more tests
@ahiuchingau ahiuchingau force-pushed the api_check-tip-presence-in-protocol-engine branch from 241f9cd to 86f5ab6 Compare December 5, 2023 22:31
@ahiuchingau ahiuchingau requested review from a team as code owners December 5, 2023 22:31
@ahiuchingau ahiuchingau requested review from TamarZanzouri and removed request for a team December 5, 2023 22:31
@ahiuchingau ahiuchingau changed the base branch from edge to chore_release-7.1.0 December 5, 2023 22:31
@ahiuchingau ahiuchingau merged commit 7749f36 into chore_release-7.1.0 Dec 5, 2023
68 of 69 checks passed
@ahiuchingau ahiuchingau deleted the api_check-tip-presence-in-protocol-engine branch December 5, 2023 22:51
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.

None yet

2 participants