-
Notifications
You must be signed in to change notification settings - Fork 183
Merge back 'chore_release-pd-8.5.0' into 'edge' #18820
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
Conversation
…t. (#18742) <!-- Thanks for taking the time to open a Pull Request (PR)! Please make sure you've read the "Opening Pull Requests" section of our Contributing Guide: https://github.com/Opentrons/opentrons/blob/edge/CONTRIBUTING.md#opening-pull-requests GitHub provides robust markdown to format your PR. Links, diagrams, pictures, and videos along with text formatting make it possible to create a rich and informative PR. For more information on GitHub markdown, see: https://docs.github.com/en/get-started/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax To ensure your code is reviewed quickly and thoroughly, please fill out the sections below to the best of your ability! --> # Overview Tuck in the last few hardware-testing changes into the 8.5 release. <!-- Describe your PR at a high level. State acceptance criteria and how this PR fits into other work. Link issues, PRs, and other relevant resources. --> ## Test Plan and Hands on Testing Does not touch public code so no testing required. <!-- Describe your testing of the PR. Emphasize testing not reflected in the code. Attach protocols, logs, screenshots and any other assets that support your testing. --> ## Changelog <!-- List changes introduced by this PR considering future developers and the end user. Give careful thought and clear documentation to breaking changes. --> ## Review requests <!-- - What do you need from reviewers to feel confident this PR is ready to merge? - Ask questions. --> ## Risk assessment <!-- - Indicate the level of attention this PR needs. - Provide context to guide reviewers. - Discuss trade-offs, coupling, and side effects. - Look for the possibility, even if you think it's small, that your change may affect some other part of the system. - For instance, changing return tip behavior may also change the behavior of labware calibration. - How do your unit tests and on hands on testing mitigate this PR's risks and the risk of future regressions? - Especially in high risk PRs, explain how you know your testing is enough. -->
(cherry picked from commit 03b3054)
# Overview In PR #18711 (AUTH-2003), we changed the liquid class display names to: `Aqueous (Deionized water)` / `Volatile (80% ethanol)` / `Viscous (50% glycerol)`. But we've decided we want to change the display names back. So: - displayName: `Aqueous`, description `Deionized water`, loadname: `water` - displayName: `Volatile`, description `80% ethanol`, loadname: `ethanol_80` - displayName: `Viscous`, description `50% glycerol`, loadname: `glycerol_50` https://opentrons.slack.com/archives/C07JJA1AJFQ/p1751036532344239?thread_ts=1750692306.637929&cid=C07JJA1AJFQ This PR reverts #18711 and #18738, and was generated by `git revert` with no manual changes: ``` git revert ec54caf 968ddc8 ``` ## Test Plan and Hands on Testing Running CI tests. ## Review requests This is what we want now, right? ## Risk assessment Low, but we're cutting it close.
Don't crash the entire app when this happens. Closes RQA-4308
#18783) …idClass json and python command
<!-- Thanks for taking the time to open a Pull Request (PR)! Please make sure you've read the "Opening Pull Requests" section of our Contributing Guide: https://github.com/Opentrons/opentrons/blob/edge/CONTRIBUTING.md#opening-pull-requests GitHub provides robust markdown to format your PR. Links, diagrams, pictures, and videos along with text formatting make it possible to create a rich and informative PR. For more information on GitHub markdown, see: https://docs.github.com/en/get-started/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax To ensure your code is reviewed quickly and thoroughly, please fill out the sections below to the best of your ability! --> # Overview Updates to the Liquid Control page for PD interop + new params added. <!-- Describe your PR at a high level. State acceptance criteria and how this PR fits into other work. Link issues, PRs, and other relevant resources. --> ## Test Plan and Hands on Testing <!-- Describe your testing of the PR. Emphasize testing not reflected in the code. Attach protocols, logs, screenshots and any other assets that support your testing. --> sandbox: http://sandbox.docs.opentrons.com/docs-pd-interop-8.5/v2/ ## Changelog -added description for ``flow_rate`` param in aspirate/dispense commands (and contrast with ``rate``). checked the API reference entries. - added description for air gap ``in_place`` and ``flow_rate`` parameters (and contrast with ``rate``) - added description for ``mm_from_edge`` behavior and warning. <!-- List changes introduced by this PR considering future developers and the end user. Give careful thought and clear documentation to breaking changes. --> ## Review requests <!-- - What do you need from reviewers to feel confident this PR is ready to merge? - Ask questions. --> Need to confirm the warning for ``mm_from_edge`` in touch tip. ## Risk assessment <!-- - Indicate the level of attention this PR needs. - Provide context to guide reviewers. - Discuss trade-offs, coupling, and side effects. - Look for the possibility, even if you think it's small, that your change may affect some other part of the system. - For instance, changing return tip behavior may also change the behavior of labware calibration. - How do your unit tests and on hands on testing mitigate this PR's risks and the risk of future regressions? - Especially in high risk PRs, explain how you know your testing is enough. --> low.
This reverts #18450 and #18479 to revert to digicert signing for our windows builds. The digicert certificate has the Common Name "Opentrons Labworks Inc." and the ATS cert has the common name "OPENTRONS LABWORKS INC.". These were both determined automatically by the CA from our identity submissions, as is apparently required in the code signing cert baseline requirements. Why are they different? A mystery for the ages. In either case, electron-updater requires that _if_ you specify a `publisherName` in your `app-update.yml` (which we do specifically on windows, since it is generated from our electron-builder config and on windows we set it because nsis packager wants it for doing signing in the first place) _then_ the autoupdate package that will be installed must have a CSC CN exactly matching an entry in `publisherName` or the update will fail. Therefore updates in between <=8.4.1 and >=8.5.0 would fail if we switched to ATS. Instead, we'll switch back to digicert for now; we'll build the new CN into our publisher names; and then whenever we're confident enough people are on >=8.5.0 and therefore have the new publisher names, we'll switch over again (we can't switch immediately because we don't do incremental updates, just full overwrites, so the intermediate update state would go away). This is upsetting. ## Testing - [x] the signing has to work again, which is never guaranteed given the shonky state of dco integration - [x] we should make sure we can update from 8.4.1 to this by making sure the CN of the digital signature on the build from this pr is exactly `Opentrons Labworks Inc.` (and updating to it in the resulting alpha) - [x] we should make sure we can update from this to something signed with the new cert (by mucking around with the latest-alpha or something? or just checking that the app-update.yml in the app's install directory has both names) Supercedes #18785 for build branch name reasons.
…#18802) # Overview In `distribute_with_liquid_class()`, we were dispensing into the same destination well twice when the distribute consists of multiple tip-fills. The bug was caused by the reuse of the variable `next_dest` in `distribute_with_liquid_class()`. `next_dest` in the outer loop is supposed to contain the next well that we have not dispensed to yet. However, we accidently overwrote the variable because we also use it as a loop variable for an unrelated inner loop: ``` for next_vol, next_dest in vol_dest_combo: ``` ## Test Plan and Hands on Testing Using `analyze`, I confirmed that the double-dispense happened with Andy's example before this change, and that it was fixed after this change. I also updated `test_order_of_water_distribution_steps_using_multi_dispense()` to perform a distribute that requires multiple tip-fills, and changed the test to check that we're dispensing into specific wells instead of `dest=mock.ANY`. I confirmed that the bug happened before this change, and it was fixed after this change. ## Risk assessment Medium: this is change deep inside our implementation. But it's necessary to fix the bug.
…e well (#18791) This PR fixes the subtext in the liquid class options in a transfer or mix form. We should only show "Assigned to [x,y,z wells]" subtext to a liquid class option if the liquid class is assigned to a liquid in at least one of the selected source/mix wells. If the liquid class is present in the source/mix labwrae, but not present in the selected source wells, we show the liquid class's generic description. Closes RQA-4106
This PR ensures the specified dispense delay for a move liquid form is applied after dispensing air gap.
…18808) This PR fixes a whitescreen that occurs after deleting a pipette that is used in a moveLiquid or mix step form.
# Overview Addresses #18803 > The documentation found [here](https://docs.opentrons.com/v2/parameters/defining.html?#the-add-parameters-function), i.e. the "The add_parameters() Function" section of the page on "Defining Parameters", has the wrong type annotation. It is documented as Parameters when it should be ParameterContext. ## Test Plan and Hands on Testing [Sandbox](http://sandbox.docs.opentrons.com/docs-fix-rtp-annotation/v2/parameters/defining.html#the-add-parameters-function) ## Changelog 1-liner ## Review requests nothing special. ## Risk assessment nil
Ensures OT-2 liquid class default values for aspirate/dispense submerge/retract z height are populated. Closes RQA-4345
…sfers (#18818) Calculate correction by volume by keying on the volume after aspiration and dispense, not the aspirate or dispense volume itself.
Manually resolved conflicts: - app-shell/electron-builder.config.js - components/src/organisms/CommandText/__tests__/CommandText.test.tsx - protocol-designer/src/pages/Designer/ProtocolSteps/StepForm/StepTools/MoveLiquidTools/__tests__/LiquidClassesStepTools.test.tsx - react-api-client/src/system/useAuthorization.ts
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## edge #18820 +/- ##
==========================================
- Coverage 27.70% 27.07% -0.63%
==========================================
Files 3204 3246 +42
Lines 250439 257281 +6842
Branches 26320 26866 +546
==========================================
+ Hits 69380 69671 +291
- Misses 181042 187588 +6546
- Partials 17 22 +5
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Hm that failing test is a little concerning. A protocol is coming out the other end of a PD migration without any Or is that expected from the export-to-Python work? |
I ran it again, and it passed -- must be a flaky test :( https://github.com/Opentrons/opentrons/actions/runs/16037463114/job/45256158103#step:4:175 I think Josh explained that the Cypress tests sometimes fail because they're clicking too fast before the PD app has gotten to the step they were expecting. So in this case, the mis-timed clicks probably failed to create a protocol with |
Yeah, the PD cypress migration test is very flakey unfortunately. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Resolved conflicts: `protocol-designer/src/pages/Designer/ProtocolSteps/StepForm/StepTools/MoveLiquidTools/hooks/useAssignLiquidClass.ts`.
8dea32a
to
4d1c163
Compare
This picks up the changes from
chore_release-8.5.0
andchore_release-pd-8.5.0
from the past few days.Manually resolved conflicts in:
protocol-designer/src/pages/Designer/ProtocolSteps/StepForm/StepTools/MoveLiquidTools/hooks/useAssignLiquidClass.ts