cFS Header Version 2 (includes CCSDS Extended header) support and major update #92
Conversation
skliper
changed the title
skliper
force-pushed
the
fix80-v2-plus-general-cleanup
branch
from
84ef00b
to
a959171
Compare
Integration Candidate: 2020-05-13
Added header guard Removed local parameter hiding global
skliper
added
the
CCB:Ready
skliper
force-pushed
the
fix80-v2-plus-general-cleanup
branch
3 times, most recently
from
4d812ea
to
c44cdfd
Compare
Major rewrite - Fix nasa#2: Added 64-bit support Added big endian payload options (use when LE selected to write BE fields) Fix nasa#3: Added checksum calculation/overide Added the rest of the override possible fields (for error checking) Fix nasa#48: Added standard type options (more clear/consistant sizing) Fix nasa#80: Added cFS Version 2 header support Added protocol options Added raw message generation support Added test script Added debug and thirtytwo build options
skliper
force-pushed
the
fix80-v2-plus-general-cleanup
branch
from
c44cdfd
to
3e62ff8
Compare
skliper
changed the title
|
@skliper Can you make the title and or description more extensive? I'm assuming Version 2 refers to CCSDS headers. |
skliper
changed the title
| #include <ws2tcpip.h> | ||
| #include <windows.h> | ||
| typedef int socklen_t; | ||
| #pragma warning(disable : 4786) |
There was a problem hiding this comment.
Has this been built on WIN32 to ensure that these pragma changes still work with the spacing changed? Sometimes these can be picky.
There was a problem hiding this comment.
Nope. I'm actually inclined to remove the special WIN32 logic. We don't test it and nothing else does special handling to support it.
There was a problem hiding this comment.
Although I am curious what works and doesn't in WSL... but that's a different issue.
astrogeco
added
CCB-20200527
and removed
CCB:Ready
| else | ||
| tempuint16 = htole16(tempuint16); | ||
|
|
||
| CopyData(cmd.Packet, &startbyte, (char *)&tempuint16, sizeof(tempuint16)); |
There was a problem hiding this comment.
Wouldn't it be easier/simpler to pass an extra parameter to CopyOut that indicated the byte order? That is, either "passthrough" (for native values), "big" to write bytes in network order, or "little" to write in inverse network order. This could be implemented in an endian-neutral code to avoid depending on these non-posix functions.
There was a problem hiding this comment.
Good suggestion! I'll write an enhancement issue.
|
Turns out the comments in cFS relating to the checksum calc (in cfe) don't match actual implementation. Added a test to actually verify the checksum, and had to update it's calculation here (it's bitwise exclusive or w/ a starting value of 0xFF). |
Describe the contribution
This is a major rewrite of the tool that resolves the issues above and:
Testing performed
See tests.sh - verified all raw outputs are as expected (and error responses)
Built from bundle with both default (64 bit) and 32 bit, confirmed same output from tests.sh from both and no warnings w/ strict flags.
Expected behavior changes
Default behavior is the same except adds checksum and doesn't actually require fields
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