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 #15, Refactor LC_SendHkCmd() to reduce switch duplication #93

Merged
merged 1 commit into from
Jun 1, 2023

Conversation

thnkslprpt
Copy link
Contributor

Checklist

Describe the contribution

  • Fixes Refactor LC_SampleAPReq and LC_HousekeepingReq #15
    • Refactors LC_HousekeepingReq() (previously named LC_SampleAPReq()) to combine 4 switch blocks into 1 for the watch results, and 4 switch blocks into 2 for the action points.
    • Minor update to LC_SampleAPReq() to reduce nesting a little (early continue in for loop)
      • The only other thing I can see to simplify LC_SampleAPReq is to add an early return to the first if condition, or to combine the 1st if condition with the 2nd (which results in a triple if-condition which is less clear than the current implementation) - it did not seem worth making these changes.

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

Expected behavior changes
No change to behavior.

Contributor Info
Avi Weiss @thnkslprpt

fsw/src/lc_cmds.c Fixed Show fixed Hide fixed
@thnkslprpt thnkslprpt force-pushed the fix-15-refactor-LC_SendHkCmd branch from b24bc2b to cb0fe14 Compare May 17, 2023 01:32
@dzbaker dzbaker requested a review from chillfig May 18, 2023 18:37
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 great! Thank you

@dzbaker dzbaker merged commit feb5bf5 into nasa:main Jun 1, 2023
@thnkslprpt thnkslprpt deleted the fix-15-refactor-LC_SendHkCmd branch June 4, 2023 09:42
@dmknutsen dmknutsen modified the milestones: Draco, Equuleus Aug 3, 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.

Refactor LC_SampleAPReq and LC_HousekeepingReq
4 participants