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 #1448, Simplify and clarify EVS_AddLog logic #2309

Merged
merged 1 commit into from
Dec 10, 2024

Conversation

thnkslprpt
Copy link
Contributor

Checklist

Describe the contribution

Testing performed
GitHub CI actions all passing successfully.

Would be good to add functional tests for this in the future - I noticed that changing the second block to just a simple else also passes all the coverage tests.

Expected behavior changes
No change.

Contributor Info
Avi Weiss @thnkslprpt

@thnkslprpt
Copy link
Contributor Author

I can also update to the more terse version if there is a general desire to move in that direction as updates are made:

if (CFE_EVS_Global.EVS_LogPtr->LogFullFlag)
...
if ((!CFE_EVS_Global.EVS_LogPtr->LogFullFlag) ||
...

Side note - LogFullFlag is actually not a bool type - although it's treated as such.

Works fine obviously, but I think I'll open a new issue about it - just checking if there are any other similar cases first (ints being treated as bools). Probably left over from earlier times before bool was commonly used.

@chillfig chillfig self-requested a review November 7, 2024 19:01
@chillfig chillfig added CCB:Ready Ready for discussion at the Configuration Control Board (CCB) enhancement labels Nov 7, 2024
@dzbaker dzbaker added CCB:Approved Indicates code review and approval by community CCB and removed CCB:Ready Ready for discussion at the Configuration Control Board (CCB) labels Nov 7, 2024
dzbaker added a commit to nasa/cFS that referenced this pull request Dec 10, 2024
*Combines:*

cFE equuleus-rc1+dev219
osal equuleus-rc1+dev93
PSP equuleus-rc1+dev55
cFS-GroundSystem equuleus-rc1+dev14

**Includes:**

*cFE*
- nasa/cFE#2308
- nasa/cFE#2612
- nasa/cFE#2616
- nasa/cFE#2309

*osal*
- nasa/osal#1486

*PSP*
- nasa/PSP#441

*cFS-GroundSystem*
- nasa/cFS-GroundSystem#233
- nasa/cFS-GroundSystem#235
- nasa/cFS-GroundSystem#236

Co-authored by: Avi Weiss <thnkslprpt@users.noreply.github.com>
Co-authored by: Tvisha Andharia <tandharia@users.noreply.github.com>
Co-authored by: Chris Knight <CDKnightNASA@users.noreply.github.com>
@dzbaker dzbaker mentioned this pull request Dec 10, 2024
2 tasks
dzbaker added a commit to nasa/cFS that referenced this pull request Dec 10, 2024
*Combines:*

cFE equuleus-rc1+dev219
osal equuleus-rc1+dev93
PSP equuleus-rc1+dev55
cFS-GroundSystem equuleus-rc1+dev14

**Includes:**

*cFE*
- nasa/cFE#2308
- nasa/cFE#2612
- nasa/cFE#2616
- nasa/cFE#2309

*osal*
- nasa/osal#1486

*PSP*
- nasa/PSP#441

*cFS-GroundSystem*
- nasa/cFS-GroundSystem#233
- nasa/cFS-GroundSystem#235
- nasa/cFS-GroundSystem#236

Co-authored by: Avi Weiss <thnkslprpt@users.noreply.github.com>
Co-authored by: Tvisha Andharia <tandharia@users.noreply.github.com>
Co-authored by: Chris Knight <CDKnightNASA@users.noreply.github.com>
@dzbaker dzbaker merged commit 25336b2 into nasa:main Dec 10, 2024
dzbaker added a commit to nasa/cFS that referenced this pull request Dec 10, 2024
*Combines:*

cFE equuleus-rc1+dev219
osal equuleus-rc1+dev93
PSP equuleus-rc1+dev55
cFS-GroundSystem equuleus-rc1+dev14

**Includes:**

*cFE*
- nasa/cFE#2308
- nasa/cFE#2612
- nasa/cFE#2616
- nasa/cFE#2309

*osal*
- nasa/osal#1486

*PSP*
- nasa/PSP#441

*cFS-GroundSystem*
- nasa/cFS-GroundSystem#233
- nasa/cFS-GroundSystem#235
- nasa/cFS-GroundSystem#236

Co-authored by: Avi Weiss <thnkslprpt@users.noreply.github.com>
Co-authored by: Tvisha Andharia <tandharia@users.noreply.github.com>
Co-authored by: Chris Knight <CDKnightNASA@users.noreply.github.com>
@thnkslprpt thnkslprpt deleted the fix-1448-simplify-EVS_AddLog-logic branch December 10, 2024 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CCB:Approved Indicates code review and approval by community CCB enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Simplify LogOverflowCounter increment logic
4 participants