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 #1538, add capability to generate multiple tables #1549

Merged

Conversation

jphickey
Copy link
Contributor

Adds a "install_custom.cmake" hook that can be put into a CPU-specific subdirectory under the "defs" directory, that can perform extra installation steps as required for the project/mission.

Tweaks the "add_cfe_tables" function such that it can also be called from the install_custom.cmake script to generate additional/alternative table binary files for that CPU.

The main update is that it uses the "APP_NAME" parameter to associate the table files with the app library, so the same set of include files can be used. This relies on the target-scope properties being used. Historically that string wasn't verified, it could have been any unique string, but now it should match the app if this is to work as expected.

Describe the contribution
Fixes #1538

Testing performed
Add multiple/alternative table definitions for sample_app in a local 2-cpu configuration. Confirm that all table files are built and installed to the staging area correctly, and that sample_app can be tweaked to load one of the alternatives, just by changing the filename it loads.

Expected behavior changes
Because the "APP_NAME" (1st parameter) of add_cfe_tables was never validated historically, it just had to be unique, this still allows any string for backward compatibility. It will generate a new "Note" message to the user if it does not match an application name, but it is still accepted.

If the multiple table feature is used, it actually needs to match the application name, or else the include paths may be incomplete.

System(s) tested on
Ubuntu

Additional context
See also nasa/sample_app#148

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

Adds a "install_custom.cmake" hook that can be put into a CPU-specific
subdirectory under the "defs" directory, that can perform extra
installation steps as required for the project/mission.

Tweaks the "add_cfe_tables" function such that it can also be called from
the install_custom.cmake script to generate additional/alternative table
binary files for that CPU.

The main update is that it uses the "APP_NAME" parameter to associate
the table files with the app library, so the same set of include files
can be used.  This relies on the target-scope properties being used.
Historically that string wasn't verified, it could have been any unique
string, but now it should match the app if this is to work as expected.
@skliper
Copy link
Contributor

skliper commented May 18, 2021

Ping @excaliburtb, does this meet your need for building multiple versions of tables?

@excaliburtb
Copy link

This looks like what I would want. Is there a way to add include paths to a specific table?

@jphickey
Copy link
Contributor Author

This looks like what I would want. Is there a way to add include paths to a specific table?

You can always call the target_compile_definitions and/or target_include_directories cmake commands to configure how these table sources get compiled. This does require you to know the actual target name, which in this case is a combination of the target name, app name, and table basename (i.e. without extension). That is, you can do:

target_include_directories(${TGT}_${APP_NAME}_${TBLWE} ${extra_includes})

@jphickey jphickey added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label May 19, 2021
@astrogeco astrogeco added build-system CCB:Approved Indicates code review and approval by community CCB labels May 19, 2021
@astrogeco
Copy link
Contributor

CCB:2021-05-19 APPROVED

@astrogeco astrogeco changed the base branch from main to integration-candidate May 24, 2021 17:32
@astrogeco astrogeco added IC:2021-05-25 and removed CCB:Ready Ready for discussion at the Configuration Control Board (CCB) labels May 24, 2021
@astrogeco astrogeco merged commit ae1dc3b into nasa:integration-candidate May 24, 2021
@jphickey jphickey deleted the fix-1538-multi-tables branch June 8, 2021 18:21
@skliper skliper added this to the 7.0.0 milestone Sep 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build-system CCB:Approved Indicates code review and approval by community CCB
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support multiple/alternative table definitions
4 participants