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): configure modules during calibration, shorten heater shaker fixture name #14953

Merged
merged 7 commits into from
Apr 19, 2024

Conversation

b-cooper
Copy link
Contributor

@b-cooper b-cooper commented Apr 18, 2024

Overview

At the beginning of the module calibration flow, the user is asked to locate the module on the deck.
This integrated the deck configurator component directly into this location selction step of the
module calibration wizard. the selected location will now be saved directly to deck configuration.

Closes RQA-2603

Review requests

  • Run module calibration and confirm that the selected location reflects the deck configuration

Risk assessment

low

… fixture name

At the beginning of the module calibration flow, the user is asked to locate the module on the deck.
This integrated the deck configurator component directly into this location selction step of the
module calibration wizard. the selected location will now be saved directly to deck configuration.

Closes RQA-2603
@b-cooper b-cooper requested a review from a team as a code owner April 18, 2024 21:24
Copy link

codecov bot commented Apr 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.11%. Comparing base (8f50b08) to head (0713b85).
Report is 83 commits behind head on edge.

❗ Current head 0713b85 differs from pull request most recent head 1ef1dc8. Consider uploading reports for the commit 1ef1dc8 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #14953      +/-   ##
==========================================
+ Coverage   67.50%   76.11%   +8.60%     
==========================================
  Files        2521       41    -2480     
  Lines       72090     2746   -69344     
  Branches     9311        0    -9311     
==========================================
- Hits        48666     2090   -46576     
+ Misses      21228      656   -20572     
+ Partials     2196        0    -2196     
Flag Coverage Δ
api ?
app ?
components ?
g-code-testing ?
hardware ?
hardware-testing ?
labware-library ?
notify-server ?
ot3-gravimetric-test ?
protocol-designer ?
react-api-client ?
robot-server ?
shared-data 76.11% <ø> (+0.79%) ⬆️
step-generation ?
system-server ?
update-server ?
usb-bridge ?

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

see 2486 files with indirect coverage changes

Copy link
Contributor

@CaseyBatten CaseyBatten left a comment

Choose a reason for hiding this comment

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

Overall looks solid, app flow makes sense when using and I was able to successfully calibrate a thermocycler on a Flex with no interruptions.

Comment on lines 65 to +69
const cutoutId = deckConfig.find(
cc => cc.opentronsModuleSerialNumber === attachedModule.serialNumber
cc =>
cc.opentronsModuleSerialNumber === attachedModule.serialNumber &&
(attachedModule.moduleType !== THERMOCYCLER_MODULE_TYPE ||
cc.cutoutFixtureId === THERMOCYCLER_V2_FRONT_FIXTURE)
Copy link
Contributor

Choose a reason for hiding this comment

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

This fix looks appropriate and solves the issues we were having in the case of multiple thermocycler fixtures proccing on the front end from the deck config.

Comment on lines 107 to 111
// If module fixture is loaded, still visualize singleSlotFixture underneath for consistency
Object.entries(MODULE_FIXTURES_BY_MODEL).reduce<CutoutFixtureId[]>(
(acc, [_model, fixtures]) => [...acc, ...fixtures],
[]
))
Copy link
Contributor

Choose a reason for hiding this comment

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

this renders single slot fixtures in (under) every cutout...maybe this is ok?

Screen Shot 2024-04-19 at 3 47 58 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

interesting..., yeah maybe it should only be done for module fixtures. Good catch! I'll patch it up

Copy link
Contributor

@brenthagen brenthagen left a comment

Choose a reason for hiding this comment

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

LGTM !

@b-cooper b-cooper merged commit 4cc69eb into edge Apr 19, 2024
56 checks passed
@b-cooper b-cooper deleted the app_config-modules-during-calibration branch April 19, 2024 21:13
Carlos-fernandez pushed a commit that referenced this pull request May 20, 2024
… fixture name (#14953)

# Overview

At the beginning of the module calibration flow, the user is asked to
locate the module on the deck.
This integrated the deck configurator component directly into this
location selction step of the
module calibration wizard. the selected location will now be saved
directly to deck configuration.

Closes [RQA-2603](https://opentrons.atlassian.net/browse/RQA-2603)

# Review requests

- Run module calibration and confirm that the selected location reflects
the deck configuration

# Risk assessment
low


[RQA-2603]:
https://opentrons.atlassian.net/browse/RQA-2603?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
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

3 participants