Skip to content

Fix two HIGH-severity tutorial bugs (CMake example + rviz image publisher) - #34

Merged
mmmarinho merged 2 commits into
mainfrom
fix-high-severity-issues
Sep 14, 2026
Merged

mmmarinho merged 2 commits into
mainfrom
fix-high-severity-issues

Conversation

@mmmarinho

Copy link
Copy Markdown
Owner

This PR fixes the two HIGH-severity issues found during a full static + manual audit of the tutorial.

1. CMake example built a source file that does not exist

cmake_tutorial_workspace/src/cpp_cmake_example_qpoases_lib/CMakeLists.txt
declared project(test_proxsuite) and add_executable(${PROJECT_NAME} ${PROJECT_NAME}.cpp), i.e. it
tried to build test_proxsuite.cpp — which does not exist. The only C++ source present (and the
one shown to the reader in the test_qpoases.cpp tab) is src/test_qpoases.cpp.

It also had FIND_PACKAGE(Eigen3 REQUIRED) even though the example never uses Eigen.

A reader following Install a CMake package → Example: include and link the qpOASES in your project
(docs/source/cmake/cmake_packages_without_sudo.rst) would have failed at the build step.

Fix:

  • project(test_proxsuite) → project(test_qpoases)
  • source path ${PROJECT_NAME}.cpp → src/${PROJECT_NAME}.cpp
  • removed the two Eigen lines
  • in the lesson: dropped the now-inaccurate "Eigen" from the warning, corrected the
    :emphasize-lines: offset (17 → 15, since two lines were removed above it), and fixed the download
    label test_dqrobotics.cpp → test_qpoases.cpp so it matches the tab title and the actual file.

2. transformations/rviz.rst: "Publish sample images" used a non-existent executable

The tab Terminal 1: Publish sample images instructed
ros2 run rqt_image_view image_publisher. rqt_image_view is a visualiser, not a publisher —
there is no image_publisher executable in it, so the command fails. The following sentence even says
"We will use the visualiser, not the publisher", confirming a publisher was intended.

Fix: use the real ROS2 image_publisher package (ships with ros-desktop, so readers have it).
On Jazzy its executable is image_publisher_node; it takes an image path and publishes on image_raw,
so the topic is remapped to /images to match the visualiser:

ros2 run image_publisher image_publisher_node /path/to/lenna.png \
    --ros-args -r image_raw:=/images

Also corrected the second tab title from "Run the bridge" to "Run the visualiser".

3. Tracking

Added ISSUES.md at the repo root with the full audit. Both HIGH items are marked [x] fixed; the
remaining MEDIUM (dead pages, stale Humble/Foxy links) and LOW items are listed with severity,
file:line, and a suggested fix so they can be addressed in a follow-up PR.


Created by an AI agent (OpenHands) on behalf of the user.

Murilo Marinho and others added 2 commits September 14, 2026 15:20
- cmake example: rename project test_proxsuite -> test_qpoases so the
  built source matches the file shown to the reader (src/test_qpoases.cpp),
  and drop the spurious Eigen dependency. Update the lesson's warning,
  the :emphasize-lines: offset, and the test_dqrobotics.cpp download label
  to stay consistent.
- transformations/rviz.rst: 'Publish sample images' now uses the real
  ROS2 image_publisher package (Jazzy executable image_publisher_node)
  instead of the non-existent 'rqt_image_view image_publisher'. Remap
  image_raw to /images to match the visualiser, and fix the tab title.
- Add ISSUES.md tracking the full static audit (both HIGH items marked
  fixed; MEDIUM/LOW items listed for follow-up).

Co-authored-by: openhands <openhands@all-hands.dev>

@mmmarinho mmmarinho left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

👍

@mmmarinho
mmmarinho merged commit 367f817 into main Sep 14, 2026
4 checks passed
@mmmarinho
mmmarinho deleted the fix-high-severity-issues branch September 14, 2026 17:01
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.

1 participant