refactor(api): Port tip consumption to StateUpdate#16469
Merged
SyntaxColoring merged 9 commits intoedgefrom Oct 11, 2024
Merged
refactor(api): Port tip consumption to StateUpdate#16469SyntaxColoring merged 9 commits intoedgefrom
SyntaxColoring merged 9 commits intoedgefrom
Conversation
This was left over from an earlier refactor.
This makes it harder to forget to update one of them.
`nozzle_map` cannot be `None` since #14529.
To disambiguate with tip rack tip state.
SyntaxColoring
commented
Oct 11, 2024
Comment on lines
+32
to
+41
| # todo(mm, 2024-10-10): This info is duplicated between here and PipetteState because | ||
| # TipStore is using it to compute which tips a PickUpTip removes from the tip rack, | ||
| # given the pipette's current nozzle map. We could avoid this duplication by moving the | ||
| # computation to TipView, calling it from PickUpTipImplementation, and passing the | ||
| # precomputed list of wells to TipStore. | ||
| @dataclass | ||
| class _PipetteInfo: | ||
| channels: int | ||
| active_channels: int | ||
| nozzle_map: NozzleMap |
Contributor
Author
There was a problem hiding this comment.
Does this seem like a reasonable plan?
sfoster1
approved these changes
Oct 11, 2024
Member
sfoster1
left a comment
sfoster1
left a comment
There was a problem hiding this comment.
Looks good, and agree with the path forward
Comment on lines
+32
to
+41
| # todo(mm, 2024-10-10): This info is duplicated between here and PipetteState because | ||
| # TipStore is using it to compute which tips a PickUpTip removes from the tip rack, | ||
| # given the pipette's current nozzle map. We could avoid this duplication by moving the | ||
| # computation to TipView, calling it from PickUpTipImplementation, and passing the | ||
| # precomputed list of wells to TipStore. | ||
| @dataclass | ||
| class _PipetteInfo: | ||
| channels: int | ||
| active_channels: int | ||
| nozzle_map: NozzleMap |
sanni-t
approved these changes
Oct 11, 2024
Comment on lines
-82
to
-93
| if nozzle_map: | ||
| self._state.active_channels_by_pipette_id[ | ||
| pipette_id | ||
| ] = nozzle_map.tip_count | ||
| self._state.nozzle_map_by_pipette_id[pipette_id] = nozzle_map | ||
| else: | ||
| self._state.active_channels_by_pipette_id[ | ||
| pipette_id | ||
| ] = self._state.channels_by_pipette_id[pipette_id] | ||
|
|
||
| elif isinstance(action, FailCommandAction): | ||
| self._handle_failed_command(action) |
Member
There was a problem hiding this comment.
Oh nice! Is NozzleMap always available now? I think there was some state that had nozzle map optional even though I think the map was indeed available.
Contributor
Author
There was a problem hiding this comment.
Oh nice! Is
NozzleMapalways available now?
Yep, for the purposes of TipStore, at least.
I think there was some state that had nozzle map optional even though I think the map was indeed available.
Good memory. I think that's this, over in PipetteStore. I'll play around with that and see if we can simplify it now. #14529 (comment)
1 task
SyntaxColoring
added a commit
that referenced
this pull request
Apr 30, 2025
…18207) ## Overview This closes RQA-4058 (protocol analysis hanging if `pickUpTip` is given a bad labware definition), and resolves some duplication we noticed a little while ago (#16469 (comment)). ## Details When a `pickUpTip` command completes, `TipStore` marks the picked-up tips as "used." Before this PR, that worked by `PickUpTipImplementation` passing along just the single "target" tip, like A1; `TipStore` then internally expanded that into the full list of actually-picked-up tips, like A1+B1+⋯+H1, for an 8-channel pickup. The choice to do the expansion inside `TipStore` caused two problems: 1. Error handling. If it happens inside `TipStore`, and something goes wrong and raises an exception, then we've raised an exception from inside an `opentrons.protocol_engine` *state update.* We are not in a good position to handle that: it was an early architectural of `opentrons.protocol_engine` that all the real work ought to happen in command implementations, and that state updates ought to be dead-simple and always successful. (We can and should do something about that premise, but that's beyond the scope of this PR.) This is what caused RQA-4058. 2. Duplication. For `TipStore` to automatically expand the target well into the full list of actually-picked-up tips, it needed to know details about the pipette's current configuration. That was already part of `PipetteStore`'s job;`TipStore` was duplicating it. This PR tries to solve both of those by having `PickUpTipImplementation` give `TipStore` a full precomputed list of tips. `PickUpTipImplementation` produces that precomputed list with help from new functions in `PipetteView` and `TipView`. Pipette concerns like nozzle maps are expunged from `TipStore`. Overall, there's no new logic here—just shifting things around and changing what's running where.
ddcc4
pushed a commit
that referenced
this pull request
May 16, 2025
…18207) ## Overview This closes RQA-4058 (protocol analysis hanging if `pickUpTip` is given a bad labware definition), and resolves some duplication we noticed a little while ago (#16469 (comment)). ## Details When a `pickUpTip` command completes, `TipStore` marks the picked-up tips as "used." Before this PR, that worked by `PickUpTipImplementation` passing along just the single "target" tip, like A1; `TipStore` then internally expanded that into the full list of actually-picked-up tips, like A1+B1+⋯+H1, for an 8-channel pickup. The choice to do the expansion inside `TipStore` caused two problems: 1. Error handling. If it happens inside `TipStore`, and something goes wrong and raises an exception, then we've raised an exception from inside an `opentrons.protocol_engine` *state update.* We are not in a good position to handle that: it was an early architectural of `opentrons.protocol_engine` that all the real work ought to happen in command implementations, and that state updates ought to be dead-simple and always successful. (We can and should do something about that premise, but that's beyond the scope of this PR.) This is what caused RQA-4058. 2. Duplication. For `TipStore` to automatically expand the target well into the full list of actually-picked-up tips, it needed to know details about the pipette's current configuration. That was already part of `PipetteStore`'s job;`TipStore` was duplicating it. This PR tries to solve both of those by having `PickUpTipImplementation` give `TipStore` a full precomputed list of tips. `PickUpTipImplementation` produces that precomputed list with help from new functions in `PipetteView` and `TipView`. Pipette concerns like nozzle maps are expunged from `TipStore`. Overall, there's no new logic here—just shifting things around and changing what's running where.
ddcc4
pushed a commit
that referenced
this pull request
May 16, 2025
…18207) ## Overview This closes RQA-4058 (protocol analysis hanging if `pickUpTip` is given a bad labware definition), and resolves some duplication we noticed a little while ago (#16469 (comment)). ## Details When a `pickUpTip` command completes, `TipStore` marks the picked-up tips as "used." Before this PR, that worked by `PickUpTipImplementation` passing along just the single "target" tip, like A1; `TipStore` then internally expanded that into the full list of actually-picked-up tips, like A1+B1+⋯+H1, for an 8-channel pickup. The choice to do the expansion inside `TipStore` caused two problems: 1. Error handling. If it happens inside `TipStore`, and something goes wrong and raises an exception, then we've raised an exception from inside an `opentrons.protocol_engine` *state update.* We are not in a good position to handle that: it was an early architectural of `opentrons.protocol_engine` that all the real work ought to happen in command implementations, and that state updates ought to be dead-simple and always successful. (We can and should do something about that premise, but that's beyond the scope of this PR.) This is what caused RQA-4058. 2. Duplication. For `TipStore` to automatically expand the target well into the full list of actually-picked-up tips, it needed to know details about the pipette's current configuration. That was already part of `PipetteStore`'s job;`TipStore` was duplicating it. This PR tries to solve both of those by having `PickUpTipImplementation` give `TipStore` a full precomputed list of tips. `PickUpTipImplementation` produces that precomputed list with help from new functions in `PipetteView` and `TipView`. Pipette concerns like nozzle maps are expunged from `TipStore`. Overall, there's no new logic here—just shifting things around and changing what's running where.
ddcc4
pushed a commit
that referenced
this pull request
May 16, 2025
…18207) ## Overview This closes RQA-4058 (protocol analysis hanging if `pickUpTip` is given a bad labware definition), and resolves some duplication we noticed a little while ago (#16469 (comment)). ## Details When a `pickUpTip` command completes, `TipStore` marks the picked-up tips as "used." Before this PR, that worked by `PickUpTipImplementation` passing along just the single "target" tip, like A1; `TipStore` then internally expanded that into the full list of actually-picked-up tips, like A1+B1+⋯+H1, for an 8-channel pickup. The choice to do the expansion inside `TipStore` caused two problems: 1. Error handling. If it happens inside `TipStore`, and something goes wrong and raises an exception, then we've raised an exception from inside an `opentrons.protocol_engine` *state update.* We are not in a good position to handle that: it was an early architectural of `opentrons.protocol_engine` that all the real work ought to happen in command implementations, and that state updates ought to be dead-simple and always successful. (We can and should do something about that premise, but that's beyond the scope of this PR.) This is what caused RQA-4058. 2. Duplication. For `TipStore` to automatically expand the target well into the full list of actually-picked-up tips, it needed to know details about the pipette's current configuration. That was already part of `PipetteStore`'s job;`TipStore` was duplicating it. This PR tries to solve both of those by having `PickUpTipImplementation` give `TipStore` a full precomputed list of tips. `PickUpTipImplementation` produces that precomputed list with help from new functions in `PipetteView` and `TipView`. Pipette concerns like nozzle maps are expunged from `TipStore`. Overall, there's no new logic here—just shifting things around and changing what's running where.
ddcc4
pushed a commit
that referenced
this pull request
May 16, 2025
…18207) ## Overview This closes RQA-4058 (protocol analysis hanging if `pickUpTip` is given a bad labware definition), and resolves some duplication we noticed a little while ago (#16469 (comment)). ## Details When a `pickUpTip` command completes, `TipStore` marks the picked-up tips as "used." Before this PR, that worked by `PickUpTipImplementation` passing along just the single "target" tip, like A1; `TipStore` then internally expanded that into the full list of actually-picked-up tips, like A1+B1+⋯+H1, for an 8-channel pickup. The choice to do the expansion inside `TipStore` caused two problems: 1. Error handling. If it happens inside `TipStore`, and something goes wrong and raises an exception, then we've raised an exception from inside an `opentrons.protocol_engine` *state update.* We are not in a good position to handle that: it was an early architectural of `opentrons.protocol_engine` that all the real work ought to happen in command implementations, and that state updates ought to be dead-simple and always successful. (We can and should do something about that premise, but that's beyond the scope of this PR.) This is what caused RQA-4058. 2. Duplication. For `TipStore` to automatically expand the target well into the full list of actually-picked-up tips, it needed to know details about the pipette's current configuration. That was already part of `PipetteStore`'s job;`TipStore` was duplicating it. This PR tries to solve both of those by having `PickUpTipImplementation` give `TipStore` a full precomputed list of tips. `PickUpTipImplementation` produces that precomputed list with help from new functions in `PipetteView` and `TipView`. Pipette concerns like nozzle maps are expunged from `TipStore`. Overall, there's no new logic here—just shifting things around and changing what's running where.
ddcc4
pushed a commit
that referenced
this pull request
May 16, 2025
…18207) ## Overview This closes RQA-4058 (protocol analysis hanging if `pickUpTip` is given a bad labware definition), and resolves some duplication we noticed a little while ago (#16469 (comment)). ## Details When a `pickUpTip` command completes, `TipStore` marks the picked-up tips as "used." Before this PR, that worked by `PickUpTipImplementation` passing along just the single "target" tip, like A1; `TipStore` then internally expanded that into the full list of actually-picked-up tips, like A1+B1+⋯+H1, for an 8-channel pickup. The choice to do the expansion inside `TipStore` caused two problems: 1. Error handling. If it happens inside `TipStore`, and something goes wrong and raises an exception, then we've raised an exception from inside an `opentrons.protocol_engine` *state update.* We are not in a good position to handle that: it was an early architectural of `opentrons.protocol_engine` that all the real work ought to happen in command implementations, and that state updates ought to be dead-simple and always successful. (We can and should do something about that premise, but that's beyond the scope of this PR.) This is what caused RQA-4058. 2. Duplication. For `TipStore` to automatically expand the target well into the full list of actually-picked-up tips, it needed to know details about the pipette's current configuration. That was already part of `PipetteStore`'s job;`TipStore` was duplicating it. This PR tries to solve both of those by having `PickUpTipImplementation` give `TipStore` a full precomputed list of tips. `PickUpTipImplementation` produces that precomputed list with help from new functions in `PipetteView` and `TipView`. Pipette concerns like nozzle maps are expunged from `TipStore`. Overall, there's no new logic here—just shifting things around and changing what's running where.
ddcc4
pushed a commit
that referenced
this pull request
May 16, 2025
…18207) ## Overview This closes RQA-4058 (protocol analysis hanging if `pickUpTip` is given a bad labware definition), and resolves some duplication we noticed a little while ago (#16469 (comment)). ## Details When a `pickUpTip` command completes, `TipStore` marks the picked-up tips as "used." Before this PR, that worked by `PickUpTipImplementation` passing along just the single "target" tip, like A1; `TipStore` then internally expanded that into the full list of actually-picked-up tips, like A1+B1+⋯+H1, for an 8-channel pickup. The choice to do the expansion inside `TipStore` caused two problems: 1. Error handling. If it happens inside `TipStore`, and something goes wrong and raises an exception, then we've raised an exception from inside an `opentrons.protocol_engine` *state update.* We are not in a good position to handle that: it was an early architectural of `opentrons.protocol_engine` that all the real work ought to happen in command implementations, and that state updates ought to be dead-simple and always successful. (We can and should do something about that premise, but that's beyond the scope of this PR.) This is what caused RQA-4058. 2. Duplication. For `TipStore` to automatically expand the target well into the full list of actually-picked-up tips, it needed to know details about the pipette's current configuration. That was already part of `PipetteStore`'s job;`TipStore` was duplicating it. This PR tries to solve both of those by having `PickUpTipImplementation` give `TipStore` a full precomputed list of tips. `PickUpTipImplementation` produces that precomputed list with help from new functions in `PipetteView` and `TipView`. Pipette concerns like nozzle maps are expunged from `TipStore`. Overall, there's no new logic here—just shifting things around and changing what's running where.
ddcc4
pushed a commit
that referenced
this pull request
May 17, 2025
…18207) ## Overview This closes RQA-4058 (protocol analysis hanging if `pickUpTip` is given a bad labware definition), and resolves some duplication we noticed a little while ago (#16469 (comment)). ## Details When a `pickUpTip` command completes, `TipStore` marks the picked-up tips as "used." Before this PR, that worked by `PickUpTipImplementation` passing along just the single "target" tip, like A1; `TipStore` then internally expanded that into the full list of actually-picked-up tips, like A1+B1+⋯+H1, for an 8-channel pickup. The choice to do the expansion inside `TipStore` caused two problems: 1. Error handling. If it happens inside `TipStore`, and something goes wrong and raises an exception, then we've raised an exception from inside an `opentrons.protocol_engine` *state update.* We are not in a good position to handle that: it was an early architectural of `opentrons.protocol_engine` that all the real work ought to happen in command implementations, and that state updates ought to be dead-simple and always successful. (We can and should do something about that premise, but that's beyond the scope of this PR.) This is what caused RQA-4058. 2. Duplication. For `TipStore` to automatically expand the target well into the full list of actually-picked-up tips, it needed to know details about the pipette's current configuration. That was already part of `PipetteStore`'s job;`TipStore` was duplicating it. This PR tries to solve both of those by having `PickUpTipImplementation` give `TipStore` a full precomputed list of tips. `PickUpTipImplementation` produces that precomputed list with help from new functions in `PipetteView` and `TipView`. Pipette concerns like nozzle maps are expunged from `TipStore`. Overall, there's no new logic here—just shifting things around and changing what's running where.
ddcc4
pushed a commit
that referenced
this pull request
May 17, 2025
…18207) ## Overview This closes RQA-4058 (protocol analysis hanging if `pickUpTip` is given a bad labware definition), and resolves some duplication we noticed a little while ago (#16469 (comment)). ## Details When a `pickUpTip` command completes, `TipStore` marks the picked-up tips as "used." Before this PR, that worked by `PickUpTipImplementation` passing along just the single "target" tip, like A1; `TipStore` then internally expanded that into the full list of actually-picked-up tips, like A1+B1+⋯+H1, for an 8-channel pickup. The choice to do the expansion inside `TipStore` caused two problems: 1. Error handling. If it happens inside `TipStore`, and something goes wrong and raises an exception, then we've raised an exception from inside an `opentrons.protocol_engine` *state update.* We are not in a good position to handle that: it was an early architectural of `opentrons.protocol_engine` that all the real work ought to happen in command implementations, and that state updates ought to be dead-simple and always successful. (We can and should do something about that premise, but that's beyond the scope of this PR.) This is what caused RQA-4058. 2. Duplication. For `TipStore` to automatically expand the target well into the full list of actually-picked-up tips, it needed to know details about the pipette's current configuration. That was already part of `PipetteStore`'s job;`TipStore` was duplicating it. This PR tries to solve both of those by having `PickUpTipImplementation` give `TipStore` a full precomputed list of tips. `PickUpTipImplementation` produces that precomputed list with help from new functions in `PipetteView` and `TipView`. Pipette concerns like nozzle maps are expunged from `TipStore`. Overall, there's no new logic here—just shifting things around and changing what's running where.
ddcc4
pushed a commit
that referenced
this pull request
May 17, 2025
…18207) ## Overview This closes RQA-4058 (protocol analysis hanging if `pickUpTip` is given a bad labware definition), and resolves some duplication we noticed a little while ago (#16469 (comment)). ## Details When a `pickUpTip` command completes, `TipStore` marks the picked-up tips as "used." Before this PR, that worked by `PickUpTipImplementation` passing along just the single "target" tip, like A1; `TipStore` then internally expanded that into the full list of actually-picked-up tips, like A1+B1+⋯+H1, for an 8-channel pickup. The choice to do the expansion inside `TipStore` caused two problems: 1. Error handling. If it happens inside `TipStore`, and something goes wrong and raises an exception, then we've raised an exception from inside an `opentrons.protocol_engine` *state update.* We are not in a good position to handle that: it was an early architectural of `opentrons.protocol_engine` that all the real work ought to happen in command implementations, and that state updates ought to be dead-simple and always successful. (We can and should do something about that premise, but that's beyond the scope of this PR.) This is what caused RQA-4058. 2. Duplication. For `TipStore` to automatically expand the target well into the full list of actually-picked-up tips, it needed to know details about the pipette's current configuration. That was already part of `PipetteStore`'s job;`TipStore` was duplicating it. This PR tries to solve both of those by having `PickUpTipImplementation` give `TipStore` a full precomputed list of tips. `PickUpTipImplementation` produces that precomputed list with help from new functions in `PipetteView` and `TipView`. Pipette concerns like nozzle maps are expunged from `TipStore`. Overall, there's no new logic here—just shifting things around and changing what's running where.
ddcc4
pushed a commit
that referenced
this pull request
May 17, 2025
…18207) ## Overview This closes RQA-4058 (protocol analysis hanging if `pickUpTip` is given a bad labware definition), and resolves some duplication we noticed a little while ago (#16469 (comment)). ## Details When a `pickUpTip` command completes, `TipStore` marks the picked-up tips as "used." Before this PR, that worked by `PickUpTipImplementation` passing along just the single "target" tip, like A1; `TipStore` then internally expanded that into the full list of actually-picked-up tips, like A1+B1+⋯+H1, for an 8-channel pickup. The choice to do the expansion inside `TipStore` caused two problems: 1. Error handling. If it happens inside `TipStore`, and something goes wrong and raises an exception, then we've raised an exception from inside an `opentrons.protocol_engine` *state update.* We are not in a good position to handle that: it was an early architectural of `opentrons.protocol_engine` that all the real work ought to happen in command implementations, and that state updates ought to be dead-simple and always successful. (We can and should do something about that premise, but that's beyond the scope of this PR.) This is what caused RQA-4058. 2. Duplication. For `TipStore` to automatically expand the target well into the full list of actually-picked-up tips, it needed to know details about the pipette's current configuration. That was already part of `PipetteStore`'s job;`TipStore` was duplicating it. This PR tries to solve both of those by having `PickUpTipImplementation` give `TipStore` a full precomputed list of tips. `PickUpTipImplementation` produces that precomputed list with help from new functions in `PipetteView` and `TipView`. Pipette concerns like nozzle maps are expunged from `TipStore`. Overall, there's no new logic here—just shifting things around and changing what's running where. (cherry picked from commit 0855bd1)
ddcc4
pushed a commit
that referenced
this pull request
May 17, 2025
…18207) ## Overview This closes RQA-4058 (protocol analysis hanging if `pickUpTip` is given a bad labware definition), and resolves some duplication we noticed a little while ago (#16469 (comment)). ## Details When a `pickUpTip` command completes, `TipStore` marks the picked-up tips as "used." Before this PR, that worked by `PickUpTipImplementation` passing along just the single "target" tip, like A1; `TipStore` then internally expanded that into the full list of actually-picked-up tips, like A1+B1+⋯+H1, for an 8-channel pickup. The choice to do the expansion inside `TipStore` caused two problems: 1. Error handling. If it happens inside `TipStore`, and something goes wrong and raises an exception, then we've raised an exception from inside an `opentrons.protocol_engine` *state update.* We are not in a good position to handle that: it was an early architectural of `opentrons.protocol_engine` that all the real work ought to happen in command implementations, and that state updates ought to be dead-simple and always successful. (We can and should do something about that premise, but that's beyond the scope of this PR.) This is what caused RQA-4058. 2. Duplication. For `TipStore` to automatically expand the target well into the full list of actually-picked-up tips, it needed to know details about the pipette's current configuration. That was already part of `PipetteStore`'s job;`TipStore` was duplicating it. This PR tries to solve both of those by having `PickUpTipImplementation` give `TipStore` a full precomputed list of tips. `PickUpTipImplementation` produces that precomputed list with help from new functions in `PipetteView` and `TipView`. Pipette concerns like nozzle maps are expunged from `TipStore`. Overall, there's no new logic here—just shifting things around and changing what's running where. (cherry picked from commit 0855bd1)
ddcc4
pushed a commit
that referenced
this pull request
May 17, 2025
…18207) ## Overview This closes RQA-4058 (protocol analysis hanging if `pickUpTip` is given a bad labware definition), and resolves some duplication we noticed a little while ago (#16469 (comment)). ## Details When a `pickUpTip` command completes, `TipStore` marks the picked-up tips as "used." Before this PR, that worked by `PickUpTipImplementation` passing along just the single "target" tip, like A1; `TipStore` then internally expanded that into the full list of actually-picked-up tips, like A1+B1+⋯+H1, for an 8-channel pickup. The choice to do the expansion inside `TipStore` caused two problems: 1. Error handling. If it happens inside `TipStore`, and something goes wrong and raises an exception, then we've raised an exception from inside an `opentrons.protocol_engine` *state update.* We are not in a good position to handle that: it was an early architectural of `opentrons.protocol_engine` that all the real work ought to happen in command implementations, and that state updates ought to be dead-simple and always successful. (We can and should do something about that premise, but that's beyond the scope of this PR.) This is what caused RQA-4058. 2. Duplication. For `TipStore` to automatically expand the target well into the full list of actually-picked-up tips, it needed to know details about the pipette's current configuration. That was already part of `PipetteStore`'s job;`TipStore` was duplicating it. This PR tries to solve both of those by having `PickUpTipImplementation` give `TipStore` a full precomputed list of tips. `PickUpTipImplementation` produces that precomputed list with help from new functions in `PipetteView` and `TipView`. Pipette concerns like nozzle maps are expunged from `TipStore`. Overall, there's no new logic here—just shifting things around and changing what's running where. (cherry picked from commit 0855bd1)
ddcc4
pushed a commit
that referenced
this pull request
May 17, 2025
…18207) ## Overview This closes RQA-4058 (protocol analysis hanging if `pickUpTip` is given a bad labware definition), and resolves some duplication we noticed a little while ago (#16469 (comment)). ## Details When a `pickUpTip` command completes, `TipStore` marks the picked-up tips as "used." Before this PR, that worked by `PickUpTipImplementation` passing along just the single "target" tip, like A1; `TipStore` then internally expanded that into the full list of actually-picked-up tips, like A1+B1+⋯+H1, for an 8-channel pickup. The choice to do the expansion inside `TipStore` caused two problems: 1. Error handling. If it happens inside `TipStore`, and something goes wrong and raises an exception, then we've raised an exception from inside an `opentrons.protocol_engine` *state update.* We are not in a good position to handle that: it was an early architectural of `opentrons.protocol_engine` that all the real work ought to happen in command implementations, and that state updates ought to be dead-simple and always successful. (We can and should do something about that premise, but that's beyond the scope of this PR.) This is what caused RQA-4058. 2. Duplication. For `TipStore` to automatically expand the target well into the full list of actually-picked-up tips, it needed to know details about the pipette's current configuration. That was already part of `PipetteStore`'s job;`TipStore` was duplicating it. This PR tries to solve both of those by having `PickUpTipImplementation` give `TipStore` a full precomputed list of tips. `PickUpTipImplementation` produces that precomputed list with help from new functions in `PipetteView` and `TipView`. Pipette concerns like nozzle maps are expunged from `TipStore`. Overall, there's no new logic here—just shifting things around and changing what's running where. (cherry picked from commit 0855bd1)
ddcc4
pushed a commit
that referenced
this pull request
May 17, 2025
…18207) ## Overview This closes RQA-4058 (protocol analysis hanging if `pickUpTip` is given a bad labware definition), and resolves some duplication we noticed a little while ago (#16469 (comment)). ## Details When a `pickUpTip` command completes, `TipStore` marks the picked-up tips as "used." Before this PR, that worked by `PickUpTipImplementation` passing along just the single "target" tip, like A1; `TipStore` then internally expanded that into the full list of actually-picked-up tips, like A1+B1+⋯+H1, for an 8-channel pickup. The choice to do the expansion inside `TipStore` caused two problems: 1. Error handling. If it happens inside `TipStore`, and something goes wrong and raises an exception, then we've raised an exception from inside an `opentrons.protocol_engine` *state update.* We are not in a good position to handle that: it was an early architectural of `opentrons.protocol_engine` that all the real work ought to happen in command implementations, and that state updates ought to be dead-simple and always successful. (We can and should do something about that premise, but that's beyond the scope of this PR.) This is what caused RQA-4058. 2. Duplication. For `TipStore` to automatically expand the target well into the full list of actually-picked-up tips, it needed to know details about the pipette's current configuration. That was already part of `PipetteStore`'s job;`TipStore` was duplicating it. This PR tries to solve both of those by having `PickUpTipImplementation` give `TipStore` a full precomputed list of tips. `PickUpTipImplementation` produces that precomputed list with help from new functions in `PipetteView` and `TipView`. Pipette concerns like nozzle maps are expunged from `TipStore`. Overall, there's no new logic here—just shifting things around and changing what's running where. (cherry picked from commit 0855bd1)
ddcc4
pushed a commit
that referenced
this pull request
May 17, 2025
…18207) ## Overview This closes RQA-4058 (protocol analysis hanging if `pickUpTip` is given a bad labware definition), and resolves some duplication we noticed a little while ago (#16469 (comment)). ## Details When a `pickUpTip` command completes, `TipStore` marks the picked-up tips as "used." Before this PR, that worked by `PickUpTipImplementation` passing along just the single "target" tip, like A1; `TipStore` then internally expanded that into the full list of actually-picked-up tips, like A1+B1+⋯+H1, for an 8-channel pickup. The choice to do the expansion inside `TipStore` caused two problems: 1. Error handling. If it happens inside `TipStore`, and something goes wrong and raises an exception, then we've raised an exception from inside an `opentrons.protocol_engine` *state update.* We are not in a good position to handle that: it was an early architectural of `opentrons.protocol_engine` that all the real work ought to happen in command implementations, and that state updates ought to be dead-simple and always successful. (We can and should do something about that premise, but that's beyond the scope of this PR.) This is what caused RQA-4058. 2. Duplication. For `TipStore` to automatically expand the target well into the full list of actually-picked-up tips, it needed to know details about the pipette's current configuration. That was already part of `PipetteStore`'s job;`TipStore` was duplicating it. This PR tries to solve both of those by having `PickUpTipImplementation` give `TipStore` a full precomputed list of tips. `PickUpTipImplementation` produces that precomputed list with help from new functions in `PipetteView` and `TipView`. Pipette concerns like nozzle maps are expunged from `TipStore`. Overall, there's no new logic here—just shifting things around and changing what's running where. (cherry picked from commit 0855bd1)
ddcc4
pushed a commit
that referenced
this pull request
May 19, 2025
…18207) ## Overview This closes RQA-4058 (protocol analysis hanging if `pickUpTip` is given a bad labware definition), and resolves some duplication we noticed a little while ago (#16469 (comment)). ## Details When a `pickUpTip` command completes, `TipStore` marks the picked-up tips as "used." Before this PR, that worked by `PickUpTipImplementation` passing along just the single "target" tip, like A1; `TipStore` then internally expanded that into the full list of actually-picked-up tips, like A1+B1+⋯+H1, for an 8-channel pickup. The choice to do the expansion inside `TipStore` caused two problems: 1. Error handling. If it happens inside `TipStore`, and something goes wrong and raises an exception, then we've raised an exception from inside an `opentrons.protocol_engine` *state update.* We are not in a good position to handle that: it was an early architectural of `opentrons.protocol_engine` that all the real work ought to happen in command implementations, and that state updates ought to be dead-simple and always successful. (We can and should do something about that premise, but that's beyond the scope of this PR.) This is what caused RQA-4058. 2. Duplication. For `TipStore` to automatically expand the target well into the full list of actually-picked-up tips, it needed to know details about the pipette's current configuration. That was already part of `PipetteStore`'s job;`TipStore` was duplicating it. This PR tries to solve both of those by having `PickUpTipImplementation` give `TipStore` a full precomputed list of tips. `PickUpTipImplementation` produces that precomputed list with help from new functions in `PipetteView` and `TipView`. Pipette concerns like nozzle maps are expunged from `TipStore`. Overall, there's no new logic here—just shifting things around and changing what's running where. (cherry picked from commit 0855bd1)
ddcc4
pushed a commit
that referenced
this pull request
May 19, 2025
…18207) ## Overview This closes RQA-4058 (protocol analysis hanging if `pickUpTip` is given a bad labware definition), and resolves some duplication we noticed a little while ago (#16469 (comment)). ## Details When a `pickUpTip` command completes, `TipStore` marks the picked-up tips as "used." Before this PR, that worked by `PickUpTipImplementation` passing along just the single "target" tip, like A1; `TipStore` then internally expanded that into the full list of actually-picked-up tips, like A1+B1+⋯+H1, for an 8-channel pickup. The choice to do the expansion inside `TipStore` caused two problems: 1. Error handling. If it happens inside `TipStore`, and something goes wrong and raises an exception, then we've raised an exception from inside an `opentrons.protocol_engine` *state update.* We are not in a good position to handle that: it was an early architectural of `opentrons.protocol_engine` that all the real work ought to happen in command implementations, and that state updates ought to be dead-simple and always successful. (We can and should do something about that premise, but that's beyond the scope of this PR.) This is what caused RQA-4058. 2. Duplication. For `TipStore` to automatically expand the target well into the full list of actually-picked-up tips, it needed to know details about the pipette's current configuration. That was already part of `PipetteStore`'s job;`TipStore` was duplicating it. This PR tries to solve both of those by having `PickUpTipImplementation` give `TipStore` a full precomputed list of tips. `PickUpTipImplementation` produces that precomputed list with help from new functions in `PipetteView` and `TipView`. Pipette concerns like nozzle maps are expunged from `TipStore`. Overall, there's no new logic here—just shifting things around and changing what's running where. (cherry picked from commit 0855bd1)
ddcc4
pushed a commit
that referenced
this pull request
May 19, 2025
…18207) ## Overview This closes RQA-4058 (protocol analysis hanging if `pickUpTip` is given a bad labware definition), and resolves some duplication we noticed a little while ago (#16469 (comment)). ## Details When a `pickUpTip` command completes, `TipStore` marks the picked-up tips as "used." Before this PR, that worked by `PickUpTipImplementation` passing along just the single "target" tip, like A1; `TipStore` then internally expanded that into the full list of actually-picked-up tips, like A1+B1+⋯+H1, for an 8-channel pickup. The choice to do the expansion inside `TipStore` caused two problems: 1. Error handling. If it happens inside `TipStore`, and something goes wrong and raises an exception, then we've raised an exception from inside an `opentrons.protocol_engine` *state update.* We are not in a good position to handle that: it was an early architectural of `opentrons.protocol_engine` that all the real work ought to happen in command implementations, and that state updates ought to be dead-simple and always successful. (We can and should do something about that premise, but that's beyond the scope of this PR.) This is what caused RQA-4058. 2. Duplication. For `TipStore` to automatically expand the target well into the full list of actually-picked-up tips, it needed to know details about the pipette's current configuration. That was already part of `PipetteStore`'s job;`TipStore` was duplicating it. This PR tries to solve both of those by having `PickUpTipImplementation` give `TipStore` a full precomputed list of tips. `PickUpTipImplementation` produces that precomputed list with help from new functions in `PipetteView` and `TipView`. Pipette concerns like nozzle maps are expunged from `TipStore`. Overall, there's no new logic here—just shifting things around and changing what's running where. (cherry picked from commit 0855bd1)
ddcc4
pushed a commit
that referenced
this pull request
May 20, 2025
…18207) ## Overview This closes RQA-4058 (protocol analysis hanging if `pickUpTip` is given a bad labware definition), and resolves some duplication we noticed a little while ago (#16469 (comment)). ## Details When a `pickUpTip` command completes, `TipStore` marks the picked-up tips as "used." Before this PR, that worked by `PickUpTipImplementation` passing along just the single "target" tip, like A1; `TipStore` then internally expanded that into the full list of actually-picked-up tips, like A1+B1+⋯+H1, for an 8-channel pickup. The choice to do the expansion inside `TipStore` caused two problems: 1. Error handling. If it happens inside `TipStore`, and something goes wrong and raises an exception, then we've raised an exception from inside an `opentrons.protocol_engine` *state update.* We are not in a good position to handle that: it was an early architectural of `opentrons.protocol_engine` that all the real work ought to happen in command implementations, and that state updates ought to be dead-simple and always successful. (We can and should do something about that premise, but that's beyond the scope of this PR.) This is what caused RQA-4058. 2. Duplication. For `TipStore` to automatically expand the target well into the full list of actually-picked-up tips, it needed to know details about the pipette's current configuration. That was already part of `PipetteStore`'s job;`TipStore` was duplicating it. This PR tries to solve both of those by having `PickUpTipImplementation` give `TipStore` a full precomputed list of tips. `PickUpTipImplementation` produces that precomputed list with help from new functions in `PipetteView` and `TipView`. Pipette concerns like nozzle maps are expunged from `TipStore`. Overall, there's no new logic here—just shifting things around and changing what's running where. (cherry picked from commit 0855bd1)
ddcc4
pushed a commit
that referenced
this pull request
May 20, 2025
…18207) ## Overview This closes RQA-4058 (protocol analysis hanging if `pickUpTip` is given a bad labware definition), and resolves some duplication we noticed a little while ago (#16469 (comment)). ## Details When a `pickUpTip` command completes, `TipStore` marks the picked-up tips as "used." Before this PR, that worked by `PickUpTipImplementation` passing along just the single "target" tip, like A1; `TipStore` then internally expanded that into the full list of actually-picked-up tips, like A1+B1+⋯+H1, for an 8-channel pickup. The choice to do the expansion inside `TipStore` caused two problems: 1. Error handling. If it happens inside `TipStore`, and something goes wrong and raises an exception, then we've raised an exception from inside an `opentrons.protocol_engine` *state update.* We are not in a good position to handle that: it was an early architectural of `opentrons.protocol_engine` that all the real work ought to happen in command implementations, and that state updates ought to be dead-simple and always successful. (We can and should do something about that premise, but that's beyond the scope of this PR.) This is what caused RQA-4058. 2. Duplication. For `TipStore` to automatically expand the target well into the full list of actually-picked-up tips, it needed to know details about the pipette's current configuration. That was already part of `PipetteStore`'s job;`TipStore` was duplicating it. This PR tries to solve both of those by having `PickUpTipImplementation` give `TipStore` a full precomputed list of tips. `PickUpTipImplementation` produces that precomputed list with help from new functions in `PipetteView` and `TipView`. Pipette concerns like nozzle maps are expunged from `TipStore`. Overall, there's no new logic here—just shifting things around and changing what's running where. (cherry picked from commit 0855bd1)
ddcc4
pushed a commit
that referenced
this pull request
May 22, 2025
…18207) ## Overview This closes RQA-4058 (protocol analysis hanging if `pickUpTip` is given a bad labware definition), and resolves some duplication we noticed a little while ago (#16469 (comment)). ## Details When a `pickUpTip` command completes, `TipStore` marks the picked-up tips as "used." Before this PR, that worked by `PickUpTipImplementation` passing along just the single "target" tip, like A1; `TipStore` then internally expanded that into the full list of actually-picked-up tips, like A1+B1+⋯+H1, for an 8-channel pickup. The choice to do the expansion inside `TipStore` caused two problems: 1. Error handling. If it happens inside `TipStore`, and something goes wrong and raises an exception, then we've raised an exception from inside an `opentrons.protocol_engine` *state update.* We are not in a good position to handle that: it was an early architectural of `opentrons.protocol_engine` that all the real work ought to happen in command implementations, and that state updates ought to be dead-simple and always successful. (We can and should do something about that premise, but that's beyond the scope of this PR.) This is what caused RQA-4058. 2. Duplication. For `TipStore` to automatically expand the target well into the full list of actually-picked-up tips, it needed to know details about the pipette's current configuration. That was already part of `PipetteStore`'s job;`TipStore` was duplicating it. This PR tries to solve both of those by having `PickUpTipImplementation` give `TipStore` a full precomputed list of tips. `PickUpTipImplementation` produces that precomputed list with help from new functions in `PipetteView` and `TipView`. Pipette concerns like nozzle maps are expunged from `TipStore`. Overall, there's no new logic here—just shifting things around and changing what's running where. (cherry picked from commit 0855bd1)
ddcc4
pushed a commit
that referenced
this pull request
May 23, 2025
…18207) ## Overview This closes RQA-4058 (protocol analysis hanging if `pickUpTip` is given a bad labware definition), and resolves some duplication we noticed a little while ago (#16469 (comment)). ## Details When a `pickUpTip` command completes, `TipStore` marks the picked-up tips as "used." Before this PR, that worked by `PickUpTipImplementation` passing along just the single "target" tip, like A1; `TipStore` then internally expanded that into the full list of actually-picked-up tips, like A1+B1+⋯+H1, for an 8-channel pickup. The choice to do the expansion inside `TipStore` caused two problems: 1. Error handling. If it happens inside `TipStore`, and something goes wrong and raises an exception, then we've raised an exception from inside an `opentrons.protocol_engine` *state update.* We are not in a good position to handle that: it was an early architectural of `opentrons.protocol_engine` that all the real work ought to happen in command implementations, and that state updates ought to be dead-simple and always successful. (We can and should do something about that premise, but that's beyond the scope of this PR.) This is what caused RQA-4058. 2. Duplication. For `TipStore` to automatically expand the target well into the full list of actually-picked-up tips, it needed to know details about the pipette's current configuration. That was already part of `PipetteStore`'s job;`TipStore` was duplicating it. This PR tries to solve both of those by having `PickUpTipImplementation` give `TipStore` a full precomputed list of tips. `PickUpTipImplementation` produces that precomputed list with help from new functions in `PipetteView` and `TipView`. Pipette concerns like nozzle maps are expunged from `TipStore`. Overall, there's no new logic here—just shifting things around and changing what's running where. (cherry picked from commit 0855bd1)
ddcc4
pushed a commit
that referenced
this pull request
May 24, 2025
…18207) ## Overview This closes RQA-4058 (protocol analysis hanging if `pickUpTip` is given a bad labware definition), and resolves some duplication we noticed a little while ago (#16469 (comment)). ## Details When a `pickUpTip` command completes, `TipStore` marks the picked-up tips as "used." Before this PR, that worked by `PickUpTipImplementation` passing along just the single "target" tip, like A1; `TipStore` then internally expanded that into the full list of actually-picked-up tips, like A1+B1+⋯+H1, for an 8-channel pickup. The choice to do the expansion inside `TipStore` caused two problems: 1. Error handling. If it happens inside `TipStore`, and something goes wrong and raises an exception, then we've raised an exception from inside an `opentrons.protocol_engine` *state update.* We are not in a good position to handle that: it was an early architectural of `opentrons.protocol_engine` that all the real work ought to happen in command implementations, and that state updates ought to be dead-simple and always successful. (We can and should do something about that premise, but that's beyond the scope of this PR.) This is what caused RQA-4058. 2. Duplication. For `TipStore` to automatically expand the target well into the full list of actually-picked-up tips, it needed to know details about the pipette's current configuration. That was already part of `PipetteStore`'s job;`TipStore` was duplicating it. This PR tries to solve both of those by having `PickUpTipImplementation` give `TipStore` a full precomputed list of tips. `PickUpTipImplementation` produces that precomputed list with help from new functions in `PipetteView` and `TipView`. Pipette concerns like nozzle maps are expunged from `TipStore`. Overall, there's no new logic here—just shifting things around and changing what's running where. (cherry picked from commit 0855bd1)
ddcc4
pushed a commit
that referenced
this pull request
May 24, 2025
…18207) ## Overview This closes RQA-4058 (protocol analysis hanging if `pickUpTip` is given a bad labware definition), and resolves some duplication we noticed a little while ago (#16469 (comment)). ## Details When a `pickUpTip` command completes, `TipStore` marks the picked-up tips as "used." Before this PR, that worked by `PickUpTipImplementation` passing along just the single "target" tip, like A1; `TipStore` then internally expanded that into the full list of actually-picked-up tips, like A1+B1+⋯+H1, for an 8-channel pickup. The choice to do the expansion inside `TipStore` caused two problems: 1. Error handling. If it happens inside `TipStore`, and something goes wrong and raises an exception, then we've raised an exception from inside an `opentrons.protocol_engine` *state update.* We are not in a good position to handle that: it was an early architectural of `opentrons.protocol_engine` that all the real work ought to happen in command implementations, and that state updates ought to be dead-simple and always successful. (We can and should do something about that premise, but that's beyond the scope of this PR.) This is what caused RQA-4058. 2. Duplication. For `TipStore` to automatically expand the target well into the full list of actually-picked-up tips, it needed to know details about the pipette's current configuration. That was already part of `PipetteStore`'s job;`TipStore` was duplicating it. This PR tries to solve both of those by having `PickUpTipImplementation` give `TipStore` a full precomputed list of tips. `PickUpTipImplementation` produces that precomputed list with help from new functions in `PipetteView` and `TipView`. Pipette concerns like nozzle maps are expunged from `TipStore`. Overall, there's no new logic here—just shifting things around and changing what's running where. (cherry picked from commit 0855bd1)
ddcc4
pushed a commit
that referenced
this pull request
May 29, 2025
…18207) ## Overview This closes RQA-4058 (protocol analysis hanging if `pickUpTip` is given a bad labware definition), and resolves some duplication we noticed a little while ago (#16469 (comment)). ## Details When a `pickUpTip` command completes, `TipStore` marks the picked-up tips as "used." Before this PR, that worked by `PickUpTipImplementation` passing along just the single "target" tip, like A1; `TipStore` then internally expanded that into the full list of actually-picked-up tips, like A1+B1+⋯+H1, for an 8-channel pickup. The choice to do the expansion inside `TipStore` caused two problems: 1. Error handling. If it happens inside `TipStore`, and something goes wrong and raises an exception, then we've raised an exception from inside an `opentrons.protocol_engine` *state update.* We are not in a good position to handle that: it was an early architectural of `opentrons.protocol_engine` that all the real work ought to happen in command implementations, and that state updates ought to be dead-simple and always successful. (We can and should do something about that premise, but that's beyond the scope of this PR.) This is what caused RQA-4058. 2. Duplication. For `TipStore` to automatically expand the target well into the full list of actually-picked-up tips, it needed to know details about the pipette's current configuration. That was already part of `PipetteStore`'s job;`TipStore` was duplicating it. This PR tries to solve both of those by having `PickUpTipImplementation` give `TipStore` a full precomputed list of tips. `PickUpTipImplementation` produces that precomputed list with help from new functions in `PipetteView` and `TipView`. Pipette concerns like nozzle maps are expunged from `TipStore`. Overall, there's no new logic here—just shifting things around and changing what's running where. (cherry picked from commit 0855bd1)
ddcc4
pushed a commit
that referenced
this pull request
May 29, 2025
…18207) ## Overview This closes RQA-4058 (protocol analysis hanging if `pickUpTip` is given a bad labware definition), and resolves some duplication we noticed a little while ago (#16469 (comment)). ## Details When a `pickUpTip` command completes, `TipStore` marks the picked-up tips as "used." Before this PR, that worked by `PickUpTipImplementation` passing along just the single "target" tip, like A1; `TipStore` then internally expanded that into the full list of actually-picked-up tips, like A1+B1+⋯+H1, for an 8-channel pickup. The choice to do the expansion inside `TipStore` caused two problems: 1. Error handling. If it happens inside `TipStore`, and something goes wrong and raises an exception, then we've raised an exception from inside an `opentrons.protocol_engine` *state update.* We are not in a good position to handle that: it was an early architectural of `opentrons.protocol_engine` that all the real work ought to happen in command implementations, and that state updates ought to be dead-simple and always successful. (We can and should do something about that premise, but that's beyond the scope of this PR.) This is what caused RQA-4058. 2. Duplication. For `TipStore` to automatically expand the target well into the full list of actually-picked-up tips, it needed to know details about the pipette's current configuration. That was already part of `PipetteStore`'s job;`TipStore` was duplicating it. This PR tries to solve both of those by having `PickUpTipImplementation` give `TipStore` a full precomputed list of tips. `PickUpTipImplementation` produces that precomputed list with help from new functions in `PipetteView` and `TipView`. Pipette concerns like nozzle maps are expunged from `TipStore`. Overall, there's no new logic here—just shifting things around and changing what's running where. (cherry picked from commit 0855bd1)
ddcc4
pushed a commit
that referenced
this pull request
May 29, 2025
…18207) ## Overview This closes RQA-4058 (protocol analysis hanging if `pickUpTip` is given a bad labware definition), and resolves some duplication we noticed a little while ago (#16469 (comment)). ## Details When a `pickUpTip` command completes, `TipStore` marks the picked-up tips as "used." Before this PR, that worked by `PickUpTipImplementation` passing along just the single "target" tip, like A1; `TipStore` then internally expanded that into the full list of actually-picked-up tips, like A1+B1+⋯+H1, for an 8-channel pickup. The choice to do the expansion inside `TipStore` caused two problems: 1. Error handling. If it happens inside `TipStore`, and something goes wrong and raises an exception, then we've raised an exception from inside an `opentrons.protocol_engine` *state update.* We are not in a good position to handle that: it was an early architectural of `opentrons.protocol_engine` that all the real work ought to happen in command implementations, and that state updates ought to be dead-simple and always successful. (We can and should do something about that premise, but that's beyond the scope of this PR.) This is what caused RQA-4058. 2. Duplication. For `TipStore` to automatically expand the target well into the full list of actually-picked-up tips, it needed to know details about the pipette's current configuration. That was already part of `PipetteStore`'s job;`TipStore` was duplicating it. This PR tries to solve both of those by having `PickUpTipImplementation` give `TipStore` a full precomputed list of tips. `PickUpTipImplementation` produces that precomputed list with help from new functions in `PipetteView` and `TipView`. Pipette concerns like nozzle maps are expunged from `TipStore`. Overall, there's no new logic here—just shifting things around and changing what's running where. (cherry picked from commit 0855bd1)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
A refactor towards EXEC-758 and EXEC-764.
Test Plan and Hands on Testing
Changelog
TipStorewas looking forPickUpTipResults to track tips being consumed from tip racks. Convert this to use the newerStateUpdatemechanism.Review requests
None in particular.
Risk assessment
Low if tests pass.