-
Notifications
You must be signed in to change notification settings - Fork 0
Fw 301/execute code gen on cmake (Now working) #54
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
Conversation
…/HyperloopUPV-H8/template-project into FW-301/Execute_Code_gen_on_cmake
jorgesg82
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add all generated code in a .gitignore
oganigl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For me looks good, I just looked over it.
jdmarmen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job, just one thing, as you already generate the function definitions for the callbacks in the orders I would also consider to generate the flag for each order. We always use those callbacks only to change the value of the flag, so rn the user also needs to define that flag and its also a repetitive thing, what do you think @jorgesg82 ?
Core/Inc/Code_generation/Packet_generation/Packet_generation.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Jose D. <145350079+jdmarmen@users.noreply.github.com>
jdmarmen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've seen on the template ADJ that was used to test this that you asign a socket to each packet on packets.json , that's fine as we can have different datagram sockets, but, imo that should also be done with the orders, as you can send the same tcp order to different sockets. Am i missing something??
…/HyperloopUPV-H8/template-project into FW-301/Execute_Code_gen_on_cmake
The added selection for udp sockets is because its needed to use socket->send_packet(packet), but with the use of Orders it is not needed to specify the tcp socket its coming from. Maybe im wrong but thats why its not included or specified |
oganigl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For me it's fine, check the comment
Works correctly, now when you build the proyect it will create the packets and orders from your board.
You need to change the board name on the cmake tho