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): Allow starting protocols on robots with completed runs #15839

Merged
merged 6 commits into from
Jul 31, 2024

Conversation

sfoster1
Copy link
Member

@sfoster1 sfoster1 commented Jul 30, 2024

When you click the button to start protocol setup, we try to filter to only robots that are currently capable of starting a protocol. We implemented that as

  • You can connect to the robot
  • The robot doesn't have a current run

But, robots don't actually get rid of their current run until a user either clicks the little X button in the run screen on the desktop or goes back to the main screen of the ODD. That doesn't necessarily align with a user's view of "this protocol is done", and so people would think that the robot just disappeared from that little slideout.

To fix this, we should also consider robots that have a completed protocol to be done. This is a small conceptual change and unfortunately a huge pain to implement because data about whether a run is done has to come from getting the state data for that run.

Testing

  • The setup protocol slideout should show robots that have completed, failed, or cancelled active runs
    • If you start setup, it should work
  • The setup protocol slideout should show robots that do not have an active run
  • The setup protocol slideout should not show robots that have an active run that is not completed, failed, or canceled

Closes EXEC-519

sfoster1 added 3 commits July 30, 2024 15:29
This is (very) widely used and is better off in a more shared location
than having all these places poke into ProtocolUpload
This means that people don't have to make sure to explicitly close the
run context by clicking the x button in the post-run flow before picking
a new robot to run.

Use the current run's completed at timestamp to decide whether or not
the run is complete for the purposes of idle. This requires fetching the
current run, which we do by ID.

Closes EXEC-519
@sfoster1 sfoster1 requested a review from a team as a code owner July 30, 2024 19:35
@sfoster1 sfoster1 requested review from smb2268, shlokamin and mjhuff and removed request for a team July 30, 2024 19:35
Copy link
Contributor

@mjhuff mjhuff left a comment

Choose a reason for hiding this comment

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

Sweeeeet! Thanks for tackling this one. Code looks good.

I was concerned about this approach causing a lot of issues with logic contingent upon the run being current and completed, but from my scan of things, I don't think this will be an issue...but we should still test this per your plan in order to confirm, since there are a lot of moving parts here.

@sfoster1 sfoster1 merged commit 7316d72 into edge Jul 31, 2024
21 checks passed
@sfoster1 sfoster1 deleted the exec-519-display-robots-with-done-protocols branch July 31, 2024 14:17
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.

2 participants