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 #618, stubs must not depend on real msgid implementation #674

Merged

Conversation

jphickey
Copy link
Contributor

@jphickey jphickey commented May 6, 2020

Describe the contribution
This makes the SB stubs which access message structures into actual stubs, not a replica of the normal implementation. Stubs manipulate a local (stored in the UT framework) out-of-band buffer to hold the metadata about the message.

This removes the dependency on the actual definition of MsgId used by the mission and makes them agnostic to the setting of extended headers.

This revealed a few other minor issues in test cases where they were depending on values sitting in globals (also fixed).

Fixes #618

Testing performed
Build unit tests with SIMULATION=native ENABLE_UNIT_TESTS=TRUE with and without configuration for extended headers. Confirm passing.

Expected behavior changes
Unit tests now build and run when MESSAGE_FORMAT_IS_CCSDS_VER_2 is configured.

System(s) tested on
Ubuntu 20.04 LTS 64 bit

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

The CFE_SB_GetMsgId/SetMsgId functions, among others, were implemented
as replicas of the actual FSW code.  This creates a dependency on the
actual definition of MsgId used by the mission.

This makes the stub and actual stub.  Stubs not actually
read/write to the message in any way, they just manipulate a
local (stored in the UT framework) out-of-band buffer to hold
the metadata about the message.

This revealed a few other minor issues in test cases where they
were depending on values sitting in globals (fixed).
@skliper skliper added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label May 6, 2020
@skliper
Copy link
Contributor

skliper commented May 6, 2020

CCB 20200506 - Approved

@astrogeco
Copy link
Contributor

CCB 20200506 - APPROVED

@skliper skliper added CCB-20200506 and removed CCB:Ready Ready for discussion at the Configuration Control Board (CCB) labels May 6, 2020
@skliper skliper added this to the 6.8.0 milestone May 7, 2020
@astrogeco astrogeco changed the base branch from master to integration-candidate May 8, 2020 18:27
@astrogeco astrogeco added CCB:Approved Indicates code review and approval by community CCB IC-20200429 labels May 8, 2020
@astrogeco astrogeco merged commit d2c54cc into nasa:integration-candidate May 8, 2020
@skliper
Copy link
Contributor

skliper commented May 11, 2020

@jphickey does this necessitate an update to the sample_app unit tests? Looks like SAMPLE_ReportHousekeeping test is failing.

jphickey added a commit that referenced this pull request May 11, 2020
For CFE_SB_SendMsg this was still reading the length directly
from the header.  It needs to use the metadata value instead.
@jphickey
Copy link
Contributor Author

Found the issue, see commit 6f06b16, the CFE_SB_SendMsg() stub needs to read the length from the metadata. Pushed directly to IC as hotfix.

@jphickey jphickey deleted the fix-618-stub-msgid-dependency branch May 21, 2020 14:58
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.

Unit tests fail to build when MESSAGE_FORMAT_IS_CCSDS_VER_2 is enabled
3 participants