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

Update conanfile #699

Merged
merged 6 commits into from
May 28, 2024
Merged

Update conanfile #699

merged 6 commits into from
May 28, 2024

Conversation

schaubh
Copy link
Contributor

@schaubh schaubh commented May 21, 2024

  • Review: By commit
  • Merge strategy: Merge (no squash)

Description

This original update is lifted from the LASP fork of Basilisk and was authored by @patkenneally. The branch
added release notes and documentation updates.

Here the Eigen library is updated to 3.4.0 and OpenCV to 4.5.5.

Verification

A clean build was done with a fresh .conan directory and Basilisk built without issues.
All unit tests passed.

Documentation

Added release notes and updated the camera module RST documentation to point to the OpenCV 4.5.5
documentation.

Future work

None

@schaubh schaubh added the build Build system or compilation enhancement label May 21, 2024
@schaubh schaubh requested a review from leahkiner May 21, 2024 21:39
@schaubh schaubh self-assigned this May 21, 2024
@schaubh schaubh requested a review from a team as a code owner May 21, 2024 21:39
@schaubh schaubh force-pushed the feature/conan_file_updates branch 2 times, most recently from bdd4e5e to fbcf614 Compare May 27, 2024 12:56
@schaubh schaubh requested a review from joaogvcarneiro May 27, 2024 12:59
Copy link
Contributor

@joaogvcarneiro joaogvcarneiro left a comment

Choose a reason for hiding this comment

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

The changes look good, but for some reason, when I run pytest on src , Vizard is called multiple times? I don't know what's going on here.

@schaubh
Copy link
Contributor Author

schaubh commented May 27, 2024

Thanks for the feedback @joaogvcarneiro , I'll re-run this on my side as well. If you have BSK compiled with opNav set to True, then the opNav scenarios do get called one by one. That might be what you are seeing. It launches the app about 8-10 times.

@schaubh
Copy link
Contributor Author

schaubh commented May 27, 2024

Just re-built this branch, ran all test and built documentation. Yes, if you have the opNav flag on then the visual navigation scenarios are run as well, causing Vizard to be started up and closed down for each scenario. This is expected behavior.

@joaogvcarneiro joaogvcarneiro self-requested a review May 28, 2024 01:04
schaubh added 6 commits May 27, 2024 20:28
Previously <ifstream> was being included by happenstance. Instead,
includes used in the file should all be explicit.
Checking the return code will ensure that if
the subprocess fails then the conanfile fails.
This is correct and as a nice side effect it
ensures that the github action step fails when
the build fails. Additionally, os.system is
not the recommend method to run a subprocess.
@schaubh schaubh force-pushed the feature/conan_file_updates branch from fbcf614 to f53e010 Compare May 28, 2024 02:29
@schaubh schaubh merged commit 6d4b0ae into develop May 28, 2024
2 checks passed
@schaubh schaubh deleted the feature/conan_file_updates branch May 28, 2024 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Build system or compilation enhancement
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants