-
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(merge): 8.0.0 alpha.1 mergeback #15981
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
…dy in progress (#15938) # Overview Ugh... this is a pretty bad oopsy. If the ODD is downloading a robot system zip, it is possible (and in fact, common) that additional downloads of the same system zip would get started into new temp dirs. This has been an issue for the entirety of the Flex's existence, but we think this issue is made worse in China because of their harsh firewalls. This PR properly short circuits system downloads early if one is already in progress. Props to @vegano1 for finding this! ## Changelog - Do not initiate new system file downloads if one is already in progress ## Risk assessment Medium
Update text for SetupLiquids confirmation button on desktop to reflect designs. Closes RQA-2929
If both need to be shown, both need to be shown. The setup steps modal comes first and then the heater shaker modal. ## review/dev-testing - [x] on ODD, when a heater/shaker is in the protocol, the confirm steps modal comes first and then the confirm HS. clicking confirm HS is the one that starts the run - [x] on ODD, when there's no heater/shaker, the confirm steps modal still pops and it starts the run when you click confirm - [x] on Desktop, when there's a heater shaker in the protocol and you haven't disabled the popup, the confirm steps modal comes first and then confirm HS. clicking confirm HS is the one that starts the run - [x] on Desktop, when there's a heater shaker in the protocol and you have disabled the popup, the confirm steps modal is the only one and clicking confirm starts the run - [x] on Desktop, when there's no heater shaker in the protocol, the confirm steps modal is the only one and clicking confirm starts the run Closes RQA-2932
…al `PLAY` action (#15943) When a client posts a PLAY action that starts a protocol run, any existing maintenance run ID is cleared from the server. This means that MQTT should update subscribers interested in the current maintenance run.
Add stack icon, highlight, and click handling to non-module stacks (example: slot + adapter + labware)
Special case thermocycler location on ODD ProtocolDetails Hardware (A1+B1) Closes RQA-2938
Fixes RQA-2925 for desktop: the liquids entry now will count as complete for the purposes of not popping or being listed in the modal if you have no liquids. It also gets complete decorations if there are no liquids. That required adding that rightElement thing to the empty version of the setup step, and that also is what we would need to do for modules, so also add decorations for modules and fix the completion logic to count the module actually being plugged in again (this was RQA-2928). On the ODD, similar kind of logic but a lot easier to implement because the data is more local; we can default the state to !hasLiquids, and then use the same styling we already had implemented for runtime parameters for the setup step. Also, remove some stylistically out of date text transforms on the setup cards. ## testing and review - [x] Upload a protocol that has no liquids and no modules and no deck fixtures - [x] the liquids and modules tabs should should default to completed, meaning - [x] on desktop, they get complete badges and text (including on the OT-2) - [x] on the ODD, they get rendered green - [x] on desktop and ODD, they don't inhibit starting the run - [x] including that if you haven't confirmed some other step, the steps nag modal doesn't mention liquids - [x] on a protocol that requires modules, if you don't have the modules plugged in you get an action needed badge Closes RQA-2928 Closes RQA-2925
This forces you to take a pass through LPC or just commit to skipping it all. ## reviewing and testing - [x] you can't click the button on odd when there are no offsets - [x] you can't click the button on desktop when there are no offsets - [x] you can click the button on odd when there are offsets - [x] you can click the button on desktop when there are offsets Closes RQA-2930
Drop tip wizard CTAs now home the pipette if skipping the flow and close the current run context in both the "skip" and "remove tips" cases. Copy is updated throughout all drop tip flow entry points as well. This is the first time the app directly POSTs a one-off command via a maintenance run. We do POST one-off commands elsewhere in the app, but we do so using older, less-supported endpoints. If these one-off commands become more common, we should consider creating a dedicated API for doing so (more hooks!). To solve the immediate problem now, I refactored useDropTipMaintenanceRun. The real complexity of this PR is managing all the interactions with closing run context and ensuring other clients not engaging with the drop tip flows see/don't see the drop tip CTAs when appropriate. It's probably easiest to follow this PR commit-by-commit. NOTE: If a user completes a run with tips attached on both pipettes intentionally (the protocol itself has tips attached for both pipettes at the end of the run), the wizard won't display for both pipettes, only the left pipette. The run context closes as soon as the home happens, so there's no second CTA. The skip command won't home the plungers, so that's safe, but current behavior is the user will have to do drop tips via the Instruments page. I imagine this is quite the edge case, but it's worth noting. I can re-address this when I update the tip detection logic shortly. Here's a complete breakdown of all the changes: Drop Tip Wizard: Error Recovery, Desktop and ODD *DT Wiz flows remain unchanged for all ER options that are not "cancel run". *For "cancel run", only the copy changes for what was once "Skip" to something along the lines of "skip and home pipette." The pipette will home during cancel. Drop Tip Wizard: Post-Run, Desktop and ODD *If the user has tips attached AND does not see dtwiz within error recovery when doing "cancel run", they will see an unskippable modal (no X button in the upper right) that says "skip and home pipettes" and "begin drop tip wizard". *Once the drop tip flows are successfully completed, the run context closes. *If a user clicks the upper-right corner "exit" button within, the confirmation screen now contains copy alerting the user that the pipette will home after you confirm the exit. *Once the user confirms they want to exit, the pipette will home, and the run context will close. Drop Tip Wizard: Instruments Page, Desktop and ODD *The upper-right corner exit confirmation page copy and homing behavior is the same as the post-run behavior. We do not close any sort of run context here. *Note that the drop-tip banner will no longer be present on the desktop, post-run.
<!-- 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 The Z is supposed to retract out to the liquid before sending over the buffered data, however a line from testing got merged in there and this wasn't happening. Also had to fixup the api side since it was calculating the liquid height based off where the machine position was after hardware control took over, now it actually uses the return value to that it returns, converts it to deck coordinates and applies the corresponding offsets. <!-- 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. --> ## 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. -->
…nd less reliance on file reading/creating (#15956) # Overview This PR follows up both #15855 and #15907 with further refinements and small changes to defining and setting CSV parameters, along with behind the scenes refactors. The two main user/client facing changes of this PR are changing the HTTP status code when invalid data file IDs are sent to the `POST` `/runs`, `/protocols` and `/protocols/{protocolId}/analyses` endpoints from 400 to 422, and raising a new `IncompatibleParameterError` when multiple CSV file parameters are added to a single protocol. The change in status code is to reflect that the error is not because of a malformed request, but because there was something in the request that could not be processed (in this case being the data file ID). The new error was added to align with the original PRD and the current UI. While there's no inherent technical limitation for multiple CSV parameters (other than the theoretical limitations with memory, number of file handlers, etc), there are unexpected UI bugs when multiple ones are defined and this change enforces only one per protocol. The other major change of this PR is a non-user facing refactor of how we set the `CSVParameter` interface. Rather than passing the `Path` of the file to the interface and then using a `TemporaryFile` as the basis for all further access of the CSV file and it's contents, now the contents of the file (passed as `bytes`) is what everything else is based off of, including CSV parsing in `parse_as_csv`. With this change, we only ever make a temporary file handler when the user accesses `.file`. With this change reducing the chance of there being an open file handler, a new `file_opened` property was added to the public interface to reduce needless file opening. This is technically user-facing code meant more for internal usage, but I see no danger in exposing it, though if needed we can tag it is a private non-guaranteed method. ## Changelog - Raise `IncompatibleParameterError` when `add_csv_file` in the `ParameterContext` is used more than once - Change status code of invalid data file IDs posted to runs and protocols endpoints from `400` to `422` - Refactor `CSVParameter` to only open temporary file handler when user requests one ## Review requests Should we label `file_opened` as private/use another way of preventing spurious temporary file creation just to close them? ## Risk assessment Low.
This PR fixes an error where a whitescreen occurs when opening device details for a robot with robot server <7.3. Move `runTimeParameters` property from `LegacyGoodRunData` to `KnownGoodRunData` since robot server does not return RTP for server versions <7.3. Check for `runTimeParameters` property explicitly when needed.
Closes RQA-2951 In Error Recovery (and DTWiz in general), the pipette is z-homed before users move to a location to drop tips. Because this location is quite far from the deck, users will likely want to press their arrow keys to jog the pipette quickly, and this will in turn cause the robot to execute potentially 50+ moveRelative commands with no way of stopping it. While not debouncing other flows with jog controls (ex, LPC) has historical context and is less prone to users over-clicking the jog buttons, we should debounce the jog controls within dt wiz. The solution here is to allow a queue of MAX_QUEUED_JOGS. Three is reasonable after testing.
# Overview Closes [RQA-2923](https://opentrons.atlassian.net/browse/RQA-2923). show warnings/errors depending on the run status. modal header. show error details on ODD if run completed with errors. ## Test Plan and Hands on Testing - run a protocol with ER errors on ODD and desktop app. - make sure that when the run is complete you are able to see the list of errors. - warnings shown when run is successfully completed, errors if canceled or failed. ## Changelog - inject run status to `RunFailedModal`. - changed modal title - changed text to show warnings/errors depending on the run status. - ODD - show view error details if there is an error list. ## Risk assessment low. bug fixes. [RQA-2923]: https://opentrons.atlassian.net/browse/RQA-2923?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
Closes RQA-2934 We can safely get the pipetteId from a failed command that involves a pipette, so let's use that instead of loading a pipette with a custom pipetteId.
On modal produced by desktop app `DeviceLanding` "See how to setup a new robot" link, shrink link clickable area to fit content. Closes RQA-2961
Looks like CI is 504ing right now. Will re-run it in a bit. |
mjhuff
force-pushed
the
8.0.0-alpha.0-mergeback_2
branch
from
August 13, 2024 13:38
40515e6
to
d7b2b50
Compare
SyntaxColoring
approved these changes
Aug 13, 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.
Thanks. Python changes look good, looks good to merge once CI passes.
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
Keeping the spirit of #15940 alive.
All the merge conflicts were my own.