Allow robot created notifications

Gold872 / elastic-dashboard

A simple and modern dashboard for FRC

MIT License

71 stars 13 forks source link

Allow robot created notifications #48

Closed EmeraldWither closed 2 months ago

EmeraldWither commented 2 months ago

https://github.com/Gold872/elastic-dashboard/assets/68785503/6e22f1bb-cafc-4cf4-82bd-e77da0f40d26

Robot code usage:

 @Override
  public void autonomousInit() {
    Elastic.sendAlert(AlertLevel.INFO, "Autonomous!", "The robot is now in auto.");

There are 3 levels of notifications:

INFO:

WARNING:

ERROR:

The idea is that the driver can get a notification when something happens (eg. an arm extends too far and gets disabled).

Users can copy the Elastic class into their robot project, and use those methods to push notifications to the dashboard.

EmeraldWither commented 2 months ago

Hey thanks for looking at this.

There should also be some way of unit testing this, the PR for restructuring the NT dependency makes testing it much easier, so it might make more sense to add unit tests for notifications after the rework is complete

Unit testing is something that I haven't done before (😓), but my summer break starts today, so I will take a look at how to do this and will put in later.

codecov[bot] commented 2 months ago

Files	Patch %	Lines
lib/services/robot_notifications_listener.dart	13.63%	19 Missing :warning:
lib/pages/dashboard_page.dart	66.66%	1 Missing :warning:

```diff @@ Coverage Diff @@ ## main #48 +/- ## ========================================== - Coverage 45.53% 45.45% -0.08% ========================================== Files 73 74 +1 Lines 7465 7489 +24 ========================================== + Hits 3399 3404 +5 - Misses 4066 4085 +19 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Gold872 commented 2 months ago

@EmeraldWither quick question, what's the reason for having it send all if it ignores the notification on initial connection?

Also, I merged all of these changes into the nt-dependency-rework branch, so you can add unit tests for this much easier. Whenever you're ready, just make a PR to that branch. Let me know if you need any help with the tests

This is definitely a great addition, thanks for making this!

EmeraldWither commented 2 months ago

On the initial connection, when I start listening, it will always call the method with whatever is currently inside the topic, even if it's old/stale data (eg, rebooting elastic will cause the notification so show again even though we haven't actually posted anything).

By ignoring the first time that this happens, this allows us to only receive the new data.

Gold872 / elastic-dashboard

Allow robot created notifications #48

Codecov Report