-
Notifications
You must be signed in to change notification settings - Fork 179
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
chore(release): Merge chore_release-8.2.0 into edge #16765
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closes RQA-3440 This PR fixes copy and deck map issues when addressing labware nested in other labware. 5eab60c - Split the util that gets the labware location and renders appropriate copy into two separate utils. c753bc5 - Fixes copy issues. We recently refactored slot location display utils, and I just forgot to use the correct, new prop here and update relevant i18n strings. caf9340 - Fixes deck map issues. Essentially, if there are multiple labware in the slot with the failed labware, only render the topmost labware. This is consistent with deck map behavior in other places in the app. Yes, it's a pain and annoying to have to do this outside of the deck map, but after discussion with other FE devs, it sounds like the deck map redesign will fundamentally change the way we render all objects on the deck, so I don't think it's worth the effort to improve deck map internals currently.
… Error Recovery (#16636) Closes EXEC-794 8.2.0 adds predefined drop tip locations, and during drop tip wizard, the flow does not always have context concerning tip attachment status (ie, during maintenance runs). During a maintenance run, to automatically jog the pipette down somewhat before dropping the tip, we moveToAddressableArea with a predefined offset. While this works well for maintenance run type drop tip flows, it does not always play nicely during fixit type drop flows (ie, during Error Recovery). When attempting to move to an addressable area with the predefined z-offset, the command may fail with an error pointing to the z-offset being invalid given the attached tip. There are several potential ways to work around this including: * Never specify an offset, always dropping into the waste chute (and trash bin) at maximum height. This works, but could be unideal in a real world scenario. * Use the predefined z-offset during maintenance run type drop tip flows only (no offset during Error Recovery/fixit type flows). This works, however, the z-offset moved traversed during fixit type flows is less than during a maintenance run type flow. * During a fixit type flow, get some information about currently attached tip length and use that. To keep complexity down and reduce the bug surface, this PR implements option 2. Additionally, it's probably not a good idea to drop tips or blowout if the moveToAddressableArea command fails, so let's change that!
When viewing the Devices tab on the desktop app, there are a lot of GET requests to /runs/null. These errant requests are generated by two places: useRunStatuses The complex enabled logic has bit us in the past here - in this case, the custom enabled condition overrides the default condition not to fetch if there is no runId, creating the errant fetch. Prior to notifications, it was important to selectively enable this hook to prevent polling unnecessarily. However, telemetry indicates that pretty much everyone uses notifications, and this hook uses notifications, so for the sake of keeping things simple, let's just remove this logic. Worst case scenario, it's not terrible to poll here if MQTT is blocked. useNotifyRunQuery A manual refetch() does not use the underlying enabled conditional logic of useRunQuery. There are two ways to solve it, either the way in this PR, or do extend useRunQuery to return a manual refetch function that wraps the custom, lower-level enabled logic. For simplicity, I've chosen path 1, but I do think it would be worth revisiting this if we add more notification hooks with "always fetch only in specific circumstances" type logic.
… and OPT delims. (#16650)
Closes RQA-3464 While the ODD was already filtering out setup steps that can't be confirmed by the app, the desktop app wasn't. This PR just filters out those unconfirmable steps in the "confirm setup" modal.
Closes RQA-3474 See #16636 for some background context (as well as what caused the regression). The conditional logic for utilizing the manual tip offset during Error Recovery was not quite right, as we were supplying stayAtHighestPossibleZ to the moveToAddressableArea command in all instances. This commit corrects it.
…per calibration (#16657) <!-- 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 Since we treated pipettes as a special case for LLD some things got skipped for the gripper but this fixes it. <!-- 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 Grippers/pipettes should calibrate succesfully <!-- 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. -->
Works toward RQA-3469 and closes EXEC-802 The way the app handles divergent runs, those runs that are associated with a non-determinisitic protocol, currently show some confusing copy in the desktop app/ODD, and at times, exhibit unexpected behavior. This commit works toward tying up those loose ends to make divergent protocol handling seem more like a feature and not an unhandled bug.
#16691) Closes RQA-3489 and RQA-3487 Although Opentrons/ot3-firmware#813 fixes the current behavior and unblocks gripper recovery, updating the position estimators isn't sufficiently accurate for post-recovery, protocol run commands. Instead, we should home everything minus the pipette plungers. There is some minor copy update on one view only per discussion w/ design.
…+ validate wavelengths. (#16649)
Closes RQA-3488 When a different app, app B, cancels the active recovery session for app A, some state cleanup should occur for app A. The exact condition for triggering this state clean up was a bit off: currently we are cleaning up the state for app A when app B enters Error Recovery. Instead, we should clean up app A's state when app B cancels app A's recovery session.
Some linux systems are configured with one of the POSIX default locales, like "C" or "POSIX", and for some reason this will be returned by electron's getSystemPreferredLanguages() api directly. Unfortunately, passing this to the browser js standard Intl.Locale() will make it throw, and that would whitescreen the app. Instead, catch the error and treat it as an unmatched system language. Note that this crashes even if the feature flag isn't on.
…or placeLabwareState and unsafe/placeLabware command. (#16719) The `placeLabwareState` and `unsafe/placeLabware` commands rely on the `labwareID` to place labware held by the gripper back down to a deck slot. However, as correctly pointed out, this is wrong because the labwareID changes across runs. This was working for the plate reader lid because the lid is loaded with a fixed labwareID, however, that would not be the case for other labware across different runs. This pull request replaces the labwareID with labwareURI used by the `placeLabwareState` and `unsafe/placeLabware` commands so the behavior is consistent across runs.
Works toward EXEC-807 We have this code in the ODD that runs every render cycle on a lot of the "idle" views that manages the scrollbar. We instantiate a new observer on every render, but the old observer is never cleaned up. This commit ensures that we only ever instantiate one observer, and we properly remove it on component derender.
Resolve conflicts in: * api/src/opentrons/protocol_api/core/engine/labware.py * api/src/opentrons/protocol_engine/state/update_types.py * app/src/assets/localization/en/protocol_command_text.json
sfoster1
approved these changes
Nov 12, 2024
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.
Spot checks look good to me
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Incremental merge of commits in
chore_release-8.2.0
back intoedge
.Test Plan and Hands on Testing
Changelog
Resolves some trivial merge conflicts in:
And update the latest command schema for changes made in
chore_release-8.2.0
to theunsafe/placeLabware
command.Review requests
None.
Risk assessment
Low.