refactor(app): currentOffsetModal only displays latest offsets #11913

jerader · 2022-12-20T19:14:34Z

Overview

CurrentOffsetModal used to display all offsets even duplicates and labware offsets where the vector values are all 0. This PR creates a util getLatestCurrentOffsets that filters out the old offsets and the offsets that are 0. Additionally, this PR wires up the LPC cta in CurrentOffsetModal as well as keeping the disabled reason the same as in the main LPC CTA button.

Changelog

create getLatestCurrentOffsets and test
add the new util to CurrentOffsetsModal
create the CurrentOffsetModal test
create useLPCDisabledReason hook that is used in LaunchLabwarePositionCheck and CurrentOffsetsModal, create test for the hook

Review requests

go through LPC and add some offsets - go through it a few times, the CurrentOffsetModal should display only the latest offsets that were created. if there is a reason for the LPC CTA to be disabled, it should also be reflected in the modal

Risk assessment

low

codecov · 2022-12-20T19:41:18Z

Codecov Report

Merging #11913 (982a108) into edge (c50e145) will decrease coverage by 0.19%.
The diff coverage is 92.72%.

@@            Coverage Diff             @@
##             edge   #11913      +/-   ##
==========================================
- Coverage   74.17%   73.97%   -0.20%     
==========================================
  Files        2164     2186      +22     
  Lines       59753    60727     +974     
  Branches     6265     6526     +261     
==========================================
+ Hits        44323    44925     +602     
- Misses      13953    14271     +318     
- Partials     1477     1531      +54

Flag	Coverage Δ
app	`72.34% <92.72%> (-0.85%)`	⬇️

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

Impacted Files	Coverage Δ
...colRun/SetupLabware/LaunchLabwarePositionCheck.tsx	`63.33% <66.66%> (-20.60%)`	⬇️
...c/organisms/Devices/hooks/useLPCDisabledReason.tsx	`94.11% <94.11%> (ø)`
...s/ProtocolRun/SetupLabware/CurrentOffsetsModal.tsx	`85.71% <100.00%> (+63.49%)`	⬆️
...rganisms/Devices/ProtocolRun/SetupLabware/utils.ts	`88.88% <100.00%> (+4.27%)`	⬆️
...librationDetails/PipetteOffsetCalibrationItems.tsx	`70.37% <0.00%> (-6.11%)`	⬇️
...rc/organisms/Devices/RobotOverviewOverflowMenu.tsx	`87.09% <0.00%> (-3.82%)`	⬇️
...ibration/RobotSettingsPipetteOffsetCalibration.tsx	`88.88% <0.00%> (-1.12%)`	⬇️
app/src/redux/buildroot/selectors.ts	`81.48% <0.00%> (-1.06%)`	⬇️
...ngsCalibration/CalibrationDetails/OverflowMenu.tsx	`74.56% <0.00%> (-0.81%)`	⬇️
app/src/atoms/Tooltip/index.tsx	`100.00% <0.00%> (ø)`
... and 33 more

b-cooper · 2022-12-20T20:14:25Z

app/src/organisms/Devices/ProtocolRun/SetupLabware/utils.ts

