Fix #42, Use CFE_SB_CmdHdr_t for cmd sizing #43
Conversation
skliper
added
enhancement
There was a problem hiding this comment.
I'm not seeing what the actual problem was here -- the sizeof(SCH_LAB_MessageBuffer_t) union type would have been OK, correct? Is there something that works here that didn't work before? Is there some objection to having another layer of buffer type?
While the extra union might have been made unnecessary (strictly speaking) when CFE_SB_CmdHdr_t was changed to a union, I still don't see how it hurts anything to have it at this level too.
Main objection is unnecessary casting - prefer to use the union member, not cast. It's more type safe.
| @@ -128,7 +122,7 @@ void SCH_Lab_AppMain(void) | |||
| if (LocalStateEntry->Counter >= LocalStateEntry->PacketRate) | |||
| { | |||
| LocalStateEntry->Counter = 0; | |||
| CFE_SB_SendMsg(&LocalStateEntry->MsgBuf.MsgHdr); | |||
| CFE_SB_SendMsg((CFE_SB_Msg_t *)&LocalStateEntry->MsgBuf); | |||
There was a problem hiding this comment.
No need to cast here, just use &LocalStateEntry->MsgBuf.BaseMsg (or equivalent).
|
Retracting change after fixing some things related to nasa/cFE#726. |
skliper
added
invalid
Describe the contribution
Eliminates unnecessary union and uses CFE_SB_CmdHdr_t for zero parameter command sizing
Fix #42
Testing performed
Steps taken to test the contribution:
Expected behavior changes
None, but allows for updates in nasa/cFE#726
System(s) tested on
Additional context
None
Third party code
None
Contributor Info - All information REQUIRED for consideration of pull request
Jacob Hageman - NASA/GSFC