-
Notifications
You must be signed in to change notification settings - Fork 105
Updates for building on ROS 2 Jazzy #151
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
JStech
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.
Thanks for the contribution!
| #include <rviz/display_factory.h> | ||
| #include <rviz/display_context.h> | ||
| #include <rclcpp/rclcpp.hpp> | ||
| // #include <rviz_common/display_factory.hpp> Do we need this? |
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.
| // #include <rviz_common/display_factory.hpp> Do we need this? |
|
|
||
| private: | ||
| rviz::DisplayContext* context_; | ||
| rviz_common::DisplayContext* context_; |
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.
It's not related to this PR but I don't love seeing all these raw pointers
AndyZe
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.
We should get basic CI running on this repo, so we can at least see if it builds...
|
I think you should target the |
|
I opened a pull request against your fork. If you merge that, we should be able to test if it builds. 👍 |
AndyZe
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.
Blocking til ros2 branch is targeted.
Adjust CI for jazzy
|
I just approved the merge, sorry for the delay.
…On Thu, Nov 27, 2025 at 8:55 AM AndyZe ***@***.***> wrote:
***@***.**** requested changes on this pull request.
Blocking til ros2 branch is targeted.
—
Reply to this email directly, view it on GitHub
<#151 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AZRLIVOTLWMQADCA76BVHU33637GXAVCNFSM6AAAAACGWKJSTGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTKMJVGUZDCMJZGI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
AndyZe
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.
Cool, the build succeeds. Tests don't pass yet but that's still great progress.
Builds off of the ros2 branch with changes made by #143. This version adds the necessary changes to support ROS 2 Jazzy and does not include the demos from #143.