feat(robot-server,app): extend pipette offset cal to include tip length cal if needed #6641

b-cooper · 2020-09-28T16:18:17Z

Overview

Add functionality to the pipette offset calibration user flow to allow a user to set the tip length
for the pipette-tip combination that they intend to use for the pipette offset calibration. Add
branching and assertion logic to guide the state transitions in the user flow accordingly. Also add
the last of the tip length cal child components to the shared calibration panels and map to them
from the new pipette offset calibration states when appropriate.

Closes #6420

Changelog

Review requests

Try running Pipette Offset calibration without a tip length cal value for the tip rack. Should perform TLC then Pip offset
Try running Pipette Offset calibration with existing tip length cal value for the tip rack. Should behave as it did before
Ensure Tip Length cal standalone flow still functions as expected.

Risk assessment

low risk, code still behind feature flag, though pipette offset and tip length cal flows were both effected

…offset cal

…th cal if needed Add functionality to the pipette offset calibration user flow to allow a user to set the tip length for the pipette-tip combination that they intend to use for the pipette offset calibration. Add branching and assertion logic to guide the state transitions in the user flow accordingly. Also add the last of the tip length cal child components to the shared calibration panels and map to them from the new pipette offset calibration states when appropriate. Closes #6420

sfoster1 · 2020-09-28T16:34:10Z

robot-server/robot_server/robot/calibration/pipette_offset/user_flow.py

+            # recache tip length cal which has just been saved
+            self._has_calibrated_tip_length: bool =\
+                self._get_stored_tip_length_cal() is not None
+            if self._saved_offset_this_session:


only save the data if it's a legal transition

codecov · 2020-09-28T16:53:15Z

Codecov Report

❗ No coverage uploaded for pull request base (edge@953d454). Click here to learn what that means.
The diff coverage is 77.84%.

@@           Coverage Diff           @@
##             edge    #6641   +/-   ##
=======================================
  Coverage        ?   89.03%           
=======================================
  Files           ?      101           
  Lines           ?     4505           
  Branches        ?        0           
=======================================
  Hits            ?     4011           
  Misses          ?      494           
  Partials        ?        0

Impacted Files	Coverage Δ
...t_server/robot/calibration/tip_length/constants.py	`100.00% <ø> (ø)`
...rver/robot/calibration/tip_length/state_machine.py	`92.30% <ø> (ø)`
...ver/robot_server/service/session/models/session.py	`100.00% <ø> (ø)`
...ession/session_types/pipette_offset_calibration.py	`45.45% <33.33%> (ø)`
...rver/robot/calibration/pipette_offset/user_flow.py	`71.51% <60.25%> (ø)`
.../robot/calibration/pipette_offset/state_machine.py	`75.00% <85.71%> (ø)`
...server/robot_server/robot/calibration/constants.py	`97.43% <100.00%> (ø)`
...ot-server/robot_server/robot/calibration/errors.py	`100.00% <100.00%> (ø)`
...rver/robot/calibration/pipette_offset/constants.py	`97.05% <100.00%> (ø)`
..._server/robot/calibration/pipette_offset/models.py	`100.00% <100.00%> (ø)`
... and 104 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 953d454...9f2a39d. Read the comment docs.

sfoster1 · 2020-09-28T16:50:55Z

robot-server/robot_server/robot/calibration/pipette_offset/user_flow.py


+        self._saved_offset_this_session = False
        self._current_state = State.sessionStarted
        self._state_machine = PipetteOffsetCalibrationStateMachine()


Let's try implementing loading one of two different state machines instead of doing checks in the handlers

sfoster1 · 2020-09-28T16:53:50Z

app/src/components/CalibrationPanels/CompleteConfirmation.js

