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(app): Enable labware views for LPC Redesign #17384

Merged
merged 7 commits into from
Feb 3, 2025

Conversation

mjhuff
Copy link
Contributor

@mjhuff mjhuff commented Jan 30, 2025

Closes EXEC-1102

Overview

This PR is the second half of the LPC data layer refactors. I apologize for the size of this one upfront. Because so much of the store is restructured, a lot had to change. This should be the last substantially large PR for the Redesign (outside of one that will add a ton of testing once designs are finalized). Check the Review Requests section to see which changes need feedback.

In legacy LPC flows, the labware that require LPCing and their corresponding locations are injected as steps. All labware must be LPC'd during an LPC flow, so it's not uncommon to run LPC and have something like 50+ steps.

For the Redesign, the geometric identity (the URI) of each labware drives the flow. That is, a user selects a geometric identity, and is then presented with the option to LPC the "default offset" or one of the "applied location offsets" that occurs in the run. Alternatively the user may just view labware offset information here. After selecting a specific offset for which the user wants to perform LPC, a singular "do LPC for only this offset" flow occurs. Because the robot actually does care about a labware instance for loading/unloading labware, we do keep track of one and only one labwareId for each uri, even if there are multiple labware with the same uri in the run.

There are some rough view layer implementations that are in the spirit of the Redesign, but those changes are present to keep the flow functional. PRs that address the view layer directly will follow.

A couple caveats/known issues that aren't really issues since we're behind a FF anyway:

  • Because the flow still utilizes the old LPC HTTP API, there are some data manipulation adapter utils present in order to make things work. These utils will be somewhat simpler once we migrate to the new /labwareOffsets resources.
  • If you LPC the same labware in the same slot multiple times, the offsets will be funky. Again, this is directly caused by the desire to maintain the old apply offsets endpoint in this PR.
  • I'm not sure if offsets apply correctly if you do not LPC every last labware. That's ok though, because we're behind a feature flag (and this is temporary)!
Screen.Recording.2025-01-30.at.1.25.04.PM.mov

I did hack the calibration requirement to access LPC (since I tested remotely), but that's not a real change.

Test Plan and Hands on Testing

  • See video.

Changelog

  • Added wireframes for labware views for the LPC Redesign.

Review requests

  • All the changes in this PR stem from the restructuring of LPCWizardState, specifically labwareInfo. How do we feel about the shape of LPCLabwareInfo in app/src/redux/protocol-runs/types/lpc.ts?

Risk assessment

none, behind FF

See LPCLabwareInfo type. All changes are a refactor to support that new interface...mostly. LPC will
not work on this commit, but it's about as granular a commit as possible before implementing the
redesigned flow.
Roughly implements the new steps and labware restructuring from the previous commit. There are
currently no default offsets. Everything is a labware specific offset.
@mjhuff mjhuff requested a review from a team as a code owner January 30, 2025 19:26
Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

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

This all looks good to me, and like you said it seems like the stuff I don't love will go away when we have the new API.

app/src/redux/protocol-runs/types/lpc.ts Outdated Show resolved Hide resolved
@mjhuff mjhuff force-pushed the app_lpc-store-handle-labware branch from 66f3c39 to 723d910 Compare February 3, 2025 17:36
@mjhuff mjhuff merged commit c841986 into edge Feb 3, 2025
30 checks passed
@mjhuff mjhuff deleted the app_lpc-store-handle-labware branch February 3, 2025 18:00
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.

2 participants