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(app): fix TC allows module calibration with lid closed #13533

Merged
merged 2 commits into from
Sep 12, 2023

Conversation

koji
Copy link
Contributor

@koji koji commented Sep 12, 2023

Overview

fix TC allows module calibration with lid closed

close RQA-1519

ToDo update test cases later

Test Plan

Changelog

Review requests

Risk assessment

low

fix TC allows module calibration with lid closed

fix RQA-1519
@koji koji requested a review from a team September 12, 2023 19:57
@koji koji marked this pull request as ready for review September 12, 2023 19:58
@koji koji requested a review from a team as a code owner September 12, 2023 19:58
@koji koji requested review from shlokamin and removed request for a team and shlokamin September 12, 2023 19:58
@codecov
Copy link

codecov bot commented Sep 12, 2023

Codecov Report

Merging #13533 (c913b50) into chore_release-7.0.0 (443e88c) will decrease coverage by 0.03%.
Report is 15 commits behind head on chore_release-7.0.0.
The diff coverage is 5.55%.

Additional details and impacted files

Impacted file tree graph

@@                   Coverage Diff                   @@
##           chore_release-7.0.0   #13533      +/-   ##
=======================================================
- Coverage                71.30%   71.27%   -0.03%     
=======================================================
  Files                     2418     2419       +1     
  Lines                    68007    68298     +291     
  Branches                  7896     7980      +84     
=======================================================
+ Hits                     48492    48680     +188     
- Misses                   17668    17748      +80     
- Partials                  1847     1870      +23     
Flag Coverage Δ
app 68.83% <5.55%> (-0.08%) ⬇️

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

Files Changed Coverage Δ
app/src/organisms/ModuleCard/index.tsx 54.05% <0.00%> (-2.55%) ⬇️
app/src/organisms/ProtocolSetupModules/index.tsx 60.00% <0.00%> (-4.00%) ⬇️
...p/src/organisms/RobotSettingsCalibration/index.tsx 51.19% <ø> (-0.58%) ⬇️
...librationDetails/ModuleCalibrationOverflowMenu.tsx 61.11% <14.28%> (-12.23%) ⬇️

... and 12 files with indirect coverage changes

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.

lgtm, was able to see the TC open lid command called in the network tab. Just need the additional logic for when toggleLatch() is called

@@ -158,9 +158,6 @@ export function RobotSettingsCalibration({
: null
)

console.log('pipetteOffsetCalibrations', pipetteOffsetCalibrations)
Copy link
Collaborator

Choose a reason for hiding this comment

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

🔪

@@ -256,6 +259,24 @@ export const ModuleCard = (props: ModuleCardProps): JSX.Element | null => {
if (!isLatchClosed) {
Copy link
Collaborator

@jerader jerader Sep 12, 2023

Choose a reason for hiding this comment

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

I think we need additional logic in here to check if the module is a heater-shaker type, otherwise, the command is emitted even if you are launching the module cal flow for the thermocycler and the h-s attached to the robot has a closed latch.

@koji koji merged commit d0f6b19 into chore_release-7.0.0 Sep 12, 2023
@koji koji deleted the fix_app-module-calibration-for-tc branch November 29, 2023 04:34
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