Hey guys,

Could you please push a new release to foxy?

Compiler warning settings and the stricter behavior of clang/clang++ lead to the following compile error:

--- stderr: rcpputils In file included from /humble/src/ros2/rcpputils/test/dummy_shared_library/dummy_shared_library.c:15: /humble/src/ros2/rcpputils/test/dummy_shared_library/./dummy_shared_library.h:31:16: error: a function declaration without a prototype is deprecated in all versions of C [-Werror,-Wstrict-prototypes] void print_name(); ^ void /humble/src/ros2/rcpputils/test/dummy_shared_library/dummy_shared_library.c:17:16: error: a function declaration without a prototype is deprecated in all versions of C [-Werror,-Wstrict-prototypes] void print_name() ^ void 2 errors generated.

Clang with llvmproject-15.0.6 built from source.
Ubuntu 22.04
libstdc++12
Build executed as root in a docker container.

I've encountered such an error when colcon build in ros2 humble. It looks there's no operator '==' in rcpputils::fs::path.

In file included from /root/xarm/ros2_humble/src/ros/pluginlib/pluginlib/include/pluginlib/class_loader.hpp:334,
from /root/xarm/ros2_humble/src/ros/pluginlib/pluginlib/test/utest.cpp:33:
/root/xarm/ros2_humble/src/ros/pluginlib/pluginlib/include/pluginlib/./class_loader_imp.hpp: In member function ‘std::string pluginlib::ClassLoader::getPackageFromPluginXMLFilePath(const string&)’:
/root/xarm/ros2_humble/src/ros/pluginlib/pluginlib/include/pluginlib/./class_loader_imp.hpp:589:39: error: no match for ‘operator==’ (operand types are ‘rcpputils::fs::path’ and ‘rcpputils::fs::path’)
589 | if (parent.string().empty() || (p == parent)) {
| ~ ^~ ~~~~~~
| | |
| | rcpputils::fs::path
| rcpputils::fs::path
In file included from /root/xarm/ros2_humble/src/ros/pluginlib/pluginlib/include/pluginlib/class_loader.hpp:334,
from /root/xarm/ros2_humble/src/ros/pluginlib/pluginlib/test/unique_ptr_test.cpp:32:
/root/xarm/ros2_humble/src/ros/pluginlib/pluginlib/include/pluginlib/./class_loader_imp.hpp: In member function ‘std::string pluginlib::ClassLoader::getPackageFromPluginXMLFilePath(const string&)’:
/root/xarm/ros2_humble/src/ros/pluginlib/pluginlib/include/pluginlib/./class_loader_imp.hpp:589:39: error: no match for ‘operator==’ (operand types are ‘rcpputils::fs::path’ and ‘rcpputils::fs::path’)
589 | if (parent.string().empty() || (p == parent)) {
| ~ ^~ ~~~~~~
| | |
| | rcpputils::fs::path
| rcpputils::fs::path
make[2]: *** [CMakeFiles/pluginlib_utest.dir/build.make:63: CMakeFiles/pluginlib_utest.dir/test/utest.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:187: CMakeFiles/pluginlib_utest.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....
make[2]: *** [CMakeFiles/pluginlib_unique_ptr_test.dir/build.make:63: CMakeFiles/pluginlib_unique_ptr_test.dir/test/unique_ptr_test.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:243: CMakeFiles/pluginlib_unique_ptr_test.dir/all] Error 2
make: *** [Makefile:141: all] Error 2

Failed <<< pluginlib [2.24s, exited with code 2]
Aborted <<< rosidl_typesupport_fastrtps_cpp [8.29s]
Aborted <<< rcl_logging_spdlog [2.90s]
Aborted <<< rcl_yaml_param_parser [7.67s]
Aborted <<< rviz_rendering [23.2s]
Aborted <<< rosidl_typesupport_introspection_tests [18.7s]

Summary: 141 packages finished [48.2s]
1 package failed: pluginlib
5 packages aborted: rcl_logging_spdlog rcl_yaml_param_parser rosidl_typesupport_fastrtps_cpp rosidl_typesupport_introspection_tests rviz_rendering
1 package had stderr output: pluginlib
196 packages not processed

Does anyone know how to solve this?

Thank you.

I ran into this compiler error when building ROS 2 galactic on Fedora 34: https://pastebin.com/raw/3Zj1wrDY
The issue does not occur on v2.2.0 (9cad93e).

My build command: colcon build --symlink-install --packages-up-to rcpputils.
GCC version: gcc (GCC) 11.2.1 20210728 (Red Hat 11.2.1-1)
uname -r: 5.13.16-200.fc34.x86_64

Feature request

It would be nice to have a way to set environment variables. I have at least one use-case in ros2/domain_bridge#60 (comment).

Feature description

A function that allows setting a named environment variable; essentially a dual of rcpputils::get_env_var.

Implementation considerations

It could be implemented exactly how rcpputils::get_env_var is implemented, by invoking the corresponding rcutils environment helper function.

One approach:

bool set_env_var(const char * env_var, const char * env_value)
{
  if (!rcutils_set_env(env_var, env_value)) {
    std::string err = rcutils_get_error_string().str;
    // Resetting the error state since error string has been extracted
    rcutils_reset_error();
    throw std::runtime_error(err);
  }
  return true;
}

At least on windows, the remove() function fails when the to be deleted directory is non empty.
This requires sort of a recursive deletion as used within rosbag2: https://github.com/ros2/rosbag2/blob/c28c89267fa8e22e76c6540ffb700f908eca04db/rosbag2_test_common/include/rosbag2_test_common/temporary_directory_fixture.hpp#L59-L99

I'm soooo eager to remove a 100 lines from ros2_controllers, just need a release of Galactic!

Hi all!

I am new to ros2 and I came across with this guide for generating thread safety warnings with clang on the humble documentation page.
https://docs.ros.org/en/humble/The-ROS2-Project/Contributing/Quality-Guide.html

I wanted to try it myself however no matter what I have tried I was not able to generate any warnings with the documentation given example.

My project structure:

The content of the trial.cpp file:

#include "rcpputils/thread_safety_annotations.hpp"

class Foo {
public:
  void incr(int amount) {
    std::lock_guard<std::mutex> lock(mutex_);
    bar += amount;
  }

  int get() const {
    return bar;
  }

private:
  mutable std::mutex mutex_;
  int bar RCPPUTILS_TSA_GUARDED_BY(mutex_);
};

int main(int argc, char const *argv[])
{
  return 0;
}

CMakeLists.txt:

cmake_minimum_required(VERSION 3.8)
project(trial)

if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang")
  add_compile_options(-Wall -Wextra -Wpedantic)
endif()

# find dependencies
find_package(ament_cmake REQUIRED)
find_package(rclcpp REQUIRED)

# adding executables
add_executable(${PROJECT_NAME} src/trial.cpp) 
ament_target_dependencies(trial rclcpp)

if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
  add_compile_options(-Wthread-safety)   # for your whole package
endif()

#INSTALL
install(TARGETS
  trial
  DESTINATION lib/${PROJECT_NAME})

if(BUILD_TESTING)
  find_package(ament_lint_auto REQUIRED)
  # the following line skips the linter which checks for copyrights
  # comment the line when a copyright and license is added to all source files
  set(ament_cmake_copyright_FOUND TRUE)
  # the following line skips cpplint (only works in a git repo)
  # comment the line when this package is in a git repo and when
  # a copyright and license is added to all source files
  set(ament_cmake_cpplint_FOUND TRUE)
  ament_lint_auto_find_test_dependencies()
endif()

ament_package()

package.xml:

<?xml version="1.0"?>
<?xml-model href="http://download.ros.org/schema/package_format3.xsd" schematypens="http://www.w3.org/2001/XMLSchema"?>
<package format="3">
  <name>trial</name>
  <version>0.0.0</version>
  <description>TODO: Package description</description>
  <maintainer email="[email protected]">root</maintainer>
  <license>TODO: License declaration</license>

  <buildtool_depend>ament_cmake</buildtool_depend>

  <depend>rclcpp</depend>
  <!-- <depend>std_msgs</depend> -->

  <test_depend>ament_lint_auto</test_depend>
  <test_depend>ament_lint_common</test_depend>

  <export>
    <build_type>ament_cmake</build_type>
  </export>
</package>

The commands I ran for the build of the workspace (all the usual stuff and the commands referenced in the documentation)

1. source /opt/ros/humble/setup.bash
2. ros2 pkg create --build-type ament_cmake trial
3. rosdep install -i --from-path src --rosdistro foxy -y
4. CC=clang CXX=clang++ colcon build --cmake-args -DCMAKE_CXX_FLAGS='-stdlib=libc++ -D_LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS' -DFORCE_BUILD_VENDOR_PKG=ON --no-warn-unused-cli --event-handlers console_direct+ --cmake-args -DCMAKE_VERBOSE_MAKEFILE=ON

I could not find any more resources in this matter so that is why I come here. I would be glad if someone could give me an insight into the matter in any way.
Thanks in advance!

Calling rcpputils::fs::create_directories on /a/b/c/d, where d is a file and not a directory, will return true (assuming that /a/b/c/ exists) because it only checks that the path exists.

One could simply call is_directory to double check, but looking at the documentation, I don't think that's right:

This is a placeholder issue to follow through on discussion here https://discourse.ros.org/t/introduce-c-commandline-argument-parsing-library/9197 about introducing a common commandline argument parser for C++, to match argparse in Python.

https://github.com/jarro2783/cxxopts looks promising and is a popular repo.

Goal: Expose cxxopts as a vendor library, perhaps as a subpackage of this repo, and use it to simplify https://github.com/ros2/demos/blob/master/quality_of_service_demo/rclcpp/src/lifespan.cpp as the proof of usage concept

I already mentioned this in ros2/rcutils#143, but thought it would warrant its own issue, as the use case might be common.

If a binary is run with capabilities set via setcap or with the setuid bit, LD_LIBRARY_PATH is omitted during execution. Since find_library_path() relies exclusively on LD_LIBRARY_PATH, a ros2 node requiring root capabilities fails with

terminate called after throwing an instance of 'rclcpp::exceptions::RCLError'
  what():  failed to initialized rcl init options: failed to find shared library of rmw implementation. Searched rmw_fastrtps_cpp, at /tmp/binarydeb/ros-eloquent-rmw-implementation-0.8.2/src/functions.cpp:130, at /tmp/binarydeb/ros-eloquent-rcl-0.8.3/src/rcl/init_options.c:55

In our case, we need raw socket access in our node. Maybe introducing another environment variable like RMW_LIBRARY_PATH could solve this, or reading out RPATH from the binary header. Any suggestions for a workaround in the meantime would be greatly appreciated!

Inspired by Kotlin's Assert, it would be helpful to have a wrapper around assert that instead throws to provide a meaningful stack trace.

It would be nice to be able to iterate over the contents of rcpputils::fs::path (to use https://en.cppreference.com/w/cpp/filesystem/directory_iterator) - it could wrap the rcutils function https://github.com/ros2/rcutils/blob/master/include/rcutils/filesystem.h#L274

when there is only rosbag2_2021_09_27-09_11_59.db3 file and no metadata.yaml file, when I tried to play the bag file using the command "ros2 bag play rosbag2_2021_09_27-09_11_59.db3", the following error is encountered in ROS 2 Foxy terminal on windows.

[ERROR] [1652675142.815790900] [rosbag2_transport]: Failed to play: cannot get file size: The file pointer cannot be set on the specified device or file.

As per the discussion here, this is an issue in rcpputils package - ros2/rosbag2#1019.

This issue is seen happening only with larger bag files.

Expected Behavior
Bag file should be played successfully.

Actual Behavior
ros2 bag play rosbag2_2021_09_27-09_11_59.db3
ros2 bag info rosbag2_2021_09_27-09_11_59.db3
Both the above commands are leading to the below error on windows only:
[ERROR] [1652675142.815790900] [rosbag2_transport]: Failed to play: cannot get file size: The file pointer cannot be set on the specified device or file.

To Reproduce
** Steps to reproduce the behavior, e.g.

record a bag file.(around 8GB) in ROS 2 Foxy terminal.
play the bag using "ros2 bag play bagFileName". Here instead of bag folder, provide bag file name(.db3)
System (please complete the following information)
OS: Windows
ROS 2 Distro: Foxy
Version: Installed binary of latest Foxy

In order to be Quality Level 1, the Quality Declaration Rep 2004 states packages must do performance tests, unless they don't make sense. In which case, it should be documented in the QD why they aren't.

I'm creating this issue to get comments from maintainers and contributors whether it makes sense for a package like this.

While a lot of this package are just macros, type traits, and other compile-time definitions, there are some functions that may have performance implications. The files that absolutely have no worthwhile functions to test in my opinion would be (asserts.hpp, endian.hpp, pointer_traits.hpp, thread_safety_annotations.hpp and visibily_control.hpp).

It also doesn't seem worth the effort to develop performance tests for filesystem_helper.hpp since that functionality is now in the std library (++17) and it's just a matter of verifying the compilers on the supported platforms all have it before switching over to std::filesystem. I think in regards to Ubuntu, focal's build-essential should have it, MSVC 2019 supports it, and XCode 11.0 should support it. But that might require more investigation.

That leaves find_and_replace.hpp, find_library.hpp, join.hpp and split.hpp. If tests were to be written for these files, the important metrics in my mind would be the computation time of the function and allocated memory.

In my mind, there should be performance tests for the following couple of reasons, but other people might have other opinions.

• For the sake of completeness with QL 1, there should be performance tests.
• While these files might not be critical to performance and will be rarely modified, new functionality added to this package might need performance tests. Developing these tests now would provide contributors with existing infrastructure and some examples so they can add performance tests to new features

Build fails due to a missing cstdint include
https://github.com/ros2/rcpputils/blob/a77662fca77ce3f579eeecc937b0c5b9e5216b46/include/rcpputils/filesystem_helper.hpp#LL137C20-L137C28

Don't have time to make a PR right now. Currently broken on latest humble patch release.

If I follow the build-from-source instructions here:
https://index.ros.org/doc/ros2/Installation/Linux-Development-Setup/#get-ros-2-0-code

I notice that the find_library code is duplicated 3 times:

$ find src/ -name '*.h' -o -name '*.c' -o -name '*.hpp' -o -name '*.cpp' | xargs grep 'find_library'
src/ros2/rosidl_typesupport/rosidl_typesupport_cpp/src/type_support_dispatch.hpp:std::string find_library_path(const std::string & library_name);
src/ros2/rosidl_typesupport/rosidl_typesupport_cpp/src/type_support_dispatch.hpp:        std::string library_path = find_library_path(library_name);
src/ros2/rosidl_typesupport/rosidl_typesupport_cpp/src/type_support_dispatch.cpp:std::string find_library_path(const std::string & library_name)
src/ros2/rosidl_typesupport/rosidl_typesupport_c/src/type_support_dispatch.hpp:std::string find_library_path(const std::string & library_name);
src/ros2/rosidl_typesupport/rosidl_typesupport_c/src/type_support_dispatch.hpp:        std::string library_path = find_library_path(library_name);
src/ros2/rosidl_typesupport/rosidl_typesupport_c/src/type_support_dispatch.cpp:std::string find_library_path(const std::string & library_name)
src/ros2/rmw_implementation/rmw_implementation/src/functions.cpp:std::string find_library_path(const std::string & library_name)
src/ros2/rmw_implementation/rmw_implementation/src/functions.cpp:    std::string library_path = find_library_path(env_var);

It would be really nice to de-duplicate this code. If possible, could this be hoisted into this library? (I'm guessing re-written in C?)

From perusing this code, it looks there are current conditionals that depend on _WIN32 and __APPLE__, so it seems appropriate?

This effectively comes from: https://raw.githubusercontent.com/ros2/ros2/release-crystal-20190314/ros2.repos
Source: https://github.com/ros2/ros2/blob/8d34d6550a77dcfe4828466e53f0cc5401be77fe/ros2.repos

(Came about while tinkering with consuming libs from Bazel (https://index.ros.org/doc/ros2/Installation/Linux-Development-Setup/#get-ros-2-0-code).)

From discussion in #4

Hi,

Just was curious if there is a plan to sync up the 'humble' branch with changes done in the 'rolling' branch. There is a 3rd party package (ros2_controllers - ros-controls/ros2_controllers#477) that changed some of its includes to 'rccputils' from 'rcppmath' (deprecated in commit 1bd32c7), so now the main ros2_controllers branch doesn't compile in humble.

Now that it's available, there's no reason to provide a wrapper for the std functionality (IMO).

Related to #125

I'm trying to build this repository for Windows 10 UWP and am hitting the hardcoded error that unicode is not supported (from @hidmic in #35). From looking at this just for a few minutes, it seems it should be feasible to use the wide character versions of the Windows API calls and convert the data to a UTF8 encoded std::string in this case. Does this seem reasonable? Was there a reason it wasn't done this way initially?

According to the C++ standard, identifiers with double underscores are reserved for the implementation.

As such, the RCPPUTILS_THREAD_ANNOTATION_ATTRIBUTE__ macro in thread_safety_annotations.hpp is currently not compliant to the C++ standard, and should be altered. From discussion in ros-infrastructure/rosdoc2#29:

We should rename RCPPUTILS_THREAD_ANNOTATION_ATTRIBUTE__ to something like RCPPUTILS_THREAD_ANNOTATION_ATTRIBUTE_IMPL. That is standards compliant, and also fixes this particular issue. Then we can also get rid of the @cond that we are currently using.

This is doable because the macro in question is an implementation detail.

This issue tracks the progression of rcpputils to Quality Level 1 and a 1.0 version level. It follows the outline described in REP 2004.

• Version Policy
  • Follows ROS Core Quality Declaration
• Version >= 1.0.0
• Change Control Process
  • Follows ROS Core
• PRs automatically tested on Jenkins

Documentation

• Per-feature documentation.
  • Features still needing documentation on README.md or equivalent (endian.hpp, filesystem_helper.hpp, find_and_replace.hpp, find_library.hpp, join.hpp, split.hpp, pointer_traits.hpp, visibility_control.hpp)
• Per-item documentation in public API.
  • Items need doc blocks (endian.hpp, filesystem_helper.hpp, thread_safety_annotations.hpp, visibility_control.hpp)
  • Items needing doc improvements (join.hpp)
• Declared set of licenses
• Copyright statement in each source file
  • Incomplete, see #37
• Quality Declaration document (#47)

Testing:

• System tests, which cover "features"
• Tests covering public API
• Code coverage (95%)
  • 60%, see j-rivero#2 (comment)
• Performance tests
• Performance test policy: ROS Core
• Package code style: ROS Core
• Uses ament_lint_common

Dependencies:

• Runtime "ROS" dependencies are level 1
  • rcutils
• non-ROS dependencies are equivalent level 1
  • c++ standard library

Platform Support

• Supports all tier 1 platforms

Specific comments:

• filesystem_helper.hpp could attempt to include <filesystem> and use std::filesystem if possible. It's likely Foxy's supported systems already have <filesystem>
• find_and_replace.hpp
  • A regex based solution might have better performance
  • Tests could use more variation
• find_library.hpp
  • Could use more tests
• join.hpp
  • Doc block's description is a little clunky
  • delim could be an optional parameter with a null default
  • Tests could use more variation of containers and value types
• pointer_traits.hpp
  • Tests could use more variation of false cases
• split.hpp
  • is inline necessary?
• thread_safety_annotations.hpp
  • #define statements need documentation
  • Untested defines (RCPPUTILS_TSA_TRY_ACQUIRE, RCPPUTILS_TSA_TRY_ACQUIRE_SHARED, RCPPUTILS_TSA_ACQUIRED_BEFORE, RCPPUTILS_TSA_ACQUIRED_AFTER)
• visibility_control.hpp
  • Needs tests
  • Needs documentation

find_library shouldn't really return a path, but instead a loaded library.

This would permit things like ros2/rcutils#143 to not have to worry about paths, and could possibly reduce any possibility of ODR by returning currently loaded libraries rather than loading a new one.

First failure: https://github.com/ros2/rcpputils/actions/runs/145217238. Expect this test to pass. First attempt was to simply bump actions (#75), but that still failed. Given the following output this appears to be related to ament/ament_package#118

  --- output: ament_cmake_core
  [0.367s] -- Found PythonInterp: /usr/bin/python3 (found suitable version "3.6.9", minimum required is "3") 
  [0.368s] -- Using PYTHON_EXECUTABLE: /usr/bin/python3
  [0.368s] -- Override CMake install command with custom implementation using symlinks instead of copying resources
  [0.368s] -- ament_cmake_core 0.9.5
  [0.391s] Traceback (most recent call last):
  [0.391s]   File "/home/runner/work/rcpputils/rcpputils/ros_ws/build/ament_package/ament_package/templates.py", line 19, in <module>
  [0.391s]     import importlib.resources as importlib_resources
  [0.391s] ModuleNotFoundError: No module named 'importlib.resources'

Is there any reason why rcpputils::get_env_var() (#55) is provided as header-only?

I get "multiple definitions" linking errors if I #include the header twice (two different files but same executable). I hope I'm not doing anything wrong, but I don't think I am.

We're doing our first round of releases into the ROS Foxy distro and this package should also be tagged and released.

@emersonknapp Let me know if you're able to make the release, or if you'd like me to.

I am trying to create a compiled package to be used in machines without ROS2 installed. I basically have my executable, and a lib/ directory with all the necessary libraries it needs.

This solution works perfectly when adding my lib directory to LD_LIBRARY_PATH in the new machine before calling the executable, but it fails if I call it as root (Calling sudo clears LD_LIBRARY_PATH), which is needed, for example, to handle thread priorities.

Is there any reason to have LD_LIBRARY_PATH as the only place to look for libraries? Could we add more common search paths like ., ./libs or ./lib or would it be a problem for some reason that goes over my head?

The package would benefit from inclusion of the clamp and rolling mean accumulator headers found under rcppmath in the docs/FEATURES.md document. Another option could be to author a separate FEATURES document exclusive to rcppmath's scope, since these headers are currently not mentioned in the README or in the FEATURES document.

This issue comes from the an issue during ROS2 Control development: ros-controls/ros2_controllers#40

There are some fairly common math operations (in this case clamp) that are not accessible unless you use C++17 or Boost.

Do you believe these operations have a place in rcpputils? Should I make a clamp.hpp header just for it?

I am getting the following warning when compiling with Clang 13.

In file included from path/to/rcpputils/src/filesystem_helper.cpp:39:
path/to/rcpputils/include/rcpputils/filesystem_helper.hpp:94:20: warning: definition of implicit copy assignment operator for 'path' is deprecated because it has a user-declared copy constructor [-Wdeprecated-copy]
  RCPPUTILS_PUBLIC path(const path & p) = default;
                   ^
path/to/rcpputils/src/filesystem_helper.cpp:197:14: note: in implicit copy assignment operator for 'rcpputils::fs::path' first required here
      parent = *it;
             ^
1 warning generated.
---

The warning and solution seem pretty straightforward and it looks like the warning can be resolved by explicitly defining the operator in filesystem_helper.hpp as follows:

RCPPUTILS_PUBLIC path& operator=(const path & ) = default;

The Foxy branch (and consequently the includes installed in /opt/ros/foxy) seem to be missing the scope_exit header.

Is there any reason this was removed?