+export function getLatestCurrentOffsets(
+ currentOffsets: LabwareOffset[]
+): LabwareOffset[] {
+ const reverseCurrentOffsets = [...currentOffsets].reverse()


Why are we reversing the array here? We shouldn't be relying on the inherent order of the offsets that we get back from the robot.

b-cooper · 2022-12-20T20:15:20Z

app/src/organisms/Devices/ProtocolRun/SetupLabware/utils.ts

+ const latestCurrentOffsets = reverseCurrentOffsets.filter(
+ (currentOffset, index) =>
+ currentOffset.location.slotName === uniqueSlots[index] &&
+ !(


we've use isEqual(currentOffset.vector, IDENTITY_VECTOR) else for this comparison

shlokamin

code lgtm, will test on a robot in a sec

shlokamin · 2022-12-21T21:44:08Z

app/src/assets/localization/en/labware_position_check.json

@@ -82,5 +82,6 @@
 "labware_position_check_description": "<block>Labware Position Check is a guided workflow that checks every labware on the deck for an added degree of precision in your protocol.</block><block>Labware Position Check first checks tip racks, and then checks all other labware used in your protocol.</block>",
 "all_modules_and_labware_from_protocol": "All modules and labware used in the protocol",
 "module_in_slot": "{{module}} in {{slot}}",
- "run_labware_position_check": "run labware position check"
+ "run_labware_position_check": "run labware position check",
+ "get_labware_offset_data": "Get Labware Offset Data"


not introduced by this PR, and probably warrants a bigger discussion with the whole app & ui team, but we probably shouldn't be having any sort of formatting (like capitalization) being done inside of these i18n files. i think we should instead always have these json files have lower case text, and hoist the responsibility of any formatting to be done by the consuming component.

will make a note to self to bring this up when everyone is back

shlokamin · 2022-12-21T21:52:15Z

app/src/organisms/Devices/ProtocolRun/SetupLabware/utils.ts

+ const latestCurrentOffsets = nonIdentityOffsets.reduce<LabwareOffset[]>(
+ (acc, offset) => {
+ const previousMatchIndex = acc.findIndex(
+ ao =>


nit, can we name ao something more descriptive? reduce's are sorta tough to read as is

shlokamin · 2022-12-21T22:37:15Z

uh so i'm not sure what is going on, but i can't run LPC at all using protocols generated from prod PD on a robot running with sw version v6.2.0.

this line in depricatedLabware.ts is the culprit, it resolves to undefined:

const labwareDef = labwareDefinitions[currentLabware.definitionUri]

the protocol's labware key does not have a definitionUri field (it has a definitionId field), but the line above is looking for one. should we be using getInitialLabwareLocation instead?

there was some change regarding definitionId vs definitionUri that happened recently right @smb2268?

smb2268 · 2022-12-21T22:47:51Z

the protocol's labware key does not have a definitionUri field (it has a definitionId field), but the line above is looking for one. should we be using getInitialLabwareLocation instead?

Yeah we removed the translation of definitionUri to definitionId that used to happen in the schemaV6Adapter so now we should always be able to check for the definitionUri on the labware key that comes back from protocol analysis. Is this looking directly at the protocol file instead of at the analysis?

When @b-cooper and I were pairing on resolving the conflicts between the schemaV6Adapter removal stuff and his new LPC changes, we decided not to invest the time to make sure the old LPC still worked since it won't ever be accessible in a release. So all of the deprecated LPC components may have errors

@shlokamin @jerader

jerader · 2022-12-22T14:57:40Z

When @b-cooper and I were pairing on resolving the conflicts between the schemaV6Adapter removal stuff and his new LPC changes, we decided not to invest the time to make sure the old LPC still worked since it won't ever be accessible in a release. So all of the deprecated LPC components may have errors

I can test LPC with the ff turned on with my robot this morning, and if it works I am leaning towards leaving this as is since we will be removing the deprecated components in the q1 release. @shlokamin @smb2268

closes RLAB-200

jerader · 2022-12-23T19:31:50Z

After discussing with Shlok yesterday, it seems that we want to be able to get through LPC on a robot to test this out before merging. However, it does work properly on simulator & emulator.

When attempting to test on a robot that is up to date with edge, I ran into numerous null protection errors with regards to modules since my protocol didn't have any modules. Errors appeared in getCheckSteps, getPrepCommands and PrepareSpace. After fixing those errors and restarting my app and robot, my robot got into a weird state where every single protocol failed protocol analysis because they weren't valid jsons. After awhile of trying to fix it, I was unsuccessful. Due to this, i ran out of time and was unable to get through LPC. I will also document this in the ticket but please feel free to merge this in when I'm out.

b-cooper

✅

jerader force-pushed the app_filter-out-duplicates-current-offset-modal branch from 9ee5ed3 to 2d58aa1 Compare December 20, 2022 20:03

jerader marked this pull request as ready for review December 20, 2022 20:03

jerader requested a review from a team as a code owner December 20, 2022 20:03

jerader requested review from b-cooper, brenthagen, koji and a team and removed request for a team December 20, 2022 20:03

b-cooper reviewed Dec 20, 2022

View reviewed changes

jerader requested a review from shlokamin December 21, 2022 17:04

shlokamin reviewed Dec 21, 2022

View reviewed changes

jerader added 5 commits December 23, 2022 13:17

refactor(app): currentOffsetModal only displays latest offsets

facb5cf

closes RLAB-200

create useLPCDisabledReason

af030f3

fix util and add test cases

e9b9bd9

fix SetupLabware test

4bc0b4c

rename ao to currentLabwareOffsets

026ff49

jerader force-pushed the app_filter-out-duplicates-current-offset-modal branch from 0d29c1c to 026ff49 Compare December 23, 2022 18:31

b-cooper added 4 commits January 9, 2023 15:00

filter out identity offsets after deduping

3cf9ca5

remove redundant labware position check modal

e1bbd6c

test fixup

42becb0

lint

982a108

b-cooper self-requested a review January 10, 2023 21:32

b-cooper approved these changes Jan 10, 2023

View reviewed changes

b-cooper merged commit f2fcb0d into edge Jan 10, 2023

b-cooper deleted the app_filter-out-duplicates-current-offset-modal branch January 10, 2023 21:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(app): currentOffsetModal only displays latest offsets #11913

refactor(app): currentOffsetModal only displays latest offsets #11913

jerader commented Dec 20, 2022 •

edited

Loading

codecov bot commented Dec 20, 2022 •

edited

Loading

b-cooper Dec 20, 2022

b-cooper Dec 20, 2022

shlokamin left a comment

shlokamin Dec 21, 2022

shlokamin Dec 21, 2022

shlokamin commented Dec 21, 2022 •

edited

Loading

smb2268 commented Dec 21, 2022 •

edited

Loading

jerader commented Dec 22, 2022

jerader commented Dec 23, 2022

b-cooper left a comment

refactor(app): currentOffsetModal only displays latest offsets #11913

refactor(app): currentOffsetModal only displays latest offsets #11913

Conversation

jerader commented Dec 20, 2022 • edited Loading

Overview

Changelog

Review requests

Risk assessment

codecov bot commented Dec 20, 2022 • edited Loading

Codecov Report

b-cooper Dec 20, 2022

Choose a reason for hiding this comment

b-cooper Dec 20, 2022

Choose a reason for hiding this comment

shlokamin left a comment

Choose a reason for hiding this comment

shlokamin Dec 21, 2022

Choose a reason for hiding this comment

shlokamin Dec 21, 2022

Choose a reason for hiding this comment

shlokamin commented Dec 21, 2022 • edited Loading

smb2268 commented Dec 21, 2022 • edited Loading

jerader commented Dec 22, 2022

jerader commented Dec 23, 2022

b-cooper left a comment

Choose a reason for hiding this comment

jerader commented Dec 20, 2022 •

edited

Loading

codecov bot commented Dec 20, 2022 •

edited

Loading

shlokamin commented Dec 21, 2022 •

edited

Loading

smb2268 commented Dec 21, 2022 •

edited

Loading