refactor(api): Save full nozzle map configuration and update state store accordingly#14529
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## edge #14529 +/- ##
=======================================
Coverage 67.78% 67.78%
=======================================
Files 2517 2517
Lines 72043 72043
Branches 9274 9274
=======================================
Hits 48837 48837
Misses 20991 20991
Partials 2215 2215
Flags with carried forward coverage won't be shown. Click here to find out more.
|
sfoster1
left a comment
sfoster1
left a comment
There was a problem hiding this comment.
This looks good as long as it has the right serializability - will this get persisted?
| self._state.nozzle_configuration_by_id[ | ||
| private_result.pipette_id | ||
| ] = config.nozzle_map |
There was a problem hiding this comment.
Can we make nozzle_configuration_by_id values non-Optional now?
There was a problem hiding this comment.
Discussed with @Laura-Danielle. We'll defer this change. Might want to leave a # TODO explaining what needs to happen, though.
I think we're good. I'm not seeing any modification of Pydantic |
| self._state.nozzle_configuration_by_id[ | ||
| private_result.pipette_id | ||
| ] = config.nozzle_map |
There was a problem hiding this comment.
Discussed with @Laura-Danielle. We'll defer this change. Might want to leave a # TODO explaining what needs to happen, though.
sanni-t
left a comment
sanni-t
left a comment
There was a problem hiding this comment.
LGTM! Just the comment about better test assertions
| tip_configuration_lookup_table=result1.tip_configuration_lookup_table, | ||
| nominal_tip_overlap=result1.nominal_tip_overlap, | ||
| back_left_nozzle_offset=Point(x=-8.0, y=-22.0, z=-259.15), | ||
| front_right_nozzle_offset=Point(x=-8.0, y=-22.0, z=-259.15), | ||
| nozzle_map=result1.nozzle_map, |
There was a problem hiding this comment.
Hmm, just realizing these aren't great asserts. They're just comparing an item with itself; like result1.nozzle_map == result1.nozzle_map. We should check it against the actual expected nozzle map.
| nominal_tip_overlap=result2.nominal_tip_overlap, | ||
| back_left_nozzle_offset=Point(x=-8.0, y=-22.0, z=-259.15), | ||
| front_right_nozzle_offset=Point(x=-8.0, y=-22.0, z=-259.15), | ||
| nozzle_map=result2.nozzle_map, |
`nozzle_map` cannot be `None` since #14529.
`nozzle_map` cannot be `None` since #14529.
4 participants
Overview
To make a few operations easier in protocol engine, we will keep the nozzle map in state always. I will think about this further in RSS-443, but for now it should unblock other partial tip work.
Test Plan
General sanity check that partial tip still works and regular protocols operate as normal.
Changelog
LoadedStaticPipetteDataReview requests
Check that I didn't miss anything to update.
Risk assessment
Low/medium. Currently not using this information anywhere aside from back left/right values which will remain the same. Tests only updated due to new helper function pulling in mock data.