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 #45, Apply consistent Event ID names to common events #46

Merged

Conversation

thnkslprpt
Copy link
Contributor

@thnkslprpt thnkslprpt commented Oct 21, 2022

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

Copy link
Contributor

@chillfig chillfig left a comment

Choose a reason for hiding this comment

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

LC_RESET_INF_EID has comments suggesting it is still DEBUG type.

@@ -161,7 +161,7 @@
* Successful execution of this command may be verified with
* the following telemetry:
* - #LC_HkPacket_t.CmdCount will be cleared
* - The #LC_RESET_DBG_EID debug event message will be
* - The #LC_RESET_INF_EID debug event message will be
Copy link
Contributor

Choose a reason for hiding this comment

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

if LC_RESET_INF_EID is to be changed to of an INFORMATION type, these comments may need updating

Suggested change
* - The #LC_RESET_INF_EID debug event message will be
* - The #LC_RESET_INF_EID informational event message will be

@@ -401,7 +401,7 @@
* This event message is issued when a reset counters command has
* been received.
*/
#define LC_RESET_DBG_EID 27
#define LC_RESET_INF_EID 27
Copy link
Contributor

Choose a reason for hiding this comment

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

Line 397 may need updating to Type: INFORMATION.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I've updated both those changes now with the force-push Justin.
Thanks

@thnkslprpt thnkslprpt force-pushed the fix-45-apply-consistent-event-id-names branch from a894892 to d427f97 Compare November 1, 2022 02:57
Copy link
Contributor

@chillfig chillfig 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!

@dzbaker dzbaker added this to the Fornax milestone Nov 21, 2022
@dzbaker dzbaker modified the milestones: Fornax, Equuleus Dec 7, 2022
@thnkslprpt thnkslprpt force-pushed the fix-45-apply-consistent-event-id-names branch from d427f97 to c78ec3a Compare March 12, 2023 03:46
@thnkslprpt thnkslprpt force-pushed the fix-45-apply-consistent-event-id-names branch 2 times, most recently from 1c38e4f to 8ef96e5 Compare April 6, 2023 22:53
@thnkslprpt thnkslprpt force-pushed the fix-45-apply-consistent-event-id-names branch from 8ef96e5 to 4521deb Compare April 18, 2023 00:00
@thnkslprpt thnkslprpt force-pushed the fix-45-apply-consistent-event-id-names branch from 4521deb to e32aa19 Compare May 5, 2023 03:03
@thnkslprpt thnkslprpt force-pushed the fix-45-apply-consistent-event-id-names branch from e32aa19 to af3954b Compare November 1, 2023 17:39
@dzbaker dzbaker merged commit 776ef63 into nasa:main Jul 15, 2024
17 checks passed
@thnkslprpt thnkslprpt deleted the fix-45-apply-consistent-event-id-names branch July 16, 2024 09:27
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.

Inconsistent Event ID naming
3 participants