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 #796, Remove dependency on CCSDS version define #880

Merged

Conversation

skliper
Copy link
Contributor

@skliper skliper commented Sep 10, 2020

Describe the contribution
Fix #796

  • Removes MESSAGE_FORMAT_IS_CCSDS_VER2 and all references
  • Now replaced by MISSION_MSGID_V2 and MISSION_INCLUDE_CCSDS_HEADER
    cmake variables
  • Base MIDs localized to cpu1_msgids.h and improved documentation
    indicating example nature of implementation, note issue MsgId abstraction - add API to translate between topic ID and MsgId #732
    may make this obsolete
  • Updated cfe_sb.dox for message module concept
  • MsgId base type now always uint32 (reduces logic differences)
  • Removed system log report of version used, in build and obvious
    from packet sizes
  • Cleaned extra documentation from cfe_sb_msg_id_util.c
  • Removed verification limits on CFE_PLATFORM_SB_MAX_MSG_IDS
  • Removed UT_GetActualPktLenField and UT_GetActualCmdCodeField
    that depended on the define, shouldn't directly access message
    in a unit test since it's implementation dependent
  • Default CCSDS version default now always 0 (per the standard)
    but mission configurable

Testing performed
Build unit tests, passed except for sample app. Build usersguide and confirmed no errors or warnings.

Expected behavior changes
None, just no longer requires additional configuration flag

System(s) tested on

  • Hardware: cFS Dev Server
  • OS: Ubuntu 18.04
  • Versions: bundle main + this commit

Additional context
#726

Third party code
None

Contributor Info - All information REQUIRED for consideration of pull request
Jacob Hageman - NASA/GSFC

- Removes MESSAGE_FORMAT_IS_CCSDS_VER2 and all references
- Now replaced by MISSION_MSGID_V2 and MISSION_INCLUDE_CCSDS_HEADER
  cmake variables
- Base MIDs localized to cpu1_msgids.h and improved documentation
  indicating example nature of implementation, note issue nasa#732
  may make this obsolete
- Updated cfe_sb.dox for message module concept
- MsgId base type now always uint32 (reduces logic differences)
- Removed system log report of version used, in build and obvious
  from packet sizes
- Cleaned extra documentation from cfe_sb_msg_id_util.c
- Removed verification limits on CFE_PLATFORM_SB_MAX_MSG_IDS
- Removed UT_GetActualPktLenField and UT_GetActualCmdCodeField
  that depended on the define, shouldn't directly access message
  in a unit test since it's implementation dependent
- Default CCSDS version default now always 0 (per the standard)
  but mission configurable
@skliper skliper added this to the 7.0.0 milestone Sep 10, 2020
@skliper skliper added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label Sep 10, 2020
@skliper
Copy link
Contributor Author

skliper commented Sep 11, 2020

Sample app unit test failure (nasa/sample_app#87) is fixed by nasa/sample_app#94. Should merge these together.

@skliper
Copy link
Contributor Author

skliper commented Sep 21, 2020

I see this PR was missed last week. Another example of our failing process related to reviewing pull requests, which then lead to the breakage caused by nasa/sample_app#94. Not sure why we continue to utilize a process that continues to miss review-ready PRs.

@astrogeco
Copy link
Contributor

CCB 2020-09-23 APPROVED

@yammajamma yammajamma added CCB-20200923 and removed CCB:Ready Ready for discussion at the Configuration Control Board (CCB) labels Sep 23, 2020
@yammajamma yammajamma changed the base branch from main to integration-candidate September 24, 2020 12:40
@yammajamma yammajamma merged commit e01d421 into nasa:integration-candidate Sep 24, 2020
@skliper skliper deleted the fix796-reduce-ver-depend branch February 1, 2021 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Header version/implementation selection logic update
3 participants