feat(protocol-designer): allow custom labware on modules #5175

jen-fong · 2020-03-06T20:52:24Z

overview

Closes #4910 by removing restrictions when placing custom labware on modules

changelog

Add checks for custom labware in relevant functions
Add tests

review requests

#4910 - acceptance criteria here
This was tested with the python api but feel free to test it yourself.

Swapping labware

Create a custom labware using labware creator
Create a new protocol with a magnet and temperature module
On the deck view, click on the modules to add a new labware
Select upload custom labware and upload your labware
Select the custom labware as the labware to place on the magnet and temperature module

There should be no errors and labware should be placed on top of the modules

Add magnet and temperature compatible labwares to the deck in empty slots
Swap the custom labware on the magnet module with the magnet compatible labware

Labware should be swapped with no issue

Swap the custom labware on the temperature module with the temperature compatible labware

Labware should be swapped with no issue

Swap the labware back until both temperature and magnet modules both have custom labware on them again
Add a tube rack (this is incompatible with the magnet and temperature modules) to an empty slot on the deck
Try to swap the tube rack with the custom labware on both modules

Incompatible labware should not be swapped

Drag the labware from the modules to empty slots
Drag them back to the module

This should be allowed

Adding steps
Labware is not magnet compatible
Add a magnet engage step

There is no recommended engage height and no recommended label

Labware is magnet compatible and has an engage height
Add a magnet engage step

The recommended engage height is filled out in the form with the recommended label specifying the engage height below it

Temperature steps
Add a set temperature step

There are no errors

Add a deactivate step

There are no errors

closes #4910

codecov · 2020-03-06T20:52:38Z

Codecov Report

Merging #5175 into edge will increase coverage by 0.45%.
The diff coverage is 75.00%.

@@            Coverage Diff             @@
##             edge    #5175      +/-   ##
==========================================
+ Coverage   68.22%   68.68%   +0.45%     
==========================================
  Files        1106     1118      +12     
  Lines       36640    39802    +3162     
==========================================
+ Hits        24999    27338    +2339     
- Misses      11641    12464     +823

Impacted Files	Coverage Δ
...mponents/DeckSetup/LabwareOverlays/SlotControls.js	`46.66% <62.50%> (+46.66%)`	⬆️
...col-designer/src/components/DeckSetup/DeckSetup.js	`30.13% <83.33%> (+30.13%)`	⬆️
...l-designer/src/utils/labwareModuleCompatibility.js	`57.14% <100.00%> (+17.14%)`	⬆️
opentrons/hardware_control/types.py	`98.00% <0.00%> (-2.00%)`	⬇️
opentrons/protocol_api/module_geometry.py	`85.10% <0.00%> (-1.94%)`	⬇️
opentrons/drivers/smoothie_drivers/driver_3_0.py	`74.19% <0.00%> (-1.64%)`	⬇️
opentrons/protocol_api/geometry.py	`98.40% <0.00%> (-0.93%)`	⬇️
opentrons/hardware_control/execution_manager.py	`94.44% <0.00%> (-0.80%)`	⬇️
opentrons/hardware_control/modules/thermocycler.py	`94.18% <0.00%> (-0.56%)`	⬇️
opentrons/api/models.py	`100.00% <0.00%> (ø)`
... and 77 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 9671575...d5a67d6. Read the comment docs.

Kadee80

🍔

Can upload and add custom labware to Magdeck
Can upload and add custom labware to Tempdeck
Can swap custom labware between both modules
Can swap compatible labware with custom labware
Adding an engage step with a custom labware on it does not have recommended value
After swapping with compatible labware (not custom) the recommended value renders
No temperature step errors

protocol-designer/src/components/DeckSetup/DeckSetup.js

protocol-designer/src/components/DeckSetup/__tests__/DeckSetup.test.js

IanLondon · 2020-03-10T18:12:18Z

protocol-designer/src/components/DeckSetup/__tests__/DeckSetup.test.js

