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

SB message id documentation is unclear and arguably misleading #590

Closed
mprather opened this issue Apr 7, 2020 · 3 comments · Fixed by #1220 or #1243
Closed

SB message id documentation is unclear and arguably misleading #590

mprather opened this issue Apr 7, 2020 · 3 comments · Fixed by #1220 or #1243
Assignees
Labels
docs This change only affects documentation. enhancement
Milestone

Comments

@mprather
Copy link

mprather commented Apr 7, 2020

Is your feature request related to a problem? Please describe.
As a new cFS developer, I've been trying to build my first complete app utilizing the cFE Application Developer doc (and as of the last 2 days the md). Rather than copy/paste the sample_app, it has been my intent to build up my experience by starting from scratch (i.e. minimize copy/paste). Thus, when setting up message ids, I utilized the logic explained in section 6.1.1. It's fairly simple and without really knowing what CCSDS represented, I read this as follows "pick any number, let it be unique within the application, but don't set the upper 3 bits". So I did.

Then I finally got around to trying to use the ground system tool by adding my app to the tool. It was at that time, I realized that I had a problem. All my "commands" were being interpreted as "telemetry".

I figured that I might have associated the wrong info into the tool. No, everything checked out with the tool setup. Then I went through my app trying to see if maybe I was improperly initializing or associating something. I could find nothing. Then I jumped into the debugger.

I found that the first byte of my message id was being checked for 2 characteristics. My message id was failing the two checks and hence my "command" had become a "telemetry" message and could therefore never be utilized to drive command codes.

After researching CCSDS and examining the cFE logic, I was able to finally track down the exact spec that outlines what bits should be set within a message id.

The lightbulb came on .. this is why all the default apps use 0x18** or 0x08** values in their msg.h files. Unfortunately, it was not explained in the sample code or documentation.

This information is massively impactful and should not be left out of the cFE App Dev guide. The guide unfortunately makes it sounds like you can pick anything but that is far from the truth. It's much more stringent than keeping the 3 most significant bits at zero.

Describe the solution you'd like
The cFE documentation should clearly outline the following:

  1. in the default build of cFS, you are bound to CCSDS rules.
  2. explain how those rules influence the cFE api (aside from message ids, what else do devs need to know about?)
  3. and most importantly, clearly explain why/when you need to set bit fields 3 and 4 in your message id.
  4. in the sample_app, it would be nice have some contextual information related to creating proper ids (for those who might copy/paste the app).

Also, APID is mentioned in the Acronyms list but it not utilized in the dev guide document. This needs to be explained or arguably removed from the document.

Describe alternatives you've considered
None. Devs rely on the documentation to create their apps.

Additional context
I found the info that ties the cFE bit check to an actual spec at https://public.ccsds.org/Lists/CCSDS%201330P11/133x0p11.pdf (see section 4.1.2.3)

Working with dev guide updated as of commit 5602bff

Requester Info
Maurice Prather

@jphickey
Copy link
Contributor

jphickey commented Apr 8, 2020

Yes - documentation could be better.

But - in a future version of CFE, the intent is to add some additional distance between the publish/subscribe identifiers (i.e. CFE_SB_MsgId_t) and CCSDS spacepacket definitions. Note that with the "extended header" and related expansion of the address domain, different rules apply to MsgId values already.

So rather than trying to write extensive complicated documentation on all the variations, the better solution is to make it actually work more like your original interpretation of just picking a number and making it unique within the scope of the mission.

@mprather
Copy link
Author

mprather commented Apr 8, 2020

It doesn't have be be complicated. It can be terse but accurate. If the message id section simply said "for commands use 0x18** and for telemetry use 0x08** as the format for message ids" then users would have enough info to avoid costly headaches with something as simple as an int id.

@jphickey
Copy link
Contributor

jphickey commented Apr 9, 2020

Yes, that's true if using the default CCSDS encapsulation but not true if using the extended header option, which has a different mapping of bits in the MsgId.

But you are correct - in the current version, with the normal CCSDS header (non-extended), commands should be 0x18xx and telemetry should be 0x08xx.

Overall though the trend is going to be (I hope) to avoid users having to know these details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change only affects documentation. enhancement
Projects
None yet
4 participants