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 #940, Add module suffix and core name to configdata #941

Merged

Conversation

jphickey
Copy link
Contributor

@jphickey jphickey commented Oct 7, 2020

Describe the contribution
Add the system-specific module suffix (.o, .so, .obj, etc) and the default CFE core executable name to the configdata structure.

This information has several useful purposes.

Fixes #940, but related to previous issues #611 and nasa/PSP#111

Testing performed
Build and sanity check CFE on native/pc-linux, RTEMS 4.11, and VxWorks 6.9
Confirm using inspection of command line that the correct data is used for the new fields.

On linux:

-DCFE_DEFAULT_CORE_FILENAME=\"core-cpu1\" -DCFE_DEFAULT_MODULE_EXTENSION=\".so\"

On RTEMS:

-DCFE_DEFAULT_CORE_FILENAME=\"core-cpu1.exe\" -DCFE_DEFAULT_MODULE_EXTENSION=\".obj\"

On VxWorks:

-DCFE_DEFAULT_CORE_FILENAME=\"core-cpu1.exe\" -DCFE_DEFAULT_MODULE_EXTENSION=\".o\"

Nothing uses this data at runtime (yet) but I also confirmed with GDB that the "GLOBAL_CONFIGDATA" indeed has the correct info.

Expected behavior changes
None. Just making new information available.

System(s) tested on
Ubuntu 20.04 (native), RTEMS 4.11, VxWorks 6.9

Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.

Add the system-specific module suffix (.o, .so, .obj, etc) and the
default CFE core executable name to the configdata structure.

This information has several useful purposes.
@jphickey jphickey added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label Oct 7, 2020
@jphickey jphickey requested a review from skliper October 7, 2020 22:00
@jphickey
Copy link
Contributor Author

jphickey commented Oct 7, 2020

Getting this info into the basic runtime config structure is a prerequisite to the PSP change to fix nasa/PSP#111, so this should be merged first.

@skliper
Copy link
Contributor

skliper commented Oct 8, 2020

Don't you also need CMAKE_EXECUTABLE_SUFFIX and CMAKE_SHARED_LIBRARY_SUFFIX?

https://github.com/nasa/PSP/blob/b383f7c657f97c997c8129a8853a5557eddef517/cmake/Modules/Platform/RTEMS.cmake#L12-L14
https://github.com/nasa/PSP/blob/b383f7c657f97c997c8129a8853a5557eddef517/cmake/Modules/Platform/VxWorks-CFE.cmake#L12-L14

EDIT - I see now you are getting the full target name, so maybe just the library suffix? Not sure why this would ever be different... but it could be.

@jphickey
Copy link
Contributor Author

jphickey commented Oct 8, 2020

For executables, the only executable that's relevant here is cfe core itself. So I decided to just store the whole target filename, that saves the FSW from having to reassemble it, along with the assumptions thereof. Also if we ever change the filename in CMake it should still just work.

All CFE apps/libs are built as modules i.e. add_library(... MODULE ...) so this is the only extension that should appear in load commands/startup scripts. Furthermore I've never seen them be different - although CMake does differentiate between shared libs and modules, I don't know of any system where they are actually different file types.

IMO I didn't want to throw in every possible item, only the ones we actually know we care about.

Adds an #ifndef empty string for the two new fields, which
at the very least prevents IDEs (e.g. Eclipse) from redlining
the value as an undefined symbol.

(It is always passed in on command line when building via CMake)
@skliper
Copy link
Contributor

skliper commented Oct 8, 2020

@astrogeco requesting fast track into the current IC, it's needed to fix broken behavior in PSP

@jphickey
Copy link
Contributor Author

jphickey commented Oct 8, 2020

@skliper FYI see also commit 655a66c, this prevents editors (or at least my editor) from complaining about undefined symbols.

@astrogeco astrogeco changed the base branch from main to integration-candidate October 13, 2020 13:42
@astrogeco astrogeco merged commit b5ada30 into nasa:integration-candidate Oct 13, 2020
@jphickey jphickey deleted the fix-940-filename-configdata branch October 13, 2020 21:07
@astrogeco astrogeco removed the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label Oct 14, 2020
@skliper
Copy link
Contributor

skliper commented Oct 14, 2020

CCB: 2020-10-14: Approved.

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

Successfully merging this pull request may close these issues.

Add system-specific module and library suffixes to configdata struct
3 participants