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

Avoid OSAL IDs in messages/data files #965

Closed
jphickey opened this issue Oct 27, 2020 · 4 comments · Fixed by #1017 or #1027
Closed

Avoid OSAL IDs in messages/data files #965

jphickey opened this issue Oct 27, 2020 · 4 comments · Fixed by #1017 or #1027
Labels
Milestone

Comments

@jphickey
Copy link
Contributor

Is your feature request related to a problem? Please describe.
OSAL runtime types aren't really defined as part of the external (i.e. mission-scope) interface. The osal_id_t is one such example - it is a local runtime type, not really intended to be saved in data files or command/telemetry messages.

This might seem pedantic because it is simply a uint32, but when using CFE with a command/data dictionary tool this becomes apparent that the OSAL types used in telemetry and data files aren't part of the data dictionary. (OSAL itself doesn't have a cmd/tlm interface so it naturally wouldn't provide any such entity).

Describe the solution you'd like
Use the CFE_ES_ResourceID_t instead. Call CFE_ES_ResourceID_FromOSAL() when writing and CFE_ES_ResourceID_ToOSAL() when reading, to do the type conversion. The underlying value is compatible (i.e. both 32 bit uint32, same numbers) so it should be transparent to external tools.

Describe alternatives you've considered
Have OSAL provide a stablized CMD/TLM definition of IDs? (but CMD/TLM interface is not really part of OSAL's role - it is a runtime library)

Requester Info
Joseph Hickey, Vantage Systems, Inc.

@astrogeco
Copy link
Contributor

CCB 2020-10-28

  • Consider removing OSAL IDs in dump files since they're not consistent and are runtime specific
  • @jphickey to make list of where the IDs pop up in the codebase

@jphickey
Copy link
Contributor Author

jphickey commented Oct 30, 2020

Checked the CFE code, I only see two instances of OSAL IDs in messages or data files - the ModuleId field of the CFE_ES_AppInfo struct which in turn is reported within the "One App" telemetry message as well as the "query all apps" data file:

osal_id_t ModuleId; /**< \cfetlmmnemonic \ES_MODULEID

So this currently (at least out of the box) probably has no meaning or usefulness for a ground system.

I said "currently" because although AFAIK there is no interface that allows a ground system to query OSAL resources by ID, someone could easily write an app to do so, in which case the ID becomes useful.

The other option would be to just switch it to the CFE_ES_ResourceID_t type instead, which is backward compatible.

@jphickey
Copy link
Contributor Author

Found another one, the CFE_SB_PipeD_t structure is written in the SendPipeInfo command. This one had thrown me off because it appears in the cfe_sb_priv.h file - suggesting it is an internal data structure. But it actually is written to a stats file, making it effectively also a telemetry message. This is probably a separate issue to resolve...

@jphickey
Copy link
Contributor Author

Submitted #982 about "private" SB Pipe data

jphickey added a commit to jphickey/cFE that referenced this issue Nov 16, 2020
The "osal_id_t" type isn't defined in any of the CFE message/interface
header files for use within telemetry.

This ID is an ephemeral runtime value and is not relevant to a ground
system or anything outside CFE, so it makes most sense to simply
remove this ID from the telemtry structure.  Note that all commands
are name-based, not ID-based, hence why this ID is not really useful.
jphickey added a commit to jphickey/cFE that referenced this issue Nov 17, 2020
The "osal_id_t" type isn't defined in any of the CFE message/interface
header files for use within telemetry.

This ID is an ephemeral runtime value and is not relevant to a ground
system or anything outside CFE, so it makes most sense to simply
remove this ID from the telemtry structure.  Note that all commands
are name-based, not ID-based, hence why this ID is not really useful.
@astrogeco astrogeco added this to the 7.0.0 milestone Dec 1, 2020
@astrogeco astrogeco added bug and removed enhancement labels Dec 1, 2020
jphickey added a commit that referenced this issue Dec 1, 2020
Fix #965, remove OSAL ID from App/LibInfo struct
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants