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

feat(robot-server): status bar animation during firmware updates #12954

Conversation

fsinapi
Copy link
Contributor

@fsinapi fsinapi commented Jun 22, 2023

Overview

The status bar now pulses during firmware updates, per the status bar design requirements.

This PR takes the approach of adding an animation handler for the FirmwareUpdateManager. The animation handler has functions that are called whenever a subsystem update starts or finishes. The intent is for the Update animation to play on the status bar whenever a SubSystem is being updated, with the caveat that the status bar cannot be accessed while the rear panel is being updated.

This PR also adds in the Activation animation because it's a really simple addition. At the end of the OT-3 post-init tasks, the activation animation will run. I also adjusted the animation to have a fade-off at the beginning instead of just jumping to 0% lights.

Test Plan

Push this patch to a robot. Restart the robot server and you should see the status bar perform the Activation animation after it homes the Z motors.

Now push new firmware (with a temporary tag) and restart the robot server, with at least unattached pipette available. The firmware should update. When the updates first start, the status bar will turn off; after the rear panel finishes updating, the status bar should begin to pulse white. Once the updates finish (should take a few minutes), the status bar should run the Activation animation.

Now attach the extra pipette. Use the /subsystems/ routes to start an update of the pipette. The update animation should play for the duration of the update, and then revert to Idle.

Changelog

  • Added AnimationHandler to the FirmwareUpdateManager to handle status bar update animations
  • Edited the Activation animation to have a short transition to Off at the beginning
  • Added Activation animation at the end of the post-init tasks

Review requests

We do already have a light control task that sets state based on the status of the global EngineStore, but because of that dependency it doesn't map cleanly to being kicked off this early... an alternative approach could be to start the LightControlTask during hardware initialization and then provide a handle to the EngineStore once that is initialized. Would that be better than adding yet another place where the status bar gets set?

Risk assessment

Pretty low, just lights.

@fsinapi fsinapi requested review from a team June 22, 2023 13:30
@fsinapi fsinapi self-assigned this Jun 22, 2023
@fsinapi fsinapi requested a review from a team as a code owner June 22, 2023 13:30
@codecov
Copy link

codecov bot commented Jun 22, 2023

Codecov Report

Merging #12954 (fe1c3c8) into edge (5271d4f) will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #12954      +/-   ##
==========================================
- Coverage   72.90%   72.90%   -0.01%     
==========================================
  Files        2345     2345              
  Lines       64275    64274       -1     
  Branches     7153     7153              
==========================================
- Hits        46861    46860       -1     
  Misses      15743    15743              
  Partials     1671     1671              
Flag Coverage Δ
app 71.61% <ø> (ø)
components 66.49% <ø> (ø)
g-code-testing 96.44% <ø> (ø)
labware-library 49.61% <ø> (ø)
notify-server 89.13% <ø> (ø)
protocol-designer 44.45% <ø> (ø)
step-generation 85.91% <ø> (ø)

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

Impacted Files Coverage Δ
api/src/opentrons/hardware_control/ot3api.py 80.20% <ø> (ø)
robot-server/robot_server/hardware.py 81.94% <ø> (-0.25%) ⬇️

Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

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

This looks good to me. I do not love adding yet another place where we control the lights, but like you said it's tough to find a place where it's accessible during early init.

One approach we could take is to essentially make a second instance of the thing that handles the lights, or a second class with the same interface, and have the subsystem manager use a swappable handle. So we make the temporary instance during early init, and once everything is up swap it out.

@fsinapi fsinapi merged commit 7ae3efc into edge Jun 26, 2023
@fsinapi fsinapi deleted the RCORE-777-Robot-Server-should-run-Update-animation-during-firmware-updates branch June 26, 2023 17:18
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