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.

Sign up for GitHub

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

Jump to bottom

Fix #87, Change EVS_Register failure from SendEvent to WriteToSysLog #88

Merged
merged 1 commit into from
Mar 30, 2023
Merged

Fix #87, Change EVS_Register failure from SendEvent to WriteToSysLog #88

merged 1 commit into from
Mar 30, 2023
+9 −12

Conversation

thnkslprpt
Copy link
Contributor

@thnkslprpt thnkslprpt commented Mar 4, 2023 •
edited
Loading

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
jphickey
jphickey requested changes Mar 16, 2023
View reviewed changes
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.

unit-test/fm_app_tests.c Outdated

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
@thnkslprpt thnkslprpt requested a review from jphickey March 16, 2023 23:57
jphickey
jphickey approved these changes Mar 21, 2023
View reviewed changes
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
Reviewers

@jphickey jphickey jphickey approved these changes

Assignees
No one assigned
Labels
CCB:Approved enhancement Equuleus-rc1
Projects
None yet
Milestone
Equuleus
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
@thnkslprpt @jphickey @dzbaker @dmknutsen @chillfig