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 #2335, generalize update header parameter #2336

Merged
merged 1 commit into from
May 22, 2023

Conversation

jphickey
Copy link
Contributor

Checklist (Please check before submitting)

Describe the contribution
Expand the "Increment Sequence" boolean on transmit message functions to be a more general "Update Header" boolean, so it can be used for other fields like timestamps, checksums, or user-defined fields too.

Fixes #2335
Fixes #1157

Testing performed
Build and run CFE, sanity check, confirm headers are as expected

Expected behavior changes
Headers will be updated more consistently/generically.

Note -- Messages classified as Commands will have headers updated, too - specifically the sequence count and checksum. This will apply to messages being created by SCH/SCH_LAB, and the time signals sent by CFE TIME. Previously the sequence counter was not set in any of these messages, so it would have always been zero. Hence receivers would not be relying on it for anything, and thus setting it nonzero shouldn't break anything, either. But there is a slight chance that some code had a dependency on this always being zero.

System(s) tested on
Debian

Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.

Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

CodeQL found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.

Expand the "Increment Sequence" boolean on transmit message functions to
be a more general "Update Header" boolean, so it can be used for other
fields like timestamps, checksums, or user-defined fields too.
@jphickey jphickey added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label May 18, 2023
@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 May 18, 2023
dzbaker added a commit to nasa/cFS that referenced this pull request May 22, 2023
*Combines:*

cFE v7.0.0-rc4+dev318
sch_lab v2.5.0-rc4+dev57

**Includes:**

*cFE*
- nasa/cFE#2336
- nasa/cFE#2338

*sch_lab*
- nasa/sch_lab#145

Co-authored by: Daniel Knutsen <dmknutsen@users.noreply.github.com>
Co-authored by: Joseph Hickey <jphickey@users.noreply.github.com>
@dzbaker dzbaker mentioned this pull request May 22, 2023
2 tasks
@dzbaker dzbaker merged commit 691f1f7 into nasa:main May 22, 2023
21 checks passed
dzbaker added a commit to nasa/cFS that referenced this pull request May 22, 2023
*Combines:*

cFE v7.0.0-rc4+dev318
sch_lab v2.5.0-rc4+dev57

**Includes:**

*cFE*
- nasa/cFE#2336
- nasa/cFE#2338

*sch_lab*
- nasa/sch_lab#144

Co-authored by: Daniel Knutsen <dmknutsen@users.noreply.github.com>
Co-authored by: Joseph Hickey <jphickey@users.noreply.github.com>
@dmknutsen dmknutsen added this to the Equuleus milestone May 26, 2023
@jphickey jphickey deleted the fix-2335-update-header branch May 31, 2023 17:03
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 Equuleus-rc1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow updating more than just SequenceCount on message transmit Auto increment sequence on CMD packets
4 participants