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 #1949, update msgid testcase to match implementation #1950

Merged

Conversation

jphickey
Copy link
Contributor

@jphickey jphickey commented Sep 14, 2021

Describe the contribution
Updates the test case for "CFE_MSG_SetMsgId()" to write a value that is outside the set of storable values for MsgId. This currently
uses the value "-1" do this, assuming that this translates to a value with all bits set, and at least one of those bits will not correlate with an actual header field.

Caveat is that in other implementations, it is possible that all bits are storeable in the header. That is, all MsgId values can be stored in a message or vice versa, even those MsgIds which are not route-able in SB.

This also updates the documentation of CFE_MSG_SetMsgId to clarify if/when the CFE_MSG_BAD_ARGUMENT status code is returned, and that the set of MsgId values which are "settable" really depends on the MSG implementation.

Fixes #1949

Testing performed
Build and sanity check CFE, run all tests.

Expected behavior changes
None with default MSG implementation and current config.

System(s) tested on
Ubuntu

Additional context
Note that the "CFE_SB_IsValidMsgId" and the related constants CFE_SB_INVALID_MSG_ID and CFE_PLATFORM_SB_HIGHEST_VALID_MSGID are really pertinent message routing in SB and SBR, more so than MSG.

The general concept of what constitutes a "valid" msgid is different for MSG than SB/SBR. For MSG it is just a matter of how many bits the real header has, and how this bits are mapped between the header and CFE_SB_MsgId_t type. For example there are values which are "storable" in the CCSDS primary header, but are not valid for SB/SBR or usable with CFE.

In particular, it is possible that an application might be serving as an intermediate node in a DTN network - in which case it might need to decode and/or encode messages which are not actually routed on the CFE software bus at all. It should be SB (and SBR) domain to determine if a MsgId is route-able. MSG only should care if it is store-able in the header.

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

Updates the test case for "CFE_MSG_SetMsgId()" to write a value that
is outside the set of storable values for MsgId.  This test has a
caveat that it is somewhat implementation-dependent, but by passing
a MsgId value with all bits set, at least one of those bits is
likely not correlated with a real header bit.
@jphickey jphickey added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label Sep 14, 2021
@astrogeco astrogeco 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 Sep 15, 2021
@astrogeco
Copy link
Contributor

CCB:2021-09-15 APPROVED

@astrogeco astrogeco changed the base branch from main to integration-candidate September 16, 2021 20:44
@astrogeco astrogeco merged commit 25426ed into nasa:integration-candidate Sep 16, 2021
astrogeco added a commit to nasa/cFS that referenced this pull request Sep 21, 2021
*Combines*

nasa/cFE#1967

*Includes:*

nasa/cFE#1948, Update name of macros/functions added to "cfe_assert"

nasa/cFE#1950, Fix Mismatch between MSG API and test case in "TestMsgId" functional test

nasa/cFE#1962, Single time domain in functional time tests
astrogeco added a commit to astrogeco/cFS that referenced this pull request Sep 22, 2021
**Combines**

nasa/cFE#1967 v6.8.0-rc1+dev1024

nasa/osal#1158 v5.1.0-rc1+dev619

nasa/sch_lab#92 v2.4.0-rc1+dev53

nasa/sample_app#155 v1.2.0-rc1+dev73

nasa/to_lab#105 v2.4.0-rc1+dev58

nasa/ci_lab#93 v2.4.0-rc1+dev46

**Includes:**

*cFE*

nasa/cFE#1948, Update name of macros/functions added to "cfe_assert"

nasa/cFE#1950, Fix Mismatch between MSG API and test case in "TestMsgId" functional test

nasa/cFE#1962, Single time domain in functional time tests

nasa/cFE#1943, add missing inclusions in CFE API headers

nasa/cFE#1964, Use existing /ram for FS header test

nasa/cFE#1956, Add static local to function test so data section is nonzero

nasa/cFE#1960, Make invalid buffer length consistent in es task test

nasa/cFE#1953, Only check base filename in library info functional

nasa/cFE#1970, Confirm sb/time reset requirements in coverage test

nasa/cFE#1947, Fix broken link in App Developers Guide

nasa/cFE#1972, Fix #1971, avoid alias warning on some compilers

*osal*

nas/osal#1149, Enable symbol api test and MIR dump too large

nasa/osal#1152, Fix OSAL loader test hard fails if size exceeded

nasa/osal#1155, add bsp-specific configuration flag registry

nasa/osal#1157, Add os-specifc socket flag function

*Apps*

nasa/ci_lab#92, Apply CFE_SB_ValueToMsgId where required

nasa/sample_app#154, Apply CFE_SB_ValueToMsgId where required

nasa/sch_lab#91, Apply CFE_SB_ValueToMsgId where required

nasa/to_lab#104, Apply CFE_SB_ValueToMsgId where required

Co-authored-by: Jacob Hageman   <[email protected]>
Co-authored-by: Joseph Hickey   <[email protected]>
Co-authored-by: Adrien Chardon  <[email protected]>
astrogeco added a commit to nasa/cFS that referenced this pull request Sep 22, 2021
**Combines**

nasa/cFE#1967 v6.8.0-rc1+dev1024

nasa/osal#1158 v5.1.0-rc1+dev619

nasa/sch_lab#92 v2.4.0-rc1+dev53

nasa/sample_app#155 v1.2.0-rc1+dev73

nasa/to_lab#105 v2.4.0-rc1+dev58

nasa/ci_lab#93 v2.4.0-rc1+dev46

**Includes:**

*cFE*

nasa/cFE#1948, Update name of macros/functions added to "cfe_assert"

nasa/cFE#1950, Fix Mismatch between MSG API and test case in "TestMsgId" functional test

nasa/cFE#1962, Single time domain in functional time tests

nasa/cFE#1943, add missing inclusions in CFE API headers

nasa/cFE#1964, Use existing /ram for FS header test

nasa/cFE#1956, Add static local to function test so data section is nonzero

nasa/cFE#1960, Make invalid buffer length consistent in es task test

nasa/cFE#1953, Only check base filename in library info functional

nasa/cFE#1970, Confirm sb/time reset requirements in coverage test

nasa/cFE#1947, Fix broken link in App Developers Guide

nasa/cFE#1972, Fix #1971, avoid alias warning on some compilers

*osal*

nas/osal#1149, Enable symbol api test and MIR dump too large

nasa/osal#1152, Fix OSAL loader test hard fails if size exceeded

nasa/osal#1155, add bsp-specific configuration flag registry

nasa/osal#1157, Add os-specifc socket flag function

*Apps*

nasa/ci_lab#92, Apply CFE_SB_ValueToMsgId where required

nasa/sample_app#154, Apply CFE_SB_ValueToMsgId where required

nasa/sch_lab#91, Apply CFE_SB_ValueToMsgId where required

nasa/to_lab#104, Apply CFE_SB_ValueToMsgId where required

Co-authored-by: Jacob Hageman   <[email protected]>
Co-authored-by: Joseph Hickey   <[email protected]>
Co-authored-by: Adrien Chardon  <[email protected]>
@jphickey jphickey deleted the fix-1949-update-testcase branch September 22, 2021 20:03
@skliper skliper added this to the 7.0.0 milestone Sep 24, 2021
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.

Mismatch between MSG API and test case in "TestMsgId" functional test
3 participants