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(app): acknowledge animation when Flex receives a new protocol #13474

Merged
merged 3 commits into from
Sep 6, 2023

Conversation

fsinapi
Copy link
Contributor

@fsinapi fsinapi commented Sep 6, 2023

Overview

The design spec for the status bar calls for the Confirmation animation to play when a new protocol is uploaded. We already have 1) a stateless protocol engine command to run that animation, and 2) a hook in the ODD app to detect when a new protocol is uploaded. This PR just glues those two things together.

Test Plan

Pushed this to Flexy and uploaded a protocol.

  • If the robot is idle (status bar is plain white, not running a protocol) when it is uploaded, the animation plays
  • If the robot is in the middle of a protocol run, the status bar does not play the animation, instead showing the status of the running protocol

Changelog

  • Added definitions for the Status Bar Animation command to schemaV7 in shared-data
  • In the hook listening for new protocol uploads, send a status bar animation command to run the confirm animation if a new protocol was uploaded

Review requests

  • Does the command definition stuff in shared-data need to be filled in anywhere else that I'm missing?

Risk assessment

Pretty low, not core behavior

@fsinapi fsinapi requested a review from shlokamin September 6, 2023 16:51
@fsinapi fsinapi self-assigned this Sep 6, 2023
@fsinapi fsinapi requested a review from a team as a code owner September 6, 2023 16:51
@codecov
Copy link

codecov bot commented Sep 6, 2023

Codecov Report

Merging #13474 (3b07cc1) into chore_release-7.0.0 (96834e6) will increase coverage by 0.13%.
Report is 3 commits behind head on chore_release-7.0.0.
The diff coverage is 0.00%.

Additional details and impacted files

Impacted file tree graph

@@                   Coverage Diff                   @@
##           chore_release-7.0.0   #13474      +/-   ##
=======================================================
+ Coverage                71.23%   71.36%   +0.13%     
=======================================================
  Files                     1547     2433     +886     
  Lines                    50312    68070   +17758     
  Branches                  3435     7919    +4484     
=======================================================
+ Hits                     35838    48581   +12743     
- Misses                   13945    17630    +3685     
- Partials                   529     1859    +1330     
Flag Coverage Δ
app 69.03% <0.00%> (+25.07%) ⬆️
protocol-designer 46.09% <ø> (ø)
shared-data 73.88% <ø> (+2.12%) ⬆️
step-generation 87.18% <ø> (ø)

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

Files Changed Coverage Δ
app/src/App/hooks.ts 10.71% <0.00%> (ø)

... and 885 files with indirect coverage changes

Copy link
Member

@shlokamin shlokamin left a comment

Choose a reason for hiding this comment

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

lgtm! only complaint is the Incidental naming cuz its kinda vague, but honestly i dont have anything better and we can change the name later if we want. thx frank!

app/src/App/hooks.ts Outdated Show resolved Hide resolved
@fsinapi fsinapi merged commit a04c8d0 into chore_release-7.0.0 Sep 6, 2023
@fsinapi fsinapi deleted the RCORE-780_acknowledge_protocol_upload branch September 6, 2023 21:47
jerader added a commit that referenced this pull request Sep 12, 2023
…13474)

* Add schemaV7 definition for StatusBarAnimation control

* When ODD detects a new protocol, run status bar Confirm animation

* Update app/src/App/hooks.ts typo

Co-authored-by: Jethary Rader <[email protected]>

---------

Co-authored-by: Jethary Rader <[email protected]>
TamarZanzouri pushed a commit that referenced this pull request Sep 13, 2023
…13474)

* Add schemaV7 definition for StatusBarAnimation control

* When ODD detects a new protocol, run status bar Confirm animation

* Update app/src/App/hooks.ts typo

Co-authored-by: Jethary Rader <[email protected]>

---------

Co-authored-by: Jethary Rader <[email protected]>
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.

3 participants