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

fix: use size_t instead of uint64_t in play_options YAML converter #1575

Merged
merged 2 commits into from
Mar 2, 2024

Conversation

wep21
Copy link
Contributor

@wep21 wep21 commented Feb 28, 2024

  • avoid build error on apple silicon below.
/Users/daisuke/workspace/rolling/src/ros2/rosbag2/rosbag2_transport/src/rosbag2_transport/play_options.cpp:65:3: error: no matching function for call to 'optional_assign'
  optional_assign<uint64_t>(node, "read_ahead_queue_size", play_options.read_ahead_queue_size);
  ^~~~~~~~~~~~~~~~~~~~~~~~~
/Users/daisuke/workspace/rolling/install/rosbag2_storage/include/rosbag2_storage/rosbag2_storage/yaml.hpp:42:6: note: candidate function template not viable: no known conversion from 'size_t' (aka 'unsigned long') to 'unsigned long long &' for 3rd argument
void optional_assign(const Node & node, std::string field, T & assign_to)
     ^
1 error generated.                                        
make[2]: *** [CMakeFiles/rosbag2_transport.dir/src/rosbag2_transport/play_options.cpp.o] Error 1
make[1]: *** [CMakeFiles/rosbag2_transport.dir/all] Error 2
make: *** [all] Error 2
--- stderr: rosbag2_transport                             
/Users/daisuke/workspace/rolling/src/ros2/rosbag2/rosbag2_transport/src/rosbag2_transport/play_options.cpp:65:3: error: no matching function for call to 'optional_assign'
  optional_assign<uint64_t>(node, "read_ahead_queue_size", play_options.read_ahead_queue_size);
  ^~~~~~~~~~~~~~~~~~~~~~~~~
/Users/daisuke/workspace/rolling/install/rosbag2_storage/include/rosbag2_storage/rosbag2_storage/yaml.hpp:42:6: note: candidate function template not viable: no known conversion from 'size_t' (aka 'unsigned long') to 'unsigned long long &' for 3rd argument
void optional_assign(const Node & node, std::string field, T & assign_to)
     ^
1 error generated.
make[2]: *** [CMakeFiles/rosbag2_transport.dir/src/rosbag2_transport/play_options.cpp.o] Error 1
make[1]: *** [CMakeFiles/rosbag2_transport.dir/all] Error 2
make: *** [all] Error 2
---
Failed   <<< rosbag2_transport [4.53s, exited with code 2]

Signed-off-by: wep21 <daisuke.nishimatsu1021@gmail.com>
@wep21 wep21 requested a review from a team as a code owner February 28, 2024 14:48
@wep21 wep21 requested review from MichaelOrlov and james-rms and removed request for a team February 28, 2024 14:48
Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
Signed-off-by: Daisuke Nishimatsu <42202095+wep21@users.noreply.github.com>
@wep21 wep21 changed the title fix: use decltype to get size_t type for macos fix: use size_t instead of uint64_t for macos Feb 29, 2024
@fujitatomoya
Copy link
Contributor

@MichaelOrlov could you take a look when you have time?

Copy link
Contributor

@MichaelOrlov MichaelOrlov left a comment

Choose a reason for hiding this comment

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

LGTM.

@MichaelOrlov MichaelOrlov changed the title fix: use size_t instead of uint64_t for macos fix: use size_t instead of uint64_t in play_options YAML converter Mar 1, 2024
@MichaelOrlov
Copy link
Contributor

Gist: https://gist.githubusercontent.com/MichaelOrlov/12bc9bc69a93cd9f459263c138bfe08f/raw/edbf5838ca762d27c037228a03f45119f8d67ffd/ros2.repos
BUILD args: --packages-above-and-dependencies rosbag2_transport rosbag2_tests
TEST args: --packages-above rosbag2_transport rosbag2_tests
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/13398

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@MichaelOrlov MichaelOrlov merged commit a2283ab into ros2:rolling Mar 2, 2024
14 checks passed
@wep21 wep21 deleted the fix-macos branch March 2, 2024 07:29
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

3 participants