-
Notifications
You must be signed in to change notification settings - Fork 32
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
Allow LATEST in get_limits #683
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #683 +/- ##
=======================================
Coverage 70.80% 70.80%
=======================================
Files 471 471
Lines 30216 30232 +16
Branches 822 822
=======================================
+ Hits 21394 21406 +12
- Misses 8738 8741 +3
- Partials 84 85 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
openc3/lib/openc3/api/limits_api.rb
Outdated
TargetModel.packets(target_name, scope: scope).each do |packet| | ||
item = packet['items'].find { |item| item['name'] == item_name } | ||
if item | ||
# TODO: Fixme: This should be using the CVT not topics - Will possibly choose wrong packet if mixed with stored |
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.
Feels like a good opportunity to fix this?
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.
I didn't write that comment and I'm not sure what it means. The topic returns the time which we use to find 'latest'. The CvtModel only returns values.
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.
The CVT stores the most recent of each packet. The problem with topics is that they contain both realtime and stored data. Whenever you use Topics you have to make sure the data you use isn't stored, and we aren't doing that here.
openc3/lib/openc3/api/limits_api.rb
Outdated
TargetModel.packets(target_name, scope: scope).each do |packet| | ||
item = packet['items'].find { |item| item['name'] == item_name } | ||
if item | ||
hash = JSON.parse(Store.hget("#{scope}__tlm__#{target_name}", packet['packet_name']), :allow_nan => true, :create_additions => true) |
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.
This line should be a CvtModel method.
requested_item = nil | ||
if packet_name == 'LATEST' | ||
latest = -1 | ||
TargetModel.packets(target_name, scope: scope).each do |packet| |
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.
This whole loop should be a CvtModel or TargetModel method to get the latest item.
Co-authored-by: Ryan Melton <ryan@openc3.com>
closes #679