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

Reassess process for adding/editing constants defined in config folder #1579

Closed
EbenezerA99 opened this issue Jul 21, 2022 · 2 comments · Fixed by #2496
Closed

Reassess process for adding/editing constants defined in config folder #1579

EbenezerA99 opened this issue Jul 21, 2022 · 2 comments · Fixed by #2496
Assignees
Labels
bug High Priority High Priority issue that needs to be resolved.

Comments

@EbenezerA99
Copy link
Contributor

F´ Version 3.0+
Affected Component N/A

Problem Description

For repo's using fprime as a submodule, users will need to copy the config folder into their deployment folder and point the config_directory item in the settings.ini file to the new config folder. Users should then be able to add constants to the AcConstants.ini or AcConstants.fpp file. However, this process does not work(see discussion here: #1555).

Some things that can be improved

  1. Remove the need for BOTH the AcConstants.ini and AcConstants.fpp files, as they seem redundant.
  2. Allow users to add custom constants to the AcConstants file so that they can be referenced in their custom components.
  3. Allow users to edit the values of the constants that already exist in the AcConstants file.

How to Reproduce

  1. Add fprime as a submodule to your repository
  2. Copy the config folder within the fprime repo into your deployment directory
  3. Edit the config_directory item in the settings.ini file to the path of the new config folder
  4. Add a custom constant to the AcConstants.fpp or AcConstants.ini files/or change the value of a constant in those file
  5. Generate and build deployment
  6. Check build directory and check the FppConstantsAc.hpp file, the changes made in Step 4 will not be there.

Expected Behavior

The expected behavior should be for users to add or edit constants in the AcConstants.fpp/.ini file and have those changes be applied during the build of their deployment.

An even better solution would be to avoid having users copy the fprime config folder into their deployment at all. In this case, users would still have a config folder in their deployment directory, but it would only contain any new .hpp files they need for their new components and an AcConstants.fpp file that ONLY contains new constants or existing constants with edited values. The fprime build system would then have to check both config folders and use that for the build. Any constants that are defined twice would default to the user configured value.

@EbenezerA99
Copy link
Contributor Author

Note that with fprime-tools release 3.1.1, the 'fprime-util' tool now correctly looks at the path to the config folder specified for the config_directory item. Thus, users can now add custom constants and edit existing constants via updating the AcConstants.fpp file.

As a result, the main things that need to be addressed are:

  1. Remove the need for BOTH the AcConstants.ini and AcConstants.fpp files, as they seem redundant.
  2. Assess the process for configuration files for users who use fprime as a submodule and cannot edit the submodule. Is copying and pasting the entire config folder into the user's repo really the most efficient way?

@LeStarch LeStarch added the High Priority High Priority issue that needs to be resolved. label Sep 26, 2022
@LeStarch
Copy link
Collaborator

We also need to make sure that this "config" folder is built (runs autocoders, produces ac output) otherwise certain uses in C++ files are not available.

The reason the whole folder is needed is that -I flags point to a whole folder. Thus for the configuration headers you must supply all or none to avoid collisions. We could invent a scheme to reduce this, but likely that would offset the ease of use with complicated C++ antics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug High Priority High Priority issue that needs to be resolved.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants