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

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.

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 <[email protected]>

* Regenerate API

Signed-off-by: Aaron Chong <[email protected]>

---------

Signed-off-by: Aaron Chong <[email protected]>
* Moved custom deliveries to separate file naively and import naively

Signed-off-by: Aaron Chong <[email protected]>

* Moved patrol

Signed-off-by: Aaron Chong <[email protected]>

* Moved custom-compose

Signed-off-by: Aaron Chong <[email protected]>

* Added clean and delivery

Signed-off-by: Aaron Chong <[email protected]>

* Added delivery, renamed to SimpleDelivery

Signed-off-by: Aaron Chong <[email protected]>

* Clean task added

Signed-off-by: Aaron Chong <[email protected]>

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

Signed-off-by: Aaron Chong <[email protected]>

* Configurable supported tasks and name remapping

Signed-off-by: Aaron Chong <[email protected]>

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

Signed-off-by: Aaron Chong <[email protected]>

* Fix test imports

Signed-off-by: Aaron Chong <[email protected]>

* Using temporary task definition

Signed-off-by: Aaron Chong <[email protected]>

* Refactoring new rename changes

Signed-off-by: Aaron Chong <[email protected]>

* Clean up

Signed-off-by: Aaron Chong <[email protected]>

* Removed problematic and unsused component and test

Signed-off-by: Aaron Chong <[email protected]>

* Lint

Signed-off-by: Aaron Chong <[email protected]>

* Updating pnpm version in github workflow

Signed-off-by: Aaron Chong <[email protected]>

* Reverting update to pnpm version

Signed-off-by: Aaron Chong <[email protected]>

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

Signed-off-by: Aaron Chong <[email protected]>

* Refactored last parts of hard coding categories and rendering forms

Signed-off-by: Aaron Chong <[email protected]>

* Refactor callback names and error handling for misconfigs

Signed-off-by: Aaron Chong <[email protected]>

* Display error as well

Signed-off-by: Aaron Chong <[email protected]>

* Fixed more checks and failures

Signed-off-by: Aaron Chong <[email protected]>

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

Signed-off-by: Aaron Chong <[email protected]>

* Lint

Signed-off-by: Aaron Chong <[email protected]>

* Not using object as a type

Signed-off-by: Aaron Chong <[email protected]>

* Address feedback

Signed-off-by: Aaron Chong <[email protected]>

* Render using validTasks instead

Signed-off-by: Aaron Chong <[email protected]>

* Use useMemo

Signed-off-by: Aaron Chong <[email protected]>

---------

Signed-off-by: Aaron Chong <[email protected]>
Signed-off-by: Aaron Chong <[email protected]>
@aaronchongth aaronchongth changed the base branch from deploy/hammer to deploy/hammer-humble July 3, 2024 09:08
Signed-off-by: Aaron Chong <[email protected]>
Signed-off-by: Aaron Chong <[email protected]>
Signed-off-by: Aaron Chong <[email protected]>
Signed-off-by: Aaron Chong <[email protected]>
Signed-off-by: Aaron Chong <[email protected]>
Signed-off-by: Aaron Chong <[email protected]>
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 <[email protected]>
@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

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
@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
aaronchongth added a commit that referenced this pull request Jul 22, 2024
* First iteration of generic alerts

Signed-off-by: Aaron Chong <[email protected]>

* Route for unresponded alerts

Signed-off-by: Aaron Chong <[email protected]>

* Basic location complete post route for task

Signed-off-by: Aaron Chong <[email protected]>

* Tests for alert routes

Signed-off-by: Aaron Chong <[email protected]>

* Test for location complete route

Signed-off-by: Aaron Chong <[email protected]>

* Lint and tests

Signed-off-by: Aaron Chong <[email protected]>

* Basic implementation

Signed-off-by: Aaron Chong <[email protected]>

* Basic task based alerts working as expected

Signed-off-by: Aaron Chong <[email protected]>

* Only display and list alerts that have display true

Signed-off-by: Aaron Chong <[email protected]>

* Add doc to location_complete route about instability

Signed-off-by: Aaron Chong <[email protected]>

* Refactored alert db interactions to repository, with lru cache for location alerts checking

Signed-off-by: Aaron Chong <[email protected]>

* Regenerated docs and return early on gateway last location checking

Signed-off-by: Aaron Chong <[email protected]>

* Lint and better typing with static methods

Signed-off-by: Aaron Chong <[email protected]>

