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

Fix #897, EVS unregistered AppID #898

Merged

Conversation

jphickey
Copy link
Contributor

Describe the contribution
Do not overload the EventCount field to track whether an "unregistered" event was generated, which by definition comes from a different app than the one that might be actively using this same table entry.

This just makes a separate field to track that state, leaving the EventCount to serve its primary purpose of counting actual events generated from a valid/registered AppID.

Fixes #897

Testing performed
Temporarily modified/hacked "sample_app" to send several events before calling CFE_EVS_Register().
Confirmed that one (and only one) "unregistered" event is reported e.g.:

EVS Port1 42/1/CFE_EVS 41: App SAMPLE_APP not registered with Event Services. Unable to send event.

Also confirmed that this "unregistered" event did not change the valid EventCount in the EVS telemetry data.

Expected behavior changes
No more incrementing a counter for another app.

System(s) tested on
Ubuntu 20.04

Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.

Do not use EventCount to track whether an "unregistered" event
was generated, because that by definition came from a different app
than the one that "owns" that field.

This just adds a separate field to track that state, so it doesn't
potentially modify the counter for an unrelated app.
@jphickey jphickey force-pushed the fix-897-evs-unreg-appid branch from 4ed1559 to 02b3a54 Compare September 29, 2020 20:24
@jphickey jphickey marked this pull request as ready for review September 29, 2020 20:24
@jphickey jphickey added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label Sep 29, 2020
@jphickey
Copy link
Contributor Author

Rebased to current main branch

@astrogeco astrogeco added CCB-20200930 and removed CCB:Ready Ready for discussion at the Configuration Control Board (CCB) labels Sep 30, 2020
@astrogeco
Copy link
Contributor

CCB 2020-09-30: APPROVED

@astrogeco
Copy link
Contributor

@jphickey do we need to document this new Unregistered "counter" somewhere?

@astrogeco astrogeco changed the base branch from main to integration-candidate October 1, 2020 21:30
@astrogeco astrogeco merged commit 5a64dd7 into nasa:integration-candidate Oct 1, 2020
@jphickey jphickey deleted the fix-897-evs-unreg-appid branch October 7, 2020 13:41
@skliper skliper added this to the 7.0.0 milestone Sep 24, 2021
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.

EVS improperly uses the EventCount member for incorrect appID filtering
3 participants