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

re #479 Update Limits Monitor to differentiate between ignoring stale… #500

Merged
merged 4 commits into from Jul 7, 2017

Conversation

donaldatball
Copy link
Contributor

… packets and ignoring out-of-limits items. Also rework the GUI a little bit so full item names are displayed, and provide the option to ignore items or packets for out-of-limits items.

… packets and ignoring out-of-limits items. Also rework the GUI a little bit so full item names are displayed, and provide the option to ignore items or packets for out-of-limits items.
@donaldatball
Copy link
Contributor Author

@ryanatball @jasonatball Guys, I modified limits monitor quite a bit with this change, see if you like the changes before I start wrenching on the AHK stuff.

Big changes:

  • Limits monitor now differentiates between ignoring a packet and ignoring packet staleness. That is, if packet staleness is being ignored, but the packet is received and a field is out-of-limts, it'll still show up (this is Jason's original request, I think).
  • I added the target and packet name to the GUI so it is easier to figure out what it out of limits.
  • I added a button to block the entire packet associated with an item instead of just the item.
  • The config file now supports IGNORE_ITEM IGNORE_PKT, and IGNORE_STALE

@coveralls
Copy link

coveralls commented Jul 5, 2017

Coverage Status

Coverage decreased (-0.004%) to 84.76% when pulling b458766 on limits_mon_staleness into d6aa756 on master.

@ghost
Copy link

ghost commented Jul 5, 2017

Looks good so far. Can you add timestamps to the log messages on the Log tab?

@ghost
Copy link

ghost commented Jul 5, 2017

Looks good however I noticed that the text color in the value box appears to be one second (one refresh period) slower than the bar moving and the overall state. Has this always been like this?

@donaldatball
Copy link
Contributor Author

@ryanatball Yup, sure thing.

@jasonatball Yeah, I didn't mess with the rate for the thread that updates the values. I can, though.

index = @stale.delete_item(item)
widget = @items.delete("#{target_name} #{packet_name}") if index
@remove_item_callback.call(widget) if widget
# TBD get all out-of-limits items for the packet that is no longer stale.
Copy link

Choose a reason for hiding this comment

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

I'm not sure this TBD is necessary. Won't the CmdTlmServer notify you of everything that is out of limits?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it will-- that comment is wrong and I'll make it go away.

Donald Hall added 2 commits July 5, 2017 16:39
@coveralls
Copy link

coveralls commented Jul 6, 2017

Coverage Status

Coverage increased (+0.01%) to 84.776% when pulling 6069a4d on limits_mon_staleness into d6aa756 on master.

@coveralls
Copy link

coveralls commented Jul 6, 2017

Coverage Status

Coverage decreased (-0.004%) to 84.76% when pulling 106aaff on limits_mon_staleness into d6aa756 on master.

@donaldatball
Copy link
Contributor Author

@ryanatball @jasonatball Ok, I've addressed all comments and the AHK is updated. I think this should be ready for final review.

@ghost
Copy link

ghost commented Jul 7, 2017

Looks like it's working well. 👍

@ghost
Copy link

ghost commented Jul 7, 2017

👍

@donaldatball donaldatball merged commit 210ca71 into master Jul 7, 2017
@donaldatball donaldatball deleted the limits_mon_staleness branch July 7, 2017 16:25
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