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

Add JSON dictionary generation to CMake system #2598

Merged
merged 27 commits into from
Apr 25, 2024

Conversation

thomas-bc
Copy link
Collaborator

Related Issue(s) #2591
Has Unit Tests (y/n)
Documentation Included (y/n)

Change Description

Address #2591

Opening as draft to start get some feedback to improve upon

@thomas-bc
Copy link
Collaborator Author

This branch requires these changes to FPP: jwest115/fpp#1

@thomas-bc
Copy link
Collaborator Author

Rebased to catch latest devel, including fpp v2.1.0a7 with fpp-to-dict

@thomas-bc thomas-bc marked this pull request as ready for review April 18, 2024 23:21
@thomas-bc
Copy link
Collaborator Author

@LeStarch I finally managed to fix the bug on the RPI build by explicitly calling out the global version target as a dependency to the add_custom_command() for the dictionary... I have no clue why though, especially why it would only fail on that specific deployment and not others... 🤷‍♂️

This should be ready for review

@LeStarch
Copy link
Collaborator

@thomas-bc I bet this is because RPI does not use SystemResources and as such there is no dependence on version for the deployment.....except in the install step.

Svc/SystemResources/SystemResources.cpp Dismissed Show resolved Hide resolved
Svc/SystemResources/SystemResources.cpp Dismissed Show dismissed Hide dismissed
Copy link
Collaborator

@LeStarch LeStarch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are several things of note:

  1. We need a plan for supporting sub-topologies. Not needed now, but someday.
  2. We should switch the CACHE variable used to hold the current dictionary to be a property of the module that creates it. I do not recommend doing this now, but when we tackle point 1.
  3. We need to move version.py at some point.

@LeStarch LeStarch merged commit 8e9c5ce into nasa:devel Apr 25, 2024
35 checks passed
@thomas-bc thomas-bc deleted the issue/2591-dict-autocode branch June 25, 2024 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants