-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[Greedy LAD] Add new functionality for Latest Available Data views #6432
Conversation
mergin master.
…utside the current time bounds
…s data outside the time bounds
…and is not outside of time bounds
…current time bounds
Codecov Report
@@ Coverage Diff @@
## master #6432 +/- ##
==========================================
- Coverage 54.83% 54.79% -0.04%
==========================================
Files 626 626
Lines 26591 26616 +25
Branches 2403 2403
==========================================
+ Hits 14580 14585 +5
- Misses 11348 11368 +20
Partials 663 663
*This pull request uses carry forward flags. Click here to find out more.
... and 6 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
@jvigliotta do you have an issue for this bad boy? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks really good, thank you.
I don't think the notifications are scalable. We have displays with like 50 LAD views in them, so I think we need to ship this initially with no notifications until we come up with a better UI solution to this.
I suspect that we can simplify things by creating a separate class for handling LAD telemetry, like a LADTelemetryHandler
or something, but that's a problem for another time.
I do not! I'll work with @akhenry to get one, I don't have access to the original |
this.emit('dataInsideTimeBounds'); | ||
this.openmct.notifications.info(`Latest available data for "${this.domainObject.name}" is no longer outside of current time bounds`); | ||
if (this.dataOutsideTimeBounds) { | ||
this.dataOutsideTimeBounds = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this flag actually used for anything any more?
follow up for tests: #6451 |
Closes #6449
Describe your changes:
Have added a method to the Telemetry API that will allow you to turn off "Greedy LAD". Greedy LAD functionality has been enabled by default in the Telemetry API. When a Telemetry Collection is used with a strategy of 'latest', if Greedy LAD is active (which it is by default), then the Telemetry Collection will no longer drop data that falls before the start bound if no new data has come in to replace existing data. It will continue to hold onto the data until new data comes in.
All Submissions:
It's backwards compatible, but it does change some default functionality to add more visibility to data, but it is able to be turned off.
Author Checklist
Reviewer Checklist