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(protocol-designer,shared-data,components,app): adapter split support #13237

Merged
merged 74 commits into from
Aug 18, 2023

Conversation

jerader
Copy link
Collaborator

@jerader jerader commented Aug 4, 2023

closes RAUT-609 and RAUT-612
closes RAUT-561 and RQA-1076 and RQA-1114

Overview

Adapter Split work in PD and App

Note: I will add more test coverage in a follow up pr

Test Plan

For Pd:
sandbox: https://sandbox.designer.opentrons.com/pd_adapter-split/

  1. import an older version PD protocol that has an adapter (one with H-s would work) and make sure that it migrates and exports successfully with the correct loadLabware commands
  2. make a new protocol and add a H-S or Temperature module, make sure you can load the adapter + labware on top
  3. mess around with the moveLabware form, move labware on/off the adapters and modules and make sure it properly errors if you try to move a labware onto an adapter/module that doesn't work with that labware. Also make sure the movelabware form allows you to select tipracks and not select the trash.

For the app:

  1. upload a protocol that has an adapter, make sure the deck thumbnail loads the adapter + liquid information correctly. Check Protocol details in the labware and liquids tab.
  2. go to protocol setup and make sure labware setup, liquid setup list and map view show up correctly
  3. go through LPC, should work as expected
  4. run a protocol, look at the command text, it should display the adapter and labware on top of adapters text correctly
  5. look through protocol and labware setup on the odd, map and list should display correctly

Changelog

PD:

  • add to 7_0_0 migration and the fileCreator to accommodate load adapter
  • separate out the adapters in the deck setup to their own AdapterControls layer and change labware restrictions for H-S and Temperature module to allow for acceptable adapters
  • add some more restrictions to moveLabware

Shared-data:

  • added type NonStackedLabware and extended the LabwareLocation type to include labwareId.

App:

  • added a bunch of logic to accommodate for if the labwareLocation has labwareId which included all the deck map renders in the app and odd (deckThumbnail, protocol setup, odd protocol setup) and the command text.

Review requests

see test plan

Risk assessment

low

@codecov
Copy link

codecov bot commented Aug 4, 2023

Codecov Report

Merging #13237 (456b3e9) into chore_release-7.0.0 (48c5930) will decrease coverage by 7.95%.
Report is 26 commits behind head on chore_release-7.0.0.
The diff coverage is 24.35%.

Impacted file tree graph

@@                   Coverage Diff                   @@
##           chore_release-7.0.0   #13237      +/-   ##
=======================================================
- Coverage                71.72%   63.77%   -7.95%     
=======================================================
  Files                     1582     1705     +123     
  Lines                    52280    31993   -20287     
  Branches                  3321     7739    +4418     
=======================================================
- Hits                     37499    20405   -17094     
+ Misses                   14260     9767    -4493     
- Partials                   521     1821    +1300     
Flag Coverage Δ
api ?
app 69.93% <28.80%> (+27.24%) ⬆️
components 61.47% <8.33%> (-0.96%) ⬇️
g-code-testing ?
hardware ?
hardware-testing ?
labware-library 49.17% <ø> (ø)
notify-server ?
ot3-gravimetric-test ?
protocol-designer 46.10% <22.15%> (-0.95%) ⬇️
react-api-client 70.11% <ø> (ø)
robot-server ?
shared-data 74.46% <ø> (+<0.01%) ⬆️
step-generation 87.18% <ø> (ø)
system-server ?
update-server ?
usb-bridge ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
...ecules/PythonLabwareOffsetSnippet/createSnippet.ts 83.33% <0.00%> (ø)
...yHistoricOffsets/hooks/getLabwareLocationCombos.ts 22.80% <0.00%> (ø)
app/src/organisms/ApplyHistoricOffsets/index.tsx 95.65% <ø> (ø)
...vices/ProtocolRun/SetupLabware/LabwareListItem.tsx 61.64% <0.00%> (ø)
...ices/ProtocolRun/SetupLabware/SetupLabwareList.tsx 100.00% <ø> (ø)
.../SetupLabwarePositionCheck/CurrentOffsetsTable.tsx 70.83% <ø> (ø)
...ices/ProtocolRun/SetupLiquids/SetupLiquidsList.tsx 94.28% <ø> (ø)
...vices/ProtocolRun/SetupModules/SetupModulesMap.tsx 100.00% <ø> (ø)
.../Devices/ProtocolRun/useLabwareOffsetForLabware.ts 0.00% <0.00%> (ø)
...terventionModal/MoveLabwareInterventionContent.tsx 63.41% <ø> (ø)
... and 46 more

