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.

Sign up for GitHub

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

Jump to bottom

Fix rolling builds #289

Merged
merged 4 commits into from
Feb 24, 2024
Merged

Fix rolling builds #289

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0504c3e
Replace std::bind with lambda
achim-k Feb 23, 2024
8ad8dd9
Fix deprecation error on rolling
achim-k Feb 23, 2024
a1ec511
comments
achim-k Feb 23, 2024
8f13556
lint fix
achim-k Feb 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,13 @@ elseif("$ENV{ROS_VERSION}" STREQUAL "2")
ros2_foxglove_bridge/src/parameter_interface.cpp
ros2_foxglove_bridge/src/generic_client.cpp
)

# Check if the ROS_DISTRO is greater than iron and set a compile definition if that's the case.
string(COMPARE GREATER $ENV{ROS_DISTRO} "iron" ROS_DISTRO_GT_IRON)
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure this will do what you want? > on a string comparison?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think technically yes because ROS distro names go in alphabetical order for ROS2 (but letters are re-used from ROS1) but I also think we might want something more robust than this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also not super happy with this approach but not sure what else to do. I could check if ROS_DISTRO == rolling but then we will have to change this everytime a new release is spun off. Any other ideas?

Copy link
Contributor

Choose a reason for hiding this comment

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

😭

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you use target_compile_definitions(... PUBLIC RCLCPP_VERSION_MAJOR=${rclcpp_VERSION_MAJOR})?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That seems to work, great suggestion

if(ROS_DISTRO_GT_IRON)
target_compile_definitions(foxglove_bridge_component PRIVATE ROS_DISTRO_GT_IRON)
endif()

target_include_directories(foxglove_bridge_component
PUBLIC
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/foxglove_bridge_base/include>
Expand Down
7 changes: 7 additions & 0 deletions ros2_foxglove_bridge/src/generic_client.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,10 +123,17 @@ GenericClient::GenericClient(rclcpp::node_interfaces::NodeBaseInterface* nodeBas
_typeIntrospectionHdl = (reinterpret_cast<decltype(get_ts)>(
_typeIntrospectionLib->get_symbol(typeinstrospection_symbol_name)))();

#ifdef ROS_DISTRO_GT_IRON
_requestTypeSupportHdl =
rclcpp::get_message_typesupport_handle(requestTypeName, TYPESUPPORT_LIB_NAME, *_typeSupportLib);
_responseTypeSupportHdl = rclcpp::get_message_typesupport_handle(
responseTypeName, TYPESUPPORT_LIB_NAME, *_typeSupportLib);
#else
_requestTypeSupportHdl =
rclcpp::get_typesupport_handle(requestTypeName, TYPESUPPORT_LIB_NAME, *_typeSupportLib);
_responseTypeSupportHdl =
rclcpp::get_typesupport_handle(responseTypeName, TYPESUPPORT_LIB_NAME, *_typeSupportLib);
#endif

rcl_ret_t ret = rcl_client_init(this->get_client_handle().get(), this->get_rcl_node_handle(),
_serviceTypeSupportHdl, serviceName.c_str(), &client_options);
Expand Down
4 changes: 3 additions & 1 deletion ros2_foxglove_bridge/src/ros2_foxglove_bridge.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,9 @@ void FoxgloveBridge::subscribe(foxglove::ChannelId channelId, ConnectionHandle c
try {
auto subscriber = this->create_generic_subscription(
topic, datatype, qos,
std::bind(&FoxgloveBridge::rosMessageHandler, this, channelId, clientHandle, _1),
[this, channelId, clientHandle](std::shared_ptr<rclcpp::SerializedMessage> msg) {
this->rosMessageHandler(channelId, clientHandle, msg);
},
subscriptionOptions);
subscriptionsByClient.emplace(clientHandle, std::move(subscriber));
} catch (const std::exception& ex) {
Expand Down
Loading