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, shared-data): build out functionality for tracking liquid height #15666

Closed
wants to merge 3 commits into from

Conversation

aaron-kulkarni
Copy link
Contributor

@aaron-kulkarni aaron-kulkarni commented Jul 15, 2024

Overview

Added public getter and private setter for tracking liquid level heights of wells.

re EXEC-603

Test Plan

Still need to write a ton of unit tests.

Changelog

  • Added new exception: NoLiquidHeightDataError. This will only be for internal use and a user should never receive it.
  • get_well_last_measured_height starts in protocol_api/labware, calls down to protocol_api/core/labware, then finally down to protocol_engine/state/labware
  • set_well_last_measured_height is only called in liquid_probe_in_place in protocol_engine/execution/pipetting.py
  • Adds a new attribute to labware definitoin called WellState. WellState has attribute lastMeasuredLiquidHeight. I decided to make the WellState attribute for future cases where we might want more detailed well state tracking.

Review requests

Not sure if the way I'm setting it is correct. I couldn't find a convenient and seemingly correct way to edit labware state so I'm just doing it in liquid_probe_in_place.

I also ended up having to add wellStates to the labware schema which seems wrong but I don't know how else I would do it.

Risk assessment

Pretty low.

…eight of wells

Added public getter and private setter for tracking liquid level heights of wells.

re EXEC-603
@Opentrons Opentrons deleted a comment from github-actions bot Jul 15, 2024
Copy link
Contributor

A PR has been opened to address analyses snapshot changes. Please review the changes here: https://github.com/Opentrons/opentrons/pull/

Copy link
Contributor

A PR has been opened to address analyses snapshot changes. Please review the changes here: https://github.com/Opentrons/opentrons/pull/

@aaron-kulkarni
Copy link
Contributor Author

I think this is the wrong way to go about this. I'm going to start a new PR that implements WellState in a better way.

@aaron-kulkarni aaron-kulkarni deleted the exec-603-add-last-height branch July 16, 2024 19:08
@aaron-kulkarni
Copy link
Contributor Author

Here is the other PR that takes a different approach to this problem: #15681

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.

1 participant