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 #87, Change EVS_Register failure from SendEvent to WriteToSysLog #88

Conversation

thnkslprpt
Copy link
Contributor

@thnkslprpt thnkslprpt commented Mar 4, 2023

Checklist

Describe the contribution
Fixes #87
CFE_EVS_SendEvent replaced by CFE_ES_WriteToSysLog on failure of CFE_EVS_Register.

Testing performed
GitHub CI actions (incl. Build + Run, Unit Tests etc.) all passing successfully.
Coverage maintained at 100%:

  lines......: 100.0% (1443 of 1443 lines)
  functions..: 100.0% (72 of 72 functions)
  branches...: 100.0% (525 of 525 branches)

Expected behavior changes
Failure can be successfully recorded somewhere even without the EVS now.

Contributor Info
Avi Weiss @thnkslprpt

@dzbaker dzbaker requested a review from jphickey March 16, 2023 18:40
Copy link
Contributor

@jphickey jphickey left a comment

Choose a reason for hiding this comment

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

Minor update requested, but otherwise looks good! Thanks.


call_count_CFE_EVS_SendEvent = UT_GetStubCount(UT_KEY(CFE_EVS_SendEvent));
UtAssert_True(call_count_CFE_EVS_SendEvent == 0, "CFE_EVS_SendEvent was called %u time(s), expected 0",
call_count_CFE_EVS_SendEvent);
Copy link
Contributor

Choose a reason for hiding this comment

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

I would recommend taking out these two assertions, no real need to assert that a function was not called. Also no need to add the global variable either.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No worries @jphickey
Removed those two.

@thnkslprpt thnkslprpt force-pushed the fix-87-change-EVS_Register-failure-to-WriteToSysLog branch 3 times, most recently from 1774fa5 to 24042f1 Compare March 16, 2023 23:06
@thnkslprpt thnkslprpt force-pushed the fix-87-change-EVS_Register-failure-to-WriteToSysLog branch from 24042f1 to 9d5d771 Compare March 16, 2023 23:13
Copy link
Contributor

@jphickey jphickey left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@dzbaker dzbaker merged commit 274c8bf into nasa:main Mar 30, 2023
@thnkslprpt thnkslprpt deleted the fix-87-change-EVS_Register-failure-to-WriteToSysLog branch March 30, 2023 20:47
@dmknutsen dmknutsen added this to the Equuleus milestone May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FM tries to send an event message on failure of registering with EVS
5 participants