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 #2514, change CFE_MSG_Message from union to struct #2515

Merged
merged 1 commit into from
Feb 23, 2024

Conversation

jphickey
Copy link
Contributor

Checklist (Please check before submitting)

Describe the contribution
Having this abstract type as a "struct" makes it match the Command and Telemetry abstract types. Futhermore, it better conveys the intent that this is an abstract object and should not be directly used or accessed in other ways.

It may still be implemented as a union underneath (depending on how MSG module chooses to implement) but that is hidden from public API. In the case of the default MSG module implementation, there are just a handful of cases where it is accessed internally as bytes, and those are simple enough to do with a cast.

Fixes #2514

Testing performed
Build and run all tests

Expected behavior changes
API simplification

System(s) tested on
Debian

Additional context
There is a (very slight) possibility that some app code has used the union CFE_MSG_Message directly, and the change from union to struct will cause an error if any such code exists (but that could should be fixed anyway). None of the current CFS apps appear to do this, so its OK in that domain.

If any users have implemented a custom MSG module, this will require a matching/corresponding update in that implementation.

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

Having this abstract type as a "struct" makes it match the Command
and Telemetry abstract types.  Futhermore, it better conveys the
intent that this is an abstract object and should not be directly
used or accessed in other ways.

It may still be implemented as a union underneath (depending on
how MSG module chooses to implement) but that is hidden from
public API.  In the case of the default MSG module implementation,
there are just a handful of cases where it is accessed internally
as bytes, and those are simple enough to do with a cast.
@jphickey jphickey added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label Feb 13, 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 Feb 15, 2024
dzbaker added a commit to nasa/cFS that referenced this pull request Feb 22, 2024
*Combines:*

cFE equuleus-rc1+dev94
osal equuleus-rc1+dev47
to_lab equuleus-rc1+dev42

**Includes:**

*cFE*
- nasa/cFE#2515

*osal*
- nasa/osal#1448
- nasa/osal#1450

*to_lab*
- nasa/to_lab#191

Co-authored by: Joseph Hickey <[email protected]>
Co-authored by: Avi Weiss <[email protected]>
dzbaker added a commit to nasa/cFS that referenced this pull request Feb 23, 2024
*Combines:*

cFE equuleus-rc1+dev96
osal equuleus-rc1+dev53
to_lab equuleus-rc1+dev44

**Includes:**

*cFE*
- nasa/cFE#2515
- nasa/cFE#2330

*osal*
- nasa/osal#1448
- nasa/osal#1146
- nasa/osal#1357
- nasa/osal#1354
- nasa/osal#1331

*to_lab*
- nasa/to_lab#191
- nasa/to_lab#136

Co-authored by: Joseph Hickey <[email protected]>
Co-authored by: Avi Weiss <[email protected]>
Co-authored by: Sam Price <[email protected]>
@dzbaker dzbaker merged commit daf86b9 into nasa:main Feb 23, 2024
22 checks passed
dzbaker added a commit to nasa/cFS that referenced this pull request Feb 23, 2024
*Combines:*

cFE equuleus-rc1+dev96
osal equuleus-rc1+dev53
to_lab equuleus-rc1+dev44

**Includes:**

*cFE*
- nasa/cFE#2515
- nasa/cFE#2330

*osal*
- nasa/osal#1448
- nasa/osal#1146
- nasa/osal#1357
- nasa/osal#1354
- nasa/osal#1331

*to_lab*
- nasa/to_lab#191
- nasa/to_lab#136

Co-authored by: Joseph Hickey <[email protected]>
Co-authored by: Avi Weiss <[email protected]>
Co-authored by: Sam Price <[email protected]>
@jphickey jphickey deleted the fix-2514-msg-api-struct branch March 7, 2024 01:07
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CFE_MSG_Message_t should really be a struct, not a union
2 participants