-
Notifications
You must be signed in to change notification settings - Fork 728
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
fisheye model in ros2 #547
Conversation
@SteveMacenski can you check why all checks have failed? there is actually no problem inside the logs. |
@soeroesg ignore the ros2.build.com ones, only look at the Circle CI builds. the build farm is failing because of some cv_bridge issues that are killing camera calibration for us. Run that test locally and see if it passes |
@soeroesg In the
This should avoid the error that was generated. Don't worry about the build farm failures at this time, just pay attention to the CircleCI test. Also, you can test the build in
|
I think the remaining issues are from the opencv issue. Though Circle should pass even if the PR builders fail in that case. @soeroesg have you pulled in ros2 master to get the https://github.com/ros-perception/image_pipeline/blob/ros2/tools/ros2_dependencies.repos file so we're building from source? |
hm, I only see that the test of camera_calibration fails, but where can I see the actual error message? |
https://github.com/ros-perception/image_pipeline/blob/ros2/.circleci/config.yml#L30 it should be printing test-results but we don't store artifacts. Do you not see this failure locally? Have you pulled in |
I pulled in the upstream/ros2 but I don't have access to Docker at the moment so i cannot test it locally... but I will find a way |
Try rebasing again now that #564 is in. It likely won't fix the issue but it's worth a try.
The problem is not that we don't store artifacts, it's that I'll test this change locally and see if I get the same results. |
I pulled in the latest upstream and tried the docker container. Colcon test gives a strange error (ModuleNotFoundError) which I don't know how to handle, so I I attach here the full console output, I hope it helps.
|
The error you're seeing is because |
@soeroesg We have switched this repository to use Github Actions instead of CircleCI. Would you mind rebasing your branch on the |
I rebased again on the latest ros2 and I still get an error, but a new one (see below, Enum seems to be missing). I don't know why it worked before then... The line from enum import Enum is just above the erroneous line. /usr/local/lib/python3.8/dist-packages/_pytest/assertion/rewrite.py:170: in exec_module
----------- coverage: platform linux, python 3.8.2-final-0 ----------- =========================== short test summary info ============================ |
I am just blind and I was inspecting the melodic branch :S |
Yes. I wish I could disable the check but someone with higher access than me has to disable it on the repo. |
@soeroesg This is still awaiting testing, correct? |
yes, this still needs to be tested by a volunteer, I do not have ROS2 on my computer at the moment |
Tested this on ros2 foxy, it passes all tests. Issued one warning.
|
@JWhitleyWork @pmusau17 I merged the upstream changes |
@soeroesg This needs a serious rebase but could probably still be merged (sorry for the huge delay). Please let me know if you are still able to work on it. |
This was actually rebased and merged in #677 |
implemented the fisheye mono and fisheye stereo calibration based on the code in the melodic branch.
(Please test it because I have no ROS2 to test)