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 #382, Align CF Command Counter implementation to cFS standard #399

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dzbaker
Copy link
Contributor

@dzbaker dzbaker commented Jul 11, 2023

Checklist

Describe the contribution

  • Fixes CF command counters format inconsistent with cFS #382
    • Command counters renamed
      • cmd renamed to CommandCounter
      • err renamed to CommandErrorCounter
    • Command counters changed from size 16 to size 8, and moved out of their own struct into the HK packet directly (this is how all other cFS components/apps implemented the counters)
    • hk renamed to HkPacket (most cFS components/apps use this common name for the housekeeping packet)

Note: if the CamelCase is an issue, can still implement these changes as snake_case, although I guess now is as good a time as any to start converting CF variables to CamelCase. If so I can open a new issue to flag that all variables should be changed over - all at once (if someone wants to implement that) or gradually as changes are made.
CF is the only cFS app/component to use snake_case for naming variables.

Re-introduced from #383.#394, #397 reverted this PR.

Testing performed
GitHub CI actions all passing successfully (incl. Build + Run, Unit Tests etc.).

Expected behavior changes
Command/CommandError counters now held directly in the HK packet, instead of through an additional struct member.

This PR improves consistency across cFS, and makes future maintenance easier as variables are named and located as expected.

Contributor Info
Avi Weiss @thnkslprpt

@dzbaker
Copy link
Contributor Author

dzbaker commented Jul 11, 2023

@thnkslprpt I reverted the original (#383) so we could move it to the next release. Does everything look good here?

@thnkslprpt
Copy link
Contributor

@thnkslprpt I reverted the original (#383) so we could move it to the next release. Does everything look good here?

Yep - looks good mate.
Cheers

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.

CF command counters format inconsistent with cFS
3 participants