... and 1538 files with indirect coverage changes

Comment on lines +65 to +66
'opentrons_96_aluminumblock_biorad_wellplate_200ul',
'opentrons_96_aluminumblock_nest_wellplate_100ul',
Copy link
Collaborator Author

@jerader jerader Aug 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe i should filter out these old aluminum block defs more locally and not here? We basically don't want them selectable at all from the aluminumBlock category

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably ok for now. As long as there's no change legacy protocols will need these definitions (assuming they get swapped out in migration)

@jerader jerader changed the title feat(protocol-designer): adapter split support feat(protocol-designer, shared-data): adapter split support Aug 4, 2023
@jerader jerader changed the title feat(protocol-designer, shared-data): adapter split support feat(protocol-designer, shared-data, components, app, labware-library): adapter split support Aug 8, 2023
@jerader jerader changed the title feat(protocol-designer, shared-data, components, app, labware-library): adapter split support feat(protocol-designer, shared-data, components, app): adapter split support Aug 8, 2023
@jerader jerader marked this pull request as ready for review August 8, 2023 19:57
@jerader jerader requested review from a team as code owners August 8, 2023 19:57
@jerader jerader requested review from b-cooper, shlokamin and a team and removed request for a team August 8, 2023 19:57
@jerader
Copy link
Collaborator Author

jerader commented Aug 9, 2023

Components that need test coverage from this. It will likely be completed in a follow up PR:

getLabwareLocationCombos.test.ts
LabwareListItem.test.tsx
Create tests for parseInitialLoadedLabwareByAdapter, parseInitialLoadAdapterBySlot, adapterControls and getAdapterAndLabwareSplitInfo.ts
getLabwareOffsetLocation.test.tsx
getLabwareRenderInfo.test.ts
getProtocolModulesInfo.test.ts
getSlotLabwareName.test.ts
CheckItem.test.tsx
ProtocolSetupLabware.test.tsx
LabwareSelectionModal.test.tsx
createSnippet.test.tsx

ticket for the follow up: https://opentrons.atlassian.net/browse/RAUT-624

@jerader jerader removed the request for review from a team August 9, 2023 19:08
errorToShow={
!props.canSave && bothFieldsSelected
? i18n.t(
'form.step_edit_form.labwareLabel.errors.labwareSlotIncompatible'
Copy link
Collaborator Author

@jerader jerader Aug 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will specify the labware and the new location in this string in a follow up ticket as well as specifying the adapter name in the dropdown.

https://opentrons.atlassian.net/browse/RAUT-621

@jerader jerader changed the base branch from edge to remove_load_adapter_from_pe August 14, 2023 17:45
@jerader jerader changed the base branch from remove_load_adapter_from_pe to edge August 14, 2023 17:48
@jerader jerader requested a review from a team as a code owner August 14, 2023 20:54
@jerader
Copy link
Collaborator Author

jerader commented Aug 18, 2023

Follow up work post merge:

  1. add test coverage https://opentrons.atlassian.net/browse/RAUT-624
  2. refactor offsetTable to not include protocolData in ResultSummary https://opentrons.atlassian.net/browse/RAUT-624
  3. refactor ProtocolSetupLabware - needs to be reorganized and split up into several components. There is copied logic that can be its own util. https://opentrons.atlassian.net/browse/RAUT-631
  4. refactor getSlotLabwareName https://opentrons.atlassian.net/browse/RAUT-628

Copy link
Contributor

@b-cooper b-cooper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🌐

@jerader jerader merged commit 29dced3 into chore_release-7.0.0 Aug 18, 2023
45 of 51 checks passed
@jerader jerader deleted the pd_adapter-split branch August 18, 2023 23:55
TamarZanzouri pushed a commit that referenced this pull request Sep 13, 2023
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.

None yet

2 participants