* Hammer/generic alerts without location (#951)

* First round of cleanup

Signed-off-by: Aaron Chong <[email protected]>

* Regenerate API

Signed-off-by: Aaron Chong <[email protected]>

---------

Signed-off-by: Aaron Chong <[email protected]>

* Hammer/demo tasks (#925)

* Moved custom deliveries to separate file naively and import naively

Signed-off-by: Aaron Chong <[email protected]>

* Moved patrol

Signed-off-by: Aaron Chong <[email protected]>

* Moved custom-compose

Signed-off-by: Aaron Chong <[email protected]>

* Added clean and delivery

Signed-off-by: Aaron Chong <[email protected]>

* Added delivery, renamed to SimpleDelivery

Signed-off-by: Aaron Chong <[email protected]>

* Clean task added

Signed-off-by: Aaron Chong <[email protected]>

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

Signed-off-by: Aaron Chong <[email protected]>

* Configurable supported tasks and name remapping

Signed-off-by: Aaron Chong <[email protected]>

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

Signed-off-by: Aaron Chong <[email protected]>

* Fix test imports

Signed-off-by: Aaron Chong <[email protected]>

* Using temporary task definition

Signed-off-by: Aaron Chong <[email protected]>

* Refactoring new rename changes

Signed-off-by: Aaron Chong <[email protected]>

* Clean up

Signed-off-by: Aaron Chong <[email protected]>

* Removed problematic and unsused component and test

Signed-off-by: Aaron Chong <[email protected]>

* Lint

Signed-off-by: Aaron Chong <[email protected]>

* Updating pnpm version in github workflow

Signed-off-by: Aaron Chong <[email protected]>

* Reverting update to pnpm version

Signed-off-by: Aaron Chong <[email protected]>

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

Signed-off-by: Aaron Chong <[email protected]>

* Refactored last parts of hard coding categories and rendering forms

Signed-off-by: Aaron Chong <[email protected]>

* Refactor callback names and error handling for misconfigs

Signed-off-by: Aaron Chong <[email protected]>

* Display error as well

Signed-off-by: Aaron Chong <[email protected]>

* Fixed more checks and failures

Signed-off-by: Aaron Chong <[email protected]>

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

Signed-off-by: Aaron Chong <[email protected]>

* Lint

Signed-off-by: Aaron Chong <[email protected]>

* Not using object as a type

Signed-off-by: Aaron Chong <[email protected]>

* Address feedback

Signed-off-by: Aaron Chong <[email protected]>

* Render using validTasks instead

Signed-off-by: Aaron Chong <[email protected]>

* Use useMemo

Signed-off-by: Aaron Chong <[email protected]>

---------

Signed-off-by: Aaron Chong <[email protected]>

* Update ros2 pydantic messages, fix tests

Signed-off-by: Aaron Chong <[email protected]>

* Revert update of pnpm lock file

Signed-off-by: Aaron Chong <[email protected]>

* Lint

Signed-off-by: Aaron Chong <[email protected]>

* Use specific exceptions and more clean up

Signed-off-by: Aaron Chong <[email protected]>

* Port CI fixes from #955 but targeting ubuntu 22 and ROS 2 Humble

Signed-off-by: Aaron Chong <[email protected]>

* Revert "Port CI fixes from #955 but targeting ubuntu 22 and ROS 2 Humble"

This reverts commit 1fd22ee.

Signed-off-by: Aaron Chong <[email protected]>

* Update pnpm version

Signed-off-by: Aaron Chong <[email protected]>

* Setup pnpm and node manually

Signed-off-by: Aaron Chong <[email protected]>

* Using HOME env var of runner

Signed-off-by: Aaron Chong <[email protected]>

* workflow

Signed-off-by: Aaron Chong <[email protected]>

* workflow

Signed-off-by: Aaron Chong <[email protected]>

* Workflow

Signed-off-by: Aaron Chong <[email protected]>

* Address feedback on frontend

Signed-off-by: Aaron Chong <[email protected]>

* Address comments that don't break behavior

Signed-off-by: Aaron Chong <[email protected]>

* Started generic exceptions

Signed-off-by: Aaron Chong <[email protected]>

* Proper db schema for alert request and response

Signed-off-by: Aaron Chong <[email protected]>

* Updated alerts model, frontend tweaks for subscription, using pagination for unackw query, updated API

Signed-off-by: Aaron Chong <[email protected]>

* Attempt to setup minimal RMF during bootstrap step

Signed-off-by: Aaron Chong <[email protected]>

* Fix mistake on ci path

Signed-off-by: Aaron Chong <[email protected]>

* Fix workflow

Signed-off-by: Aaron Chong <[email protected]>

* Revert CI changes

Signed-off-by: Aaron Chong <[email protected]>

* Updated API, renamed event to pushAlert, filter before

Signed-off-by: Aaron Chong <[email protected]>

---------

Signed-off-by: Aaron Chong <[email protected]>
(cherry picked from commit 090dd9b)
Signed-off-by: Aaron Chong <[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.

2 participants