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 EDS discrepancies after #175 merge #248

Closed
skliper opened this issue Sep 30, 2019 · 6 comments
Closed

Fix EDS discrepancies after #175 merge #248

skliper opened this issue Sep 30, 2019 · 6 comments
Labels
Milestone

Comments

@skliper
Copy link
Contributor

skliper commented Sep 30, 2019

After merging #206, this changed some things in the CCSDS header and there needs to be a corresponding update to the EDS XML files and related items to ensure that the EDS actually matches the running code.

This change circles back to the EDS side and updates the XML files so they correspond to the code.

@skliper skliper added this to the 6.6.0 milestone Sep 30, 2019
@skliper skliper self-assigned this Sep 30, 2019
@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Imported from trac issue 217. Created by jphickey on 2017-10-26T19:58:26, last modified: 2019-03-05T14:58:28

@skliper skliper added the bug label Sep 30, 2019
@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by jphickey on 2017-10-26 20:33:39:

I have a few updates for consideration, split into separate commits for independent review. They are all minor and nothing actually changes any functionality except for the one bugfix.

  1. [changeset:f10a142] XML and corresponding header changes

This is the main objective. This updates the XML EDS files to once again match the implementation post #206. However, I did had to circle back to the code again and make some minor changes to the way the structs were defined in order to get things to really match. The changes are fairly trivial and low risk but we can discuss.

  1. [changeset:6e3ecbd] Move CFE_SB RoutingTblIdx type

Less critical but should be included for correctness. The "CFE_SB_MsgRouteIdx_t" is not an external type and should not be used externally. This moves it from the public header into the private header.

  1. [changeset:cc5c8fc] Fix bug route file output

In doing this changeset I noticed a minor off-by-one bug in the route output file. It should be writing a raw number into the file but it was writing the internal index. This was not quite compatible with the old behavior, so this corrects it. This is the only commit in this set that actually changes behavior at all.

  1. [changeset:c6008ca] Improve forward compatibility

Finally - If there is a plan to improve the type safety of CFE_SB_MsgId_t values going forward, then we need include these wrappers in CFE 6.6 in order to get some forward compatibility. With this, apps can be updated to use the wrapper, which in turn can make them compatible with a future release that implements strong typedef for Msg Id.

This avoids the chicken and egg problem by introducing the egg here. Nothing uses the wrapper yet. It is just there for future code to make use of.

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by cdknight on 2017-10-27 10:50:20:

recommend approve (FYI Joe ticket #245 has the "wrappers". I created it and forgot to cc you, and now can't add you to the cc)

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by glimes on 2017-10-27 12:50:23:

recommend approve

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by sstrege on 2017-10-27 12:55:47:

Recommend/approve

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by jhageman on 2019-03-05 14:58:28:

Milestone renamed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant