-
Notifications
You must be signed in to change notification settings - Fork 25
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 #52, Apply consistent Event ID names to common events #53
Fix #52, Apply consistent Event ID names to common events #53
Conversation
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.
CS_RESET_INF_EID
has header comment in its definition that suggest it is Debug type. May just need an update on the header comment to change from Type: DEBUG to Type: INFORMATION
fsw/src/cs_events.h
Outdated
@@ -60,7 +60,7 @@ | |||
* | |||
* This event message is issued when a reset counters command has been received. | |||
*/ | |||
#define CS_RESET_DBG_EID 3 | |||
#define CS_RESET_INF_EID 3 |
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.
Why does this event name change from using DBG to INF?
Line 57 may need an update to indicate this event is informational type.
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.
Should be updated now Justin.
d3335ce
to
6ab5451
Compare
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.
Looks good!
Cheers mate. |
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 missed this. fsw/src/cs_table_processing.c/line 112 needs a semi-colon.
6ab5451
to
7bdfb22
Compare
CCB:2023.03.09: Putting on hold until naming conventions for event, etc are merged. |
7bdfb22
to
57d7b56
Compare
57d7b56
to
a5f0cf4
Compare
de94f05
to
c31a276
Compare
c31a276
to
ed928b0
Compare
It looks like in the recent rebase, some updates got merged incorrectly, causing the workflow failure |
ed928b0
to
ba3051c
Compare
Rebased to latest mainline and manually fixed up some merge overlaps re: payload member. Workflows now pass and this should be good to go. |
Thanks Joe. |
Checklist
Describe the contribution
Testing performed
Only GitHub CI actions.
Expected behavior changes
No impact on code behavior (no logic changes).
Consistent Event ID names for the events which are common to all/most cFS components and apps will improve consistency and ease make code review/debugging easier.
Contributor Info
Avi Weiss @thnkslprpt