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

New Deployment generation feature #123

Merged
merged 21 commits into from
Apr 4, 2023
Merged

Conversation

thomas-bc
Copy link
Contributor

Originating Project/Creator
Affected Component
Affected Architectures(s) fprime-util new
Related Issue(s) nasa/fprime#1926
Has Unit Tests (y/n)
Builds Without Errors (y/n) y
Unit Tests Pass (y/n)
Documentation Included (y/n) y

Change Description

  • Adds in a new deployment cookiecutter template and the associated logic for new --deployment
  • Refactoring to enable utilities that are not tied to an build/deployment/project more easily:
    • Moved utility_entry() to cli.py because it was odd in build_helper.py
    • The logic of loading the build etc... from build_helper.py is now its own function, which utility_entry() calls only if it needs to
    • Moved all special command handler functions out of cli.py and into a new commands.py file for better readability
    • The flag controlling which new target to generate is now an argparse mutually_exclusive_group so we can defer that logic to argparse instead of handling it

Rationale

Addresses nasa/fprime#1926. Will be used for short tutorials and workshops. Has been requested for a long time. The created deployment contains the basic CNDH stack so that users can get started easily.

Request for comments

The cookiecutter stuff is in fbuild/interaction.py which is a little odd hierarchy/naming. I was thinking of putting the new_*** functions in a new module under util/ but that would make yet more refactoring. Let me know if you think that's good or if you see issues with that and I'll put it in for the future new --project feature.

Future Work

TODO: loadParameter() has been commented out in the generated configureTopology() because the function is not autocoded yet. A fix will likely be made where the function gets autocoded even if not parameters are used. Should be un-commented then.

We could add the following cookiecutter options:

  • use phases or handcoded topology?
  • useTlm channelizer vs packtizer?
  • which Ground driver? TCP, UART...

@thomas-bc thomas-bc added the enhancement New feature or request label Apr 4, 2023
@thomas-bc thomas-bc requested a review from LeStarch April 4, 2023 00:05
src/fprime/util/commands.py Fixed Show resolved Hide resolved
@LeStarch
Copy link
Collaborator

LeStarch commented Apr 4, 2023

Some minor change requests, otherwise this refactor looks really nice!

@LeStarch
Copy link
Collaborator

LeStarch commented Apr 4, 2023

Two other comments:

  1. Spelling failed, those new templates bring a lot of spelling that we fixed in F´ core
  2. Installation, make sure to run pip install without -e to make sure the templates installed correctly.

@thomas-bc
Copy link
Contributor Author

  1. spelling fixed
  2. I was able to install locally without -e with no issue

I addressed your comments, ready for re-review

@thomas-bc thomas-bc requested a review from LeStarch April 4, 2023 02:46
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.

This looks good to me. I very much relate to your last 4 commits.

@LeStarch LeStarch merged commit 67a6e1d into nasa:devel Apr 4, 2023
@thomas-bc thomas-bc deleted the feat/new-deployment branch October 11, 2023 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants