Skip to content

Execute code generation in cmake #48

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

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

Cantonplas
Copy link
Contributor

@Cantonplas Cantonplas commented Feb 28, 2025

When you build the project it should detect the name of the board and creates the generated code of the board. Needs further testing but should work.

Oops now i see that i merged the other pr into this one, but well this pr only changes 4 lines in generator.py and the cmakelist

g0nz4I0
g0nz4I0 previously requested changes Mar 3, 2025
Copy link
Member

@g0nz4I0 g0nz4I0 left a comment

Choose a reason for hiding this comment

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

Remove the change that modifies the STLIB apth, also maybe it would be interesting if you change the base branch, and compare it with the other branch instead of with main.

Copy link
Member

@jmaralo jmaralo left a comment

Choose a reason for hiding this comment

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

Careful! You are removing the state_machine code generation, put it back and adapt it to the new structure, it will be needed later on

jmaralo
jmaralo previously requested changes Mar 3, 2025
Copy link
Member

@jmaralo jmaralo left a comment

Choose a reason for hiding this comment

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

Forget the previous review comment, but still the code is a bit hard to read, you should split all into four, one generator for data packets, another one for orders, another one for protections and another one for the state machine, and also split the generator, descriptors and templates into four. The code as is is a bit unclear and the option to just execute one generator is desirable

@Cantonplas Cantonplas requested a review from Copilot April 2, 2025 18:58
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR integrates board-specific code generation into the build process by removing legacy auto‐generated files and introducing new generator scripts and jinja2 templates under the Code_generation directory. It updates generator entry points to automatically detect the board name and produce board-specific DataPackets, OrderPackets, Protections, and state machine code.

  • Removed legacy auto-generated files from Communications/Packets.
  • Added new code generation scripts and templates in Code_generation.
  • Updated the primary generator to accept a board name via command-line and invoke multiple generation routines.

Reviewed Changes

Copilot reviewed 17 out of 22 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
Core/Inc/state_machine.hpp Removed autogenerated state machine file.
Core/Inc/Communications/Packets/Packet_generation/* Removed legacy packet generation files.
Core/Inc/Code_generation/State_machine_generation/State_machine_generation.py Minor adjustment to imports and removal of main block.
Core/Inc/Code_generation/Packet_generation/* New jinja2-based templates and generation scripts have been added.
Core/Inc/Code_generation/Generator.py New unified generator which integrates board selection and invokes generation of packets and protections.
Files not reviewed (5)
  • .gitmodules: Language not supported
  • CMakeLists.txt: Language not supported
  • Core/Inc/Code_generation/JSON_ADE: Language not supported
  • Core/Inc/Communications/JSON_ADE: Language not supported
  • Tests/VirtualMCU: Language not supported
Comments suppressed due to low confidence (1)

Core/Inc/Code_generation/Generator.py:68

  • [nitpick] Replace use of globals() with an explicit mapping (e.g., a dictionary of board instances) for improved clarity and maintainability.
board_instance = globals()[board_input]

@Cantonplas Cantonplas requested review from jmaralo and g0nz4I0 April 2, 2025 19:04
@Cantonplas Cantonplas dismissed stale reviews from jmaralo and g0nz4I0 April 2, 2025 19:04

.

g0nz4I0
g0nz4I0 previously approved these changes Apr 6, 2025
Copy link
Member

@g0nz4I0 g0nz4I0 left a comment

Choose a reason for hiding this comment

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

As always, just merge it to main, and see if it works properly by using it, best way to find bugs 😁 .

@Cantonplas Cantonplas dismissed g0nz4I0’s stale review April 6, 2025 18:29

The merge-base changed after approval.

@Cantonplas Cantonplas requested a review from g0nz4I0 April 9, 2025 18:32
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