@@ -44,12 +46,19 @@ const contentsBySessionType: { [SessionType]: { headerText: string } } = {
 }

 export function CompleteConfirmation(props: CalibrationPanelProps): React.Node {


If we add an intermediate complete state we can get rid of the bool

…start

sfoster1 · 2020-10-06T12:47:59Z

robot-server/robot_server/robot/calibration/constants.py

+TRASH_WELL = 'A1'
+TRASH_REF_POINT_OFFSET = Point(-57.84, -55, 0)  # offset from center of trash
+
+CAL_BLOCK_SETUP_BY_MOUNT: Dict[Mount, Dict[str, str]] = {


Bit of a nit but the actual lef elements here (the dicts that contain load_name slot and wells should probably be a dataclass IMO

sfoster1 · 2020-10-06T12:52:54Z

robot-server/robot_server/robot/calibration/pipette_offset/user_flow.py

+        self._nozzle_height_at_reference: Optional[float] = None
+
+        self._load_tiprack(tip_rack_def)
+        self._has_calibrated_tip_length: bool =\


So this only happens if there is no stored tip length calibration for this pipette/tiprack combo, right?

I don't think that works with the labware-cal-initiated tip length _re_calibration. If you have a valid TLC that you want to redo, the only way for the system to accomplish that would be to DELETE it using the HTTP endpoint and then start this flow. But if you then decide to skip it, there's no more calibration.

Basically we may want a flag as a create param that is like "force tip length recalibration"

right that makes sense

sfoster1

Happy with the code!

b-cooper · 2020-10-07T15:30:41Z

robot-server/robot_server/robot/calibration/pipette_offset/models.py

@@ -12,6 +12,14 @@ class SessionCreateParams(BaseModel):
        ...,
        description='The mount on which the pipette is attached, left or right'
    )
+    tipLengthLoad: bool = Field(


The name of this variable is a little confusing to me. Is it a boolean of "should Perform TLC prior to pipette offset cal"? If so, maybe something like shouldCalibrateTipLength would be clearer?

b-cooper added 11 commits September 15, 2020 12:48

add new states and transitions to accomodate tlc at beginning of pip …

5ad3818

…offset cal

initialize deck

942abc4

move to reference point in shared utils

d01dcb1

raise if wrong branch attempted

51516a1

create flag unment req method

e9d43e0

update save offset func internals

022985a

iron out merge conflicts

acd2dfa

make move to reference point shared command

45b36be

wire up extended pip offset flow with branching in app

eeba86d

a bit of clean up

8607c78

b-cooper marked this pull request as ready for review September 28, 2020 16:26

b-cooper requested review from a team as code owners September 28, 2020 16:26

b-cooper requested review from shlokamin, a team and ahiuchingau and removed request for a team September 28, 2020 16:26

b-cooper added app Affects the `app` project hmg hardware, motion, and geometry robot server Affects the `robot-server` project labels Sep 28, 2020

sfoster1 requested changes Sep 28, 2020

View reviewed changes

remove log

344dc59

sfoster1 requested changes Sep 28, 2020

View reviewed changes

sfoster1 and others added 3 commits September 29, 2020 09:56

start splitting state machines into the two cases

d91f8c1

Fixup: Tests, add generic state enum and decide which flow to use on …

75bcbf7

…start

Fixup: Tavern test failure

d288172

b-cooper requested a review from a team as a code owner October 5, 2020 19:33

sfoster1 requested changes Oct 6, 2020

View reviewed changes

Fixup: make dataclass and have a load tip length flag

809cdaf

Laura-Danielle requested a review from sfoster1 October 6, 2020 14:58

Fixup: Add app side of calibration

f460c93

sfoster1 approved these changes Oct 6, 2020

View reviewed changes

sfoster1 requested a review from nusrat813 October 6, 2020 20:35

Temporary Commit For QA

820efea

b-cooper commented Oct 7, 2020

View reviewed changes

Laura-Danielle added 4 commits October 7, 2020 13:42

Fixup: logic for has tip length calibration

30d099d

Remove unnecessary logs

ac73dfd

Fixup: JS mock

3c792ae

Actually fix fixture

9f2a39d

Laura-Danielle merged commit 5819f29 into edge Oct 7, 2020

Laura-Danielle deleted the robot-server_tlc-in-pip-offset-cal branch October 7, 2020 19:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(robot-server,app): extend pipette offset cal to include tip length cal if needed #6641

feat(robot-server,app): extend pipette offset cal to include tip length cal if needed #6641

b-cooper commented Sep 28, 2020

sfoster1 Sep 28, 2020

codecov bot commented Sep 28, 2020 •

edited

Loading

sfoster1 Sep 28, 2020

sfoster1 Sep 28, 2020

sfoster1 Oct 6, 2020

sfoster1 Oct 6, 2020

sfoster1 Oct 6, 2020

Laura-Danielle Oct 6, 2020

sfoster1 left a comment

b-cooper Oct 7, 2020

		@@ -44,12 +46,19 @@ const contentsBySessionType: { [SessionType]: { headerText: string } } = {
		}

		export function CompleteConfirmation(props: CalibrationPanelProps): React.Node {

feat(robot-server,app): extend pipette offset cal to include tip length cal if needed #6641

feat(robot-server,app): extend pipette offset cal to include tip length cal if needed #6641

Conversation

b-cooper commented Sep 28, 2020

Overview

Changelog

Review requests

Risk assessment

Choose a reason for hiding this comment

codecov bot commented Sep 28, 2020 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfoster1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Sep 28, 2020 •

edited

Loading