search

ouster-lidar / ouster-sdk

Ouster, Inc. sample code
Other
458 stars 437 forks source link

Update curl_client.h #571

Closed mrob-lab closed 4 months ago

mrob-lab commented 9 months ago

Fixed bug with project building with catkin_make on ROS Melodic.

Related Issues & PRs

Hello! We were unable to build the project for ROS Melodic with catkin_make. We have made a suggestion for a change for the following lines of code to make catkin_make be possible to build it. Thank you!

Summary of Changes

Changed string concatenation in file curl_client.h.

Validation

Build is valid in docker nvidia/cudagl:11.4.2-runtime-ubuntu18.04 image with installed ROS Melodic and ouster packages.

twslankard commented 9 months ago

Thanks for the PR, @mrob-lab.

Just curious - what was the build error you encountered? This change looks fine to me, but it will probably fail our internal CI because of the (perhaps needlessly) strict clang-format rules we have.

Cheers, Tom

Staff Engineer Ouster Inc.

Samahu commented 9 months ago

@twslankard please refer to my comment on what the user is complaining about https://github.com/ouster-lidar/ouster-ros/pull/249#issuecomment-1799831292

Samahu commented 9 months ago

@twslankard please refer to my comment on what the user is complaining about ouster-lidar/ouster-ros#249 (comment)

The main of the issue is that we used the new formatting functionality from spdlog which the Ubuntu 18.04 does not support. This PR simply is undoing the change that was made in SDK 0.10 to this file.

twslankard commented 9 months ago

@Samahu unless I'm missing something, this change isn't removing the use of spdlog formatting. I'm seeing a removal of a string concatenation.

Samahu commented 9 months ago

@Samahu unless I'm missing something, this change isn't removing the use of spdlog formatting. I'm seeing a removal of a string concatenation.

It is probably then that the using of std::string instead of const char* (for the warn method) that is causing the build to fail on Ubuntu 18.04.