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

fix(protocol-designer): fix multi-channel to single-channel switch issue with trash bin destination #16789

Merged
merged 2 commits into from
Nov 13, 2024

Conversation

syao1226
Copy link
Collaborator

@syao1226 syao1226 commented Nov 13, 2024

fix RQA-3543

Overview

Fixing a TypeError when switching from a multi-channel to a single-channel pipette with the trash bin or waste chute as the destination labware, caused by an inability to get the labware definition for these destinations.

Test Plan and Hands on Testing

Changelog

  • Added a condition to differentiate trash bin/waste chute from other labware when updating dispense_wells with a multi-channel pipette. Used getDefaultWells for the trash bin/waste chute, as they lack well locations and labware definitions.

Review requests

Risk assessment

@syao1226 syao1226 requested a review from jerader November 13, 2024 17:10
@syao1226 syao1226 marked this pull request as ready for review November 13, 2024 17:10
@syao1226 syao1226 requested a review from a team as a code owner November 13, 2024 17:10
Comment on lines 517 to 518
// @ts-expect-error(sa, 2021-6-14): appliedPatch.pipette does not exist. Address in #3161
const pipetteId: string = appliedPatch.pipette
Copy link
Collaborator

Choose a reason for hiding this comment

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

does this work? I know there are a bunch of ts-expect-errors in this file but if this way works, i'd prefer to limit our usage of ts-expect-errors when possible. At some point we need to create the types for appliedPatch fully 😄

Suggested change
// @ts-expect-error(sa, 2021-6-14): appliedPatch.pipette does not exist. Address in #3161
const pipetteId: string = appliedPatch.pipette
const pipetteId: string = appliedPatch.pipette as string

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes this does work! should i also make this change on line 487?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

or i'll move it outside of the conditional statement as they both used pipetteId

Copy link
Collaborator

@jerader jerader left a comment

Choose a reason for hiding this comment

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

awesome work! just left a comment about using ts-expect-error

@syao1226 syao1226 merged commit 28261dc into edge Nov 13, 2024
14 checks passed
@syao1226 syao1226 deleted the pd-switching-pipette branch November 13, 2024 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants