Skip to content

Fix# 543, Expose CFE_SB_GetPktType#593

Merged
astrogeco merged 1 commit intonasa:integration-candidatefrom
skliper:fix543-expose-getpkttype
Apr 14, 2020
Merged

Fix# 543, Expose CFE_SB_GetPktType#593
astrogeco merged 1 commit intonasa:integration-candidatefrom
skliper:fix543-expose-getpkttype

Conversation

@skliper
Copy link
Contributor

@skliper skliper commented Apr 8, 2020

Describe the contribution
Fix #543
Note this only exposes and adds the stub. Requirement (#587)/functional test/coverage test still in work.

Testing performed
Steps taken to test the contribution:

  1. Built with SIMULATION=native and ENABLE_UNIT_TESTS=true
  2. Tests passed
  3. Ran with cFS-GroundStation and observed reported sequence counter that wasn't reporting actual sequence count
  4. Note it does pass the sequence count test, which is what SB uses GetPktType for

Expected behavior changes
None

System(s) tested on

  • Hardware: cFS Dev Server 3
  • OS: Ubuntu 18.04
  • Versions: Bundle w/ this change

Additional context
None

Third party code
None

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

@skliper skliper added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label Apr 8, 2020
@astrogeco
Copy link
Contributor

astrogeco commented Apr 8, 2020

CCB 20200408 - Approved with small changes
Change invalid to be zero and valid to be nonzeros.

jphickey
jphickey reviewed Apr 8, 2020
View reviewed changes
Copy link
Contributor

@jphickey jphickey left a comment

Choose a reason for hiding this comment

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

Minor nitpicks/comments. Generally looks good to me though.

@skliper skliper force-pushed the fix543-expose-getpkttype branch from 97518d0 to 3e0c6ad Compare April 8, 2020 17:49
@skliper
Copy link
Contributor Author

skliper commented Apr 8, 2020

Updated per requests.

Ping @tngo67

@skliper
Copy link
Contributor Author

skliper commented Apr 8, 2020

Standby... screwed up the changes

@skliper skliper force-pushed the fix543-expose-getpkttype branch from 3e0c6ad to c98c4cc Compare April 8, 2020 17:56
@skliper
Copy link
Contributor Author

skliper commented Apr 8, 2020

Now ready (last one changes didn't fit commit topic, now fixed).

@skliper skliper added this to the 6.8.0 milestone Apr 8, 2020
@skliper
Copy link
Contributor Author

skliper commented Apr 8, 2020

Nope... one more change pending...

@skliper skliper force-pushed the fix543-expose-getpkttype branch from c98c4cc to b4ef452 Compare April 8, 2020 18:00
@skliper
Copy link
Contributor Author

skliper commented Apr 8, 2020

Fixed stub leftover cast.

@skliper skliper force-pushed the fix543-expose-getpkttype branch from b4ef452 to 8d294f8 Compare April 8, 2020 18:10
@skliper
Copy link
Contributor Author

skliper commented Apr 8, 2020

Fixed doxygen warning

@astrogeco astrogeco added CCB - 20200408 and removed CCB:Ready Ready for discussion at the Configuration Control Board (CCB) labels Apr 8, 2020
@astrogeco
Copy link
Contributor

astrogeco commented Apr 8, 2020

@skliper do we need to wait for @tngo67 to review?

@tngo67
Copy link

tngo67 commented Apr 8, 2020

@skliper do we need to wait for @tngo67 to review?

I'm good with the changes.

@astrogeco astrogeco added the CCB:Approved Indicates code review and approval by community CCB label Apr 8, 2020
@jwilmot
Copy link

jwilmot commented Apr 9, 2020

Looks good, but the comment "The extended secondary header is expected in an upcoming revision of 133.0.B-1" in cfe_sb_msg_id_util should be changed to "Multi-mission Interoperable extended secondary headers should be registered in Space Assigned Numbers Authority (SANA). The process for SANA registration is documented in 133.0.B-2. Mission specific headers need not be registered"

@skliper
Copy link
Contributor Author

skliper commented Apr 9, 2020

Updated standards comment per @jwilmot request. Should be ready for IC.

jphickey
jphickey approved these changes Apr 9, 2020
View reviewed changes
Copy link
Contributor

@jphickey jphickey left a comment

Choose a reason for hiding this comment

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

Everything looks good to me now. Would only recommend squashing the fixups down for final merge.

@skliper skliper force-pushed the fix543-expose-getpkttype branch from 1927130 to 573b88e Compare April 9, 2020 13:48
@skliper
Copy link
Contributor Author

skliper commented Apr 9, 2020

Squashed

Fix nasa#543, Expose CFE_SB_GetPktType
b5e625d 
Moves prototype into cfe_sb.h, moves code from private
to the msg_id_util area.  Also added abstracted defines
for packet types.

Adds the unit test stub and updated the standards comments
to reflect the current process.
@skliper skliper force-pushed the fix543-expose-getpkttype branch from 573b88e to b5e625d Compare April 9, 2020 13:51
@skliper
Copy link
Contributor Author

skliper commented Apr 9, 2020

More descriptive commit...

@astrogeco astrogeco changed the base branch from master to integration-candidate April 14, 2020 21:15
@astrogeco astrogeco merged commit d500291 into nasa:integration-candidate Apr 14, 2020
@astrogeco astrogeco mentioned this pull request Apr 15, 2020
Merged
@skliper skliper deleted the fix543-expose-getpkttype branch February 1, 2021 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jphickey jphickey jphickey approved these changes

Assignees

No one assigned

Labels

CCB:Approved Indicates code review and approval by community CCB

Projects

None yet

Milestone

Bootes

Development

Successfully merging this pull request may close these issues.

expose CFE_SB_GetPktType (and add stub)

5 participants

@skliper @astrogeco @tngo67 @jwilmot @jphickey