-
Notifications
You must be signed in to change notification settings - Fork 212
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
Fix #2095, Automatic suppression of flooding events #2109
Conversation
CLA incoming; will have to submit to my management to fill out |
@jhnphm We just got a new consolidated form that includes all the apps, I'll email it. |
Updated unit test to improve coverage, and fixed doc errors. Updated commit message to add more detail. |
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 think overall there are some concerns that need to be resolved here. Simplify the logic, mainly.
I didn't look too closely at the unit tests but it seems to require modification to every test case. This is definitely not ideal, I'd try to avoid that if at all possible. I would think that it could be worked around by giving each app a huge amount of credit during the common setup function (e.g. INT32_MAX if you also use my suggestion of going with a signed credit value and a decrement)
I simplified it a bit, but still requires resetting the token count to |
This adds automatic event suppression, which works by decrementing a token counter every time an event message is sent. When it becomes negative an event message is emitted and further event messages are squelched until the tokens increase above 0. Tokens are returned (incremented) at the rate of CFE_PLATFORM_EVS_APP_EVENTS_PER_SEC, upon the next event message call for the app, based on time since last token return. This avoids a dependency on HK wake messages, which are not guaranteed to be at 1Hz, or received at all. There are limits in place to prevent tokens from being above CFE_PLATFORM_EVS_MAX_APP_EVENT_BURST and below -CFE_PLATFORM_EVS_MAX_APP_EVENT_BURST. The initial value upon registration is CFE_PLATFORM_EVS_MAX_APP_EVENT_BURST.
* Use EVS global lock * Use increments of 1000 for tokens to avoid integer division * Use global variable instead of using platform_cfg file defines directly to allow unit tests to override value * Fix EVS event message ID
…code for events being squelched.
…kspace/phamj/CSIC9/hfi/cfe into auto-event-suppression
@skliper @jphickey Added the changes requested/bugs pointed out during today's meeting. I'm using the global EVS mutex if that's okay. Also updated docs and took a first cut at requirements in the CSV. Not sure if the diagram included is detailed enough; it's a notional state diagram, but there really isn't a state machine in the code. I have this activity diagram but it might be too "busy": |
Closed in favor of #2117 |
Damn skliper you really did jhnphm dirty... |
@justinvvitale - I should have been more verbose in my closure statement. Transfer was coordinated w/ all parties involved to straighten out contribution ownership based on the various organizations and contracts involved with this work. There's significant process involved behind the scenes in some external contribution situations, and I realize the optics don't look good w/o context. :) |
Checklist (Please check before submitting)
Describe the contribution
Testing performed
Expected behavior changes
A clear and concise description of how this contribution will change behavior and level of impact.
System(s) tested on
Additional context
N/A
Third party code
N/A
Contributor Info - All information REQUIRED for consideration of pull request
John N Pham (@jhnphm), Northrop Grumman, Space Systems