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

Add ROS2 build step for LibRealSense #11100

Merged
merged 3 commits into from Nov 17, 2022

Conversation

Tamir91
Copy link
Contributor

@Tamir91 Tamir91 commented Nov 14, 2022

Tracked on LRS-546

changed to build with ament_cmake
package.xml Show resolved Hide resolved
@Tamir91 Tamir91 marked this pull request as ready for review November 15, 2022 14:59
@Nir-Az
Copy link
Collaborator

Nir-Az commented Nov 16, 2022

Please (carefully) squash this 35 commits into 1,
No need to fill the commit history too much.
Always better to strive for meaningful commits history (if possible)
You can ask Remi/Ohad for help doing it with the rebase command

@Nir-Az
Copy link
Collaborator

Nir-Az commented Nov 16, 2022

What can't we see the the tests results?
image
?

EDIT: I closed and reopened the PR and now the tests run

.github/workflows/build_librealsense_ROS2_Ubuntu.yaml Outdated Show resolved Hide resolved
.github/workflows/build_librealsense_ROS2_Ubuntu.yaml Outdated Show resolved Hide resolved

# Rolling Ridley
- docker_image: ubuntu:jammy
ros_distribution: rolling
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't see rolling in the matrix (line 16), why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see that 4 jobs were tested by GHA but in the matrix, we have only 3 ros-distribution, so I see that "rolling" was tested too.

image

In addition, according to the sample no need specify "rolling" in the matrix.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think they may have missed it... try.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rolling added.
Passed GHA but, I will check if we have a difference in results/test later.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Great, now please revert the error :)

.github/workflows/build_librealsense_ROS2_Ubuntu.yaml Outdated Show resolved Hide resolved
.github/workflows/buildsCI.yaml Outdated Show resolved Hide resolved
@Nir-Az
Copy link
Collaborator

Nir-Az commented Nov 16, 2022

Do we know if the GHA test will fail if the build fails?
Please try it, you can add an error in the librealsense code that cause failure, check and undo.
I want to be sure this test will fail if the build will fail
Thanks

@Tamir91 Tamir91 marked this pull request as draft November 16, 2022 09:08
@Nir-Az Nir-Az marked this pull request as ready for review November 16, 2022 09:40
@Nir-Az Nir-Az closed this Nov 16, 2022
@Nir-Az Nir-Az reopened this Nov 16, 2022
@Nir-Az
Copy link
Collaborator

Nir-Az commented Nov 16, 2022

What can't we see the the tests results? image ?

I closed and reopen the PR and now the tests run

@Tamir91 Tamir91 force-pushed the adding_ros2_build_to_gha branch 2 times, most recently from 3cc7471 to cfc206f Compare November 16, 2022 11:23
2 errors was injected in two files
@Tamir91
Copy link
Contributor Author

Tamir91 commented Nov 17, 2022

Do we know if the GHA test will fail if the build fails? Please try it, you can add an error in the librealsense code that cause failure, check and undo. I want to be sure this test will fail if the build will fail Thanks

The syntax error was injected to rs.hpp (";" character was deleted).
The GHA test failed during realsense2 building as expected.
image

Console output:

[9:58] Yurovskiy, Tamir
/__w/librealsense/librealsense/ros_ws/src/4bezu2360jo/librealsense/include/librealsense2/rs.hpp:22:9: error: expected ‘,’ or ‘;’ before ‘rs2_log_to_console’
609     22 |         rs2_log_to_console(min_severity, &e);
610        |         ^~~~~~~~~~~~~~~~~~
611  make[2]: *** [CMakeFiles/realsense2.dir/build.make:648: CMakeFiles/realsense2.dir/src/proc/sse/sse-pointcloud.cpp.o] Error 1
612  make[2]: *** Waiting for unfinished jobs....
613  make[1]: *** [CMakeFiles/Makefile2:332: CMakeFiles/realsense2.dir/all] Error 2
614  make: *** [Makefile:130: all] Error 2
615  ---
616  Failed   <<< librealsense2 [2min 32s, exited with code 2]
build_ros2_package_ci · Tamir91/librealsense@bcde8cb
Intel® RealSense™ SDK. Contribute to Tamir91/librealsense development by creating an account on GitHub.

@Nir-Az Nir-Az merged commit 3902001 into IntelRealSense:development Nov 17, 2022
@Tamir91 Tamir91 deleted the adding_ros2_build_to_gha branch December 22, 2022 08:37
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.

None yet

2 participants