Clarify/clean up spacecraft/cpu names/ids

[skliper]

Is your feature request related to a problem? Please describe.
Defined in platform config:

• CFE_PLATFORM_CPU_ID (referenced by CFE_PLATFORM_TBL_VALID_PRID_1 and CFE_CPU_ID)
  • CFE_PLATFORM_TBL_VALID_PRID_1 is used in internal table logic
  • CFE_CPU_ID looks unused
• CFE_PLATFORM_CPU_NAME (referenced by CFE_CPU_NAME)
  • CFE_CPU_NAME looks unused

Defined in mission config:

• CFE_MISSION_SPACECRAFT_ID (referenced by CFE_PLATFORM_TBL_VALID_SCID_1 and CFE_SPACECRAFT_ID)
  • CFE_SPACECRAFT_ID looks unused
  • CFE_PLATFORM_TBL_VALID_SCID_1 used in internal table logic

Defined by cmake:

• CFE_CPU_ID_VALUE (from TGTID), sets .Default_CpuId in target config
• CFE_CPU_NAME_VALUE (from TGTNAME) sets .Default_CpuName in target config
• CFE_SPACECRAFT_ID_VALUE (from SPACECRAFT_ID) sets .Default_SpacecraftId in target config

Describe the solution you'd like
Remove/deprecate unused and/or clarify use/intent. Looks like spacecraft ID may actually be defined differently (42 vs 0x42).

Describe alternatives you've considered
Stay confused.

Additional context
Table use looks like it could lead to inconsistencies.

Requester Info
Jacob Hageman - NASA/GSFC, triggered by @johnphamngc comments on nasa/PSP#154

[jphickey]

The CFE_CPU_NAME, CFE_CPU_ID, and CFE_SPACECRAFT_ID should already be deprecated.

The values from CMake are the ones which actually drive the return value of CFE_PSP_GetSpacecraftId() and CFE_PSP_GetProcessorId().

The recommended practice should be for apps to call these PSP routines to get the values at runtime, not to rely on the value in the header file. Now that CMake is being used for all, the CMake definitions should be there, so we should deprecate all header file constants.

Unfortunately, there is no CFE_PSP_GetProcessorName() or equivalent. Probably need to add this (trivial, would work just like ID, except a const char *).

[skliper]

The CFE_CPU_NAME, CFE_CPU_ID, and CFE_SPACECRAFT_ID should already be deprecated.

Correct. I'm more surprised about use of the PLATFORM/MISSION values in table logic. Didn't dig into it though.

[jphickey]

Yeah, we might have a bug here in that tables will get the wrong value if they use the header file define. Probably need to remove from these headers and add as "-D" options when invoking the compiler as part of table file builds.

[jphickey]

Quick check shows these TBL Values are only used depending on CFE_PLATFORM_TBL_VALID_SCID_COUNT and CFE_PLATFORM_TBL_VALID_PRID_COUNT being greater than 0. Default config is zero. Also a case of the code getting "compiled out" when its zero, so it could be broken.

[jphickey]

Also note about table file builds that the actual value in the header (IIRC) comes from elf2cfetbl command line parameters, which comes from CMake, so its OK. I don't know of any tables that actually contain the CPU ID / Spacecraft ID in the table content.

[johnphamngc]

I've noticed it would probably be desirable to be able to override the CPUID manually, rather than having them sequentially assigned by TGTID, in scenarios where multiple integration repositories are used to build the spacecraft software and they all need to communicate over SBN(g).

[jphickey]

Ultimately the PSP determines the CPU ID at runtime. The value from CMake is just a default. For instance the pc-linux PSP allows it to be overridden on the command line via an argument. Other FSW target PSP's could do something similar.

Table builds are a different story- if you also need to put the CPU ID in a table then this would be a new feature/enhancement.

[skliper]

Ping @excaliburtb

[excaliburtb]

So, no one should not rely on the contents of cfe_platform_cfg.h directly from an app level but from an integration level, the default values compiled into the PSP should probably come from the header, not from cmake. In other words, I would want CFE_PSP_GetProcessorId() to return CFE_PLATFORM_CPU_ID by default. The advantage of the header is that it clearly defines those values and is not obfuscated by cmake variable or build scheme. If you want to provide an optional cmake variable that allows the user to override the header content, I guess you could do that.

[jphickey]

Note that there is (currently) no requirement for a unique/different cfe_platform_cfg.h file per cpu. In fact I would always encourage/recommend NOT redefining things that aren't really necessary to redefine. So whenever possible we should try to use a common/shared platform config unless there is a good reason not to.

Also note that cfe_platform_cfg.h applies to CFE core, not to the PSP. Up until recently it wasn't even possible to #include this file from PSP code (the compiler include path didn't have this file). Although a recent change did relax that by consolidating the 3 different scopes back to 2, and thereby make this inclusion possible, I still wouldn't recommend it - again because the PSP isn't recompiled for each CPU. If more than one CPU shares the same platform, the same PSP binary library is used. CPU ID is runtime data.

Conversly - If we fixed #265 to allow independently configuring the number rather than assuming consecutive numbering, wouldn't that also address this issue? In that case, the CPU numeric IDs would be specified in targets.cmake right where the name is specified, which seems quite logical to me. They just wouldn't have to match the CMake IDs anymore.

[excaliburtb]

Yes, fixing #265 would help a lot. The point is, don't decide for me what the default cpuid is with no way to set it myself.

[excaliburtb]

Okay, there's one more way to look at this that shouldn't require me waiting for #265. Right now the default CPU ID is actually built into target_config.c which is not in the PSP itself. It's in CFE. So, I need to be able to specify, either by platform_cfg.h or a cmake variable what is used to set that default. I'm assuming target_config.c is not considered a "shared" library between targets.