+
+    let getLabwareIsCompatibleSpy
+    beforeEach(() => {
+      getLabwareIsCompatibleSpy = jest.spyOn(


As far as I understand the main difference btw a spy and a mock is that the mock requires you to explicitly mock any implementation / return value, and the spy implicitly uses the real implementation unless you overwrite it. Since getLabwareIsCompatible doesn't have any knowledge of the fixture load names, the real implementation isn't relevant to the tests here. So should we use mock instead of spyOn here?

in jest world, a spy can have a mock return value (line 91). The spies are very useful for mocking out single functions in a module as opposed to the whole thing (I think that's necessary for flow?) and then manually unmocking other fns in there. I didn't want to mock out the getLabwareIsCustom fn, just getLabwareIsCompatible` since it's checking against a specific list of labware.

IanLondon · 2020-03-10T18:42:56Z

protocol-designer/src/components/DeckSetup/__tests__/DeckSetup.test.js

@@ -0,0 +1,141 @@
+// @flow


I think we're missing a handful of cases here, since "from deck to module" and "from module to deck" (and from module A to module B and visa versa) are all distinct.

Related: even if we don't add more cases, would this test benefit from being driven by a forEach to make it more concise?

I think there are 15 cases:

Labware A from deck to empty deck slot: never blocked

Labware A from deck to deck slot with some Labware B: never blocked

Labware A from deck to empty module: block if A is not compatible

Labware A from deck to empty module: allow if A is compatible

Labware A from deck to module with some module-compatible labware B: block if A is not compatible

Labware A from deck to module with some module-compatible labware B: allow if A is compatible

For all these, A starts on module X (and A is compatible with module X):

Labware A from module X to empty deck slot: always allow

Labware A from module X to empty module Y: allow if A is compatible with Y

Labware A from module X to empty module Y: disallow if A is not compatible with Y

Labware A from module X to deck slot with labware B: disallow if B is not compatible with X

Labware A from module X to deck slot with labware B: allow if B is compatible with X

Labware A from module X to module Y that has a labware B on it: (4 cases here!) allow if A is compatible with Y and B is compatible with X. disallow in the 3 other cases where A and/or B is not compatible with the other module

Yea, I think some need to be added in there. It is much easier to see the cases on the ui.

As it turns out. this one function doesn't handle all these cases. It's handled as a joint effort between this fn and SlotControls. For example, for the case where you are moving an incompatible labware on to an empty module, there is no hovered labware so this getSwapBlocked fn will not say it is blocked. It is handled in SlotControls right when we drag the labware over the module. Maybe it is better to have some integration tests for these type of tests?

IanLondon · 2020-03-10T18:44:13Z

protocol-designer/src/components/DeckSetup/__tests__/DeckSetup.test.js

+      expect(isBlocked).toEqual(false)
+    })
+
+    it('is not blocked when no dragged labware', () => {


can this happen? This would be a 16th case, I didn't count it in my list ^

I'm not sure it can happen. It was in the function already so I added a case for it 😅 .

IanLondon

🖌️

Jenny added 3 commits March 5, 2020 10:46

feat(protocol-designer): allow custom labware on modules on deck

e4c95ab

closes #4910

add tests

968e579

clean up files and tests

3fee2c8

jen-fong added ready for review protocol designer Affects the `protocol-designer` project labels Mar 6, 2020

jen-fong requested review from Kadee80, shlokamin and IanLondon March 6, 2020 20:52

Kadee80 approved these changes Mar 10, 2020

View reviewed changes

IanLondon suggested changes Mar 10, 2020

View reviewed changes

jen-fong requested a review from IanLondon March 11, 2020 18:41

add more tests

d5a67d6

jen-fong force-pushed the pd_custom-labware-on-modules branch from 1511a05 to d5a67d6 Compare March 11, 2020 18:46

IanLondon approved these changes Mar 12, 2020

View reviewed changes

jen-fong merged commit 1c4e1b5 into edge Mar 12, 2020

mcous deleted the pd_custom-labware-on-modules branch May 6, 2020 23:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(protocol-designer): allow custom labware on modules #5175

feat(protocol-designer): allow custom labware on modules #5175

jen-fong commented Mar 6, 2020

codecov bot commented Mar 6, 2020 •

edited

Loading

Kadee80 left a comment

IanLondon Mar 10, 2020

jen-fong Mar 10, 2020

IanLondon Mar 10, 2020

jen-fong Mar 10, 2020 •

edited

Loading

jen-fong Mar 11, 2020

IanLondon Mar 10, 2020 •

edited

Loading

jen-fong Mar 10, 2020

IanLondon left a comment

feat(protocol-designer): allow custom labware on modules #5175

feat(protocol-designer): allow custom labware on modules #5175

Conversation

jen-fong commented Mar 6, 2020

overview

changelog

review requests

codecov bot commented Mar 6, 2020 • edited Loading

Codecov Report

Kadee80 left a comment

Choose a reason for hiding this comment

IanLondon Mar 10, 2020

Choose a reason for hiding this comment

jen-fong Mar 10, 2020

Choose a reason for hiding this comment

IanLondon Mar 10, 2020

Choose a reason for hiding this comment

jen-fong Mar 10, 2020 • edited Loading

Choose a reason for hiding this comment

jen-fong Mar 11, 2020

Choose a reason for hiding this comment

IanLondon Mar 10, 2020 • edited Loading

Choose a reason for hiding this comment

jen-fong Mar 10, 2020

Choose a reason for hiding this comment

IanLondon left a comment

Choose a reason for hiding this comment

codecov bot commented Mar 6, 2020 •

edited

Loading

jen-fong Mar 10, 2020 •

edited

Loading

IanLondon Mar 10, 2020 •

edited

Loading