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

Hammer/generic alerts #948

Merged
merged 42 commits into from
Jul 9, 2024
Merged

Conversation

aaronchongth
Copy link
Member

@aaronchongth aaronchongth commented May 28, 2024

What's new

This works with open-rmf/rmf_internal_msgs#71

Replaces the existing task alert system to be much more flexible and generic, with the key purpose to support the smart cart API server interaction.

image

image

Test with regular task

  • task complete
  • failed dispatch

Test with fleet alerts

ros2 topic pub /fleet_alert rmf_fleet_msgs/msg/FleetAlert '{id: test2, title: title, subtitle: subtitle, message: message, display: true, tier: 0, responses_available: [move_off, timeout], alert_parameters: [{name: waiting_at, value: test_location}], task_id: test_task}' --once

What's next:

  • Migrate delivery_alerts to use this as well
  • Frontend for new task types

Self-checks

  • I have prototyped this new feature (if necessary) on Figma
  • I'm familiar with and follow this Typescript guideline
  • I added unit-tests for new components
  • I tried testing edge cases
  • I tested the behavior of the components that interact with the backend, with an e2e test

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Copy link

codecov bot commented May 28, 2024

Codecov Report

Attention: Patch coverage is 1.32450% with 149 lines in your changes missing coverage. Please review.

Please upload report for BASE (deploy/hammer-humble@3649b5e). Learn more about missing BASE report.

Files Patch % Lines
...ackages/dashboard/src/components/alert-manager.tsx 0.00% 134 Missing ⚠️
packages/dashboard/src/components/appbar.tsx 20.00% 8 Missing ⚠️
...shboard/src/components/tasks/task-cancellation.tsx 0.00% 7 Missing ⚠️
Additional details and impacted files
@@                   Coverage Diff                   @@
##             deploy/hammer-humble     #948   +/-   ##
=======================================================
  Coverage                        ?   13.55%           
=======================================================
  Files                           ?       66           
  Lines                           ?     3099           
  Branches                        ?      911           
=======================================================
  Hits                            ?      420           
  Misses                          ?     2673           
  Partials                        ?        6           
Flag Coverage Δ
dashboard 13.55% <1.32%> (?)

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
…cation alerts checking

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Copy link
Collaborator

@koonpeng koonpeng left a comment

Choose a reason for hiding this comment

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

Haven't look through the whole thing yet.

packages/api-server/api_server/routes/alerts.py Outdated Show resolved Hide resolved
packages/api-server/api_server/routes/alerts.py Outdated Show resolved Hide resolved
packages/api-server/api_server/routes/alerts.py Outdated Show resolved Hide resolved
packages/api-server/api_server/routes/alerts.py Outdated Show resolved Hide resolved
packages/api-server/api_server/repositories/alerts.py Outdated Show resolved Hide resolved
packages/api-server/api_server/repositories/alerts.py Outdated Show resolved Hide resolved
packages/api-server/api_server/repositories/alerts.py Outdated Show resolved Hide resolved
packages/api-server/api_server/repositories/alerts.py Outdated Show resolved Hide resolved
packages/api-server/api_server/gateway.py Outdated Show resolved Hide resolved
packages/api-server/api_server/gateway.py Outdated Show resolved Hide resolved
* First round of cleanup

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Regenerate API

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

---------

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
* Moved custom deliveries to separate file naively and import naively

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Moved patrol

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Moved custom-compose

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Added clean and delivery

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Added delivery, renamed to SimpleDelivery

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Clean task added

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Moved delivery-custom tests, added return type for forms

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Configurable supported tasks and name remapping

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Changed directory to types, since it doesn't just handle descriptions

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Fix test imports

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Using temporary task definition

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Refactoring new rename changes

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Clean up

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Removed problematic and unsused component and test

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Lint

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Updating pnpm version in github workflow

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Reverting update to pnpm version

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Fix build now that we use key value strings for labels

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Refactored last parts of hard coding categories and rendering forms

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Refactor callback names and error handling for misconfigs

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Display error as well

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Fixed more checks and failures

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Split configuration and definition, only handle configurations in resource manager level

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Lint

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Not using object as a type

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Address feedback

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Render using validTasks instead

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Use useMemo

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

---------

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
@aaronchongth aaronchongth changed the base branch from deploy/hammer to deploy/hammer-humble July 3, 2024 09:08
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
…ble"

This reverts commit 1fd22ee.

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
packages/api-server/api_server/gateway.py Outdated Show resolved Hide resolved
packages/api-server/api_server/models/__init__.py Outdated Show resolved Hide resolved
packages/dashboard/src/components/appbar.tsx Outdated Show resolved Hide resolved
packages/dashboard/src/components/appbar.tsx Outdated Show resolved Hide resolved
packages/dashboard/src/components/alert-store.tsx Outdated Show resolved Hide resolved
packages/dashboard/src/components/alert-store.tsx Outdated Show resolved Hide resolved
packages/dashboard/src/components/appbar.tsx Show resolved Hide resolved
@koonpeng
Copy link
Collaborator

koonpeng commented Jul 4, 2024

not sure if this image is still updated, there are multiple issues with the design

  1. The number of available responses is unbounded, better to list them in a dropdown.
  2. The buttons are different size.

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
@koonpeng
Copy link
Collaborator

koonpeng commented Jul 5, 2024

A suggestion for the alert responses is to use split buttons https://mui.com/material-ui/react-button-group/#split-button

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
…ion for unackw query, updated API

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
packages/api-server/api_server/repositories/alerts.py Outdated Show resolved Hide resolved
packages/dashboard/src/components/alert-manager.tsx Outdated Show resolved Hide resolved
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
@aaronchongth aaronchongth merged commit 090dd9b into deploy/hammer-humble Jul 9, 2024
4 checks passed
@aaronchongth aaronchongth deleted the hammer/generic-alerts branch July 9, 2024 16:30
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.

None yet

2 participants