From e09ea1dfe5b3317ecf422723a6d1a8b1db368b40 Mon Sep 17 00:00:00 2001 From: Evan Flynn Date: Sat, 8 Feb 2025 07:00:30 -0800 Subject: [PATCH 1/3] Add compiler versions to CI matrix Relates to #349 Signed-off-by: Evan Flynn --- .github/workflows/build_test.yml | 34 ++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/.github/workflows/build_test.yml b/.github/workflows/build_test.yml index 77dc3a13..01830cb2 100644 --- a/.github/workflows/build_test.yml +++ b/.github/workflows/build_test.yml @@ -70,9 +70,30 @@ jobs: strategy: fail-fast: false matrix: + cxx_compiler: + - g++-12 + - g++-14 + - clang-14 + - clang-16 + - clang-18 ros_distro: ${{ fromJson(needs.get_ros_distros.outputs.series) }} include: - ${{ fromJson(needs.get_ros_distros.outputs.matrix) }} + - ${{ fromJson(needs.get_ros_distros.outputs.matrix) }} + - cxx_compiler: g++-12 + cc_compiler: gcc-12 + - cxx_compiler: g++-14 + cc_compiler: gcc-14 + - cxx_compiler: clang-14 + cc_compiler: clang-14 + - cxx_compiler: clang-16 + cc_compiler: clang-16 + - cxx_compiler: clang-18 + cc_compiler: clang-18 + exclude: + - { ros_distro: humble, cxx_compiler: g++-14 } + - { ros_distro: humble, cxx_compiler: clang-16 } + - { ros_distro: humble, cxx_compiler: clang-18 } + container: image: rostooling/setup-ros-docker:${{ matrix.docker_image }} steps: @@ -80,6 +101,7 @@ jobs: run: | sudo apt-get update sudo apt-get -y install policykit-1 libgtk2.0-common screen uml-utilities libc6-dev libicu-dev gcc python3 python3-pip + sudo apt-get -y install ${{ matrix.cc_compiler }} ${{ matrix.cxx_compiler }} mkdir renode_portable wget https://builds.renode.io/renode-latest.linux-portable.tar.gz tar xf renode-latest.linux-portable.tar.gz -C renode_portable --strip-components=1 @@ -96,15 +118,7 @@ jobs: package-name: usb_cam target-ros2-distro: ${{ matrix.ros_distro }} vcs-repo-file-url: "" - colcon-defaults: | - { - "build": { - "mixin": ["coverage-gcc"] - } - } - # If possible, pin the repository in the workflow to a specific commit to avoid - # changes in colcon-mixin-repository from breaking your tests. - colcon-mixin-repository: https://raw.githubusercontent.com/colcon/colcon-mixin-repository/1ddb69bedfd1f04c2f000e95452f7c24a4d6176b/index.yaml + extra-cmake-args: -DCMAKE_C_COMPILER=${{ matrix.cc_compiler }} -DCMAKE_CXX_COMPILER=${{ matrix.cxx_compiler }} # Compile again, this time enabling integration tests - name: Build integration tests shell: bash From c68f69ee30f325f8f5cca797dd02278f6f86a52f Mon Sep 17 00:00:00 2001 From: Evan Flynn Date: Sat, 8 Feb 2025 19:29:04 -0800 Subject: [PATCH 2/3] Fix errors found by clang Closes #349 Signed-off-by: Evan Flynn --- CMakeLists.txt | 7 +- include/usb_cam/formats/mjpeg.hpp | 2 - include/usb_cam/formats/pixel_format_base.hpp | 12 +-- include/usb_cam/usb_cam.hpp | 78 ++++++++++++------- 4 files changed, 63 insertions(+), 36 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 9088a143..1a83c1ef 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -65,6 +65,9 @@ rclcpp_components_register_node(${PROJECT_NAME}_node EXECUTABLE ${PROJECT_NAME}_node_exe ) +# To fix `DSO missing from command line` error with clang +target_link_libraries(${PROJECT_NAME}_node_exe stdc++) + if(SANITIZE) target_compile_options(${PROJECT_NAME} PUBLIC -fsanitize=address -fsanitize=leak) target_link_libraries(${PROJECT_NAME} -fsanitize=address -fsanitize=leak) @@ -81,11 +84,11 @@ if(BUILD_TESTING) ament_add_gtest(test_usb_cam_utils test/test_usb_cam_utils.cpp) target_link_libraries(test_usb_cam_utils - ${PROJECT_NAME}) + ${PROJECT_NAME} m stdc++) ament_add_gtest(test_pixel_formats test/test_pixel_formats.cpp) target_link_libraries(test_pixel_formats - ${PROJECT_NAME}) + ${PROJECT_NAME} m stdc++) if(INTEGRATION_TESTS) ament_add_gtest(test_usb_cam_lib test/test_usb_cam_lib.cpp) diff --git a/include/usb_cam/formats/mjpeg.hpp b/include/usb_cam/formats/mjpeg.hpp index b8d87f77..25f8b2f1 100644 --- a/include/usb_cam/formats/mjpeg.hpp +++ b/include/usb_cam/formats/mjpeg.hpp @@ -243,8 +243,6 @@ class MJPEG2RGB : public pixel_format_base size_t m_avframe_rgb_size; char * m_averror_str; int m_result = 0; - int m_counter = 0; - const int * m_linesize; const int m_align = 32; }; diff --git a/include/usb_cam/formats/pixel_format_base.hpp b/include/usb_cam/formats/pixel_format_base.hpp index 81eedc85..e575e9b8 100644 --- a/include/usb_cam/formats/pixel_format_base.hpp +++ b/include/usb_cam/formats/pixel_format_base.hpp @@ -50,11 +50,11 @@ namespace formats /// arguments for future pixel format(s) that are added. typedef struct { - std::string name = ""; - int width = 640; - int height = 480; - size_t pixels = 640 * 480; - std::string av_device_format_str = "AV_PIX_FMT_YUV422P"; + std::string name; + int width; + int height; + size_t pixels; + std::string av_device_format_str; } format_arguments_t; @@ -74,6 +74,8 @@ class pixel_format_base m_requires_conversion(requires_conversion) {} + virtual ~pixel_format_base() {} + /// @brief Name of pixel format. Used in the parameters file to select this format /// @return inline std::string name() {return m_name;} diff --git a/include/usb_cam/usb_cam.hpp b/include/usb_cam/usb_cam.hpp index 6138cee0..e453d5f9 100644 --- a/include/usb_cam/usb_cam.hpp +++ b/include/usb_cam/usb_cam.hpp @@ -96,38 +96,65 @@ std::vector> driver_supported_formats( return fmts; } -typedef struct +typedef struct capture_format_t { struct v4l2_fmtdesc format; struct v4l2_frmivalenum v4l2_fmt; } capture_format_t; -typedef struct +typedef struct parameters_t { - std::string camera_name = "usb_cam"; // can be anything - std::string device_name = "/dev/video0"; // usually /dev/video0 or something similiar - std::string frame_id = "camera"; - std::string io_method_name = "mmap"; - std::string camera_info_url = "package://usb_cam/config/camera_info.yaml"; - std::string pixel_format_name = "yuyv2rgb"; - std::string av_device_format = "YUV422P"; - int image_width = 600; - int image_height = 400; - int framerate = 30.0; - int brightness = -1; - int contrast = -1; - int saturation = -1; - int sharpness = -1; - int gain = -1; - int white_balance = -1; - int exposure = -1; - int focus = -1; - bool auto_white_balance = true; - bool autoexposure = true; - bool autofocus = false; + std::string camera_name; + std::string device_name; + std::string frame_id; + std::string io_method_name; + std::string camera_info_url; + std::string pixel_format_name; + std::string av_device_format; + int image_width; + int image_height; + int framerate; + int brightness; + int contrast; + int saturation; + int sharpness; + int gain; + int white_balance; + int exposure; + int focus; + bool auto_white_balance; + bool autoexposure; + bool autofocus; + + parameters_t() +// *INDENT-OFF* + : camera_name("usb_cam"), + device_name("/dev/video0"), + frame_id("camera"), + io_method_name("mmap"), + camera_info_url("package://usb_cam/config/camera_info.yaml"), + pixel_format_name("yuyv2rgb"), + av_device_format("YUV422P"), + image_width(600), + image_height(480), + framerate(30.0), + brightness(-1), + contrast(-1), + saturation(-1), + sharpness(-1), + gain(-1), + white_balance(-1), + exposure(-1), + focus(-1), + auto_white_balance(true), + autoexposure(true), + autofocus(false) + { + } +// *INDENT-ON* } parameters_t; -typedef struct +typedef struct image_t { char * data; size_t width; @@ -396,13 +423,10 @@ class UsbCam image_t m_image; AVFrame * m_avframe; - int m_avframe_size; AVCodec * m_avcodec; - AVCodecID m_codec_id; AVDictionary * m_avoptions; AVCodecContext * m_avcodec_context; - int64_t m_buffer_time_us; bool m_is_capturing; int m_framerate; const time_t m_epoch_time_shift_us; From 0ca285fc849d5b805f8bdc31b79743fc89697e14 Mon Sep 17 00:00:00 2001 From: Evan Flynn Date: Sat, 8 Feb 2025 19:54:33 -0800 Subject: [PATCH 3/3] Include compiler in CI artifact names Signed-off-by: Evan Flynn --- .github/workflows/build_test.yml | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/.github/workflows/build_test.yml b/.github/workflows/build_test.yml index 01830cb2..06c88a49 100644 --- a/.github/workflows/build_test.yml +++ b/.github/workflows/build_test.yml @@ -118,7 +118,15 @@ jobs: package-name: usb_cam target-ros2-distro: ${{ matrix.ros_distro }} vcs-repo-file-url: "" - extra-cmake-args: -DCMAKE_C_COMPILER=${{ matrix.cc_compiler }} -DCMAKE_CXX_COMPILER=${{ matrix.cxx_compiler }} + extra-cmake-args: -DCMAKE_C_COMPILER=${{ matrix.cc_compiler }} -DCMAKE_CXX_COMPILER=${{ matrix.cxx_compiler }} # -DLLVM_ENABLE_RUNTIMES=compiler-rt + # TODO(flynneva): re-enable once code coverage works with differet compilers + # colcon-defaults: | + # { + # "build": { + # "mixin": ["coverage-gcc"] + # } + # } + # colcon-mixin-repository: https://raw.githubusercontent.com/colcon/colcon-mixin-repository/b8436aa16c0bdbc01081b12caa253cbf16e0fb82/index.yaml # Compile again, this time enabling integration tests - name: Build integration tests shell: bash @@ -132,13 +140,14 @@ jobs: renode_portable/renode --disable-gui --version - uses: actions/upload-artifact@v4 with: - name: colcon-logs-${{ matrix.ros_distro }} + name: colcon-logs-${{ matrix.ros_distro }}-${{ matrix.cxx_compiler }} path: ${{ steps.build_and_test_step.outputs.ros-workspace-directory-name }}/log if: always() continue-on-error: true - - uses: actions/upload-artifact@v4 - with: - name: lcov-logs-${{ matrix.ros_distro }} - path: ${{ steps.build_and_test_step.outputs.ros-workspace-directory-name }}/lcov - if: always() - continue-on-error: true \ No newline at end of file + # TODO(flynneva): re-enable once code coverage works with differet compilers + # - uses: actions/upload-artifact@v4 + # with: + # name: lcov-logs-${{ matrix.ros_distro }}-${{ matrix.cxx_compiler }} + # path: ${{ steps.build_and_test_step.outputs.ros-workspace-directory-name }}/lcov + # if: always() + # continue-on-error: true \ No newline at end of file