-
Notifications
You must be signed in to change notification settings - Fork 1
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
Building FMU with arbitrary paths + some cleanup #14
Conversation
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.
Looks good, I think you don't need to write tests for this in python as we're just running a cmd. If it's easy to do maybe consider writing some C++ tests to check that the build is working.
Don't forget to add a small note to CHANGELOG file before merging :)
@@ -18,6 +19,8 @@ | |||
json_interface = absolute_path / "examples" / "wind_generator" / "config" / "interface.json" | |||
fmu_src_path = absolute_path / "examples" / "wind_generator" | |||
onnx_path = absolute_path / "examples" / "wind_generator" / "config" / "example.onnx" | |||
build_path = absolute_path / "build_fmu" |
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.
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.
My though is that we should probably clean up after building the fmu. By maybe deleting the generated fmu files and the build folder. Then it does not really matter where those files are placed while building. Bu t we could also use a temp folder for building.
Ah, this error is probably because you don't have conan installed? The first conan command will create the build folder if it does not exist. But since the folder isn't created by conan the later commands fails.
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 think the cleanup after building could be solved in another PR
This resolves #2
Resolves #9
The CMakeLists.txt and conan_install.bat files from previous repo were not flexiable with the different folders used during build.
This PR changes so that the commands for building is called from python where the folders for where the fmu source is contained and where the fmu is built and saved can be specified in the code.
This makes it so we could build the fmu in a temp folder and save the FMU anywhere the user wants.
In addition some TODOs are fixed in this branch that are related to incompatibilities between onnx and json files