Skip to content

Feature/19 locate pipeline start#21

Open
MadsJJ wants to merge 70 commits intomainfrom
feature/19-locate-pipeline-start
Open

Feature/19 locate pipeline start#21
MadsJJ wants to merge 70 commits intomainfrom
feature/19-locate-pipeline-start

Conversation

@MadsJJ
Copy link
Copy Markdown
Member

@MadsJJ MadsJJ commented Mar 5, 2026

No description provided.

MadsJJ and others added 30 commits January 8, 2026 15:44
contains untested code for finding and visualizing pipeline start point pixel. builds
Implements pipeline start point localization in 3D using DVL altitude and flat ground plane assumption. Replaces fragile skeleton method with robust ConvexHull-based endpoint
   detection. Adds triangulation infrastructure and lens distortion handling.
Remove 100ms timestamp validation for DVL and camera_info data.
Simplify callbacks to use latest available messages.

Changes:
- Remove dvl_timestamp_ member variable (no longer storing DVL timestamp)
- Simplify dvlCallback() - just store altitude, no timestamp
- Simplify maskCallback() - remove 100ms age validation logic
- Add camera_info unsubscribe after first message (static data)

Rationale: DVL and camera_info update frequently enough that
the latest message is always sufficiently recent. Removing age
checks simplifies the code and eliminates timing-related warnings.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Remove all legacy/fallback code to simplify the codebase for package split.
User controls the full system, so single-path implementation is sufficient.

Removed:
- Skeleton methods (extractSkeleton, findSkeletonEndpoints) - DEPRECATED
- Bottom-most fallback (findStartPixelBottomMost) - legacy method
- use_skeleton_method configuration flag - now always uses ConvexHull
- Depth-based backprojection (backproject, depthCallback, depth_topic)
- depth_sub_ subscription and last_depth_ member variable

Changed:
- findPipelineEndpoints() now only uses ConvexHull method
- Fails gracefully (returns nullopt) if ConvexHull detection fails
- Updated config comments to reflect ConvexHull-only approach
- Simplified locator node initialization

Rationale: Prepare for package split into vortex_pipeline_endpoints (2D)
and vortex_pipeline_3d (3D). ConvexHull method is robust and reliable.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…itude)

Fix camera frame convention in backprojectGroundPlane() to use Y=altitude
instead of Z=-altitude for the ground plane equation.

Changes:
- Ground plane equation: Y = altitude (not Z = -altitude)
- Ray intersection: t = altitude / ray_y (not t = -altitude / ray_z)
- Add check for ray parallel to ground (ray_y ≈ 0)
- Return point: (X, altitude, Z) where Z is forward distance
- Updated comments to clarify camera frame: X=right, Y=down, Z=forward

Rationale: Standard camera frame has Y pointing downward (image rows),
not Z pointing downward. This fixes the 3D coordinates to be physically
correct and consistent with typical camera conventions.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…for hardware reasons.

The endpoints package will run on orin which already has the image data. The resulting endpoints will then be sent to the 3d node for projection and triangulation which runs on the pi.
* visualization using 2d reprojection and 3d markers.
* pitch correction for camera
- Replace pitch-based rotation with full tf2 transform
- Fix coordinate frame to use NED convention (Z-down)
- Add camera position offset to ray intersection
- Remove debug print statements
- Pass already-computed 3D points instead of recomputing
- Simplify function signature and implementation
- Add tf2_ros and tf2_geometry_msgs to pipeline_geometry target
- Update package.xml with required dependencies
- Improve reprojection visualization
- Remove unused marker publisher
- Replace PoseStamped with vortex_msgs/Landmark
- Set type to PIPELINE_START
- Remove unused nav_msgs dependency
- Use LandmarkTypeClass for type/subtype fields
- Add id field
- Update vortex-msgs to feat/landmark-msg-refactor branch
- Remove std::cout from core detector library
- Demote per-frame RCLCPP_INFO to RCLCPP_DEBUG in node
- Use --ros-args --log-level debug to enable verbose output
- modular approach where you can choose between lowest pixel or furthest points. also supports more methods for the future (no true / false stuff)
@jorgenfj
Copy link
Copy Markdown
Contributor

+1500 lines.
Rejected

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 12, 2026

Codecov Report

❌ Patch coverage is 0% with 289 lines in your changes missing coverage. Please review.
✅ Project coverage is 0.00%. Comparing base (af7c5fb) to head (25d280d).

Files with missing lines Patch % Lines
...tion_estimator/src/ros/position_estimator_node.cpp 0.00% 104 Missing ⚠️
...pipeline_image_endpoints/src/ros/detector_node.cpp 0.00% 57 Missing ⚠️
...on_estimator/src/core/backproject_ground_plane.cpp 0.00% 34 Missing ⚠️
...pipeline_image_endpoints/src/core/lowest_pixel.cpp 0.00% 22 Missing ⚠️
...tex_pipeline_image_endpoints/src/core/detector.cpp 0.00% 21 Missing ⚠️
...eline_image_endpoints/src/core/furthest_points.cpp 0.00% 20 Missing ⚠️
...ipeline_image_endpoints/src/core/preprocessing.cpp 0.00% 18 Missing ⚠️
vortex_pipeline_image_endpoints/src/core/debug.cpp 0.00% 13 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff           @@
##            main     #21    +/-   ##
======================================
  Coverage   0.00%   0.00%            
======================================
  Files          9       8     -1     
  Lines        493     289   -204     
  Branches      53      43    -10     
======================================
+ Misses       493     289   -204     
Flag Coverage Δ
unittests 0.00% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
vortex_pipeline_image_endpoints/src/core/debug.cpp 0.00% <0.00%> (ø)
...ipeline_image_endpoints/src/core/preprocessing.cpp 0.00% <0.00%> (ø)
...eline_image_endpoints/src/core/furthest_points.cpp 0.00% <0.00%> (ø)
...tex_pipeline_image_endpoints/src/core/detector.cpp 0.00% <0.00%> (ø)
...pipeline_image_endpoints/src/core/lowest_pixel.cpp 0.00% <0.00%> (ø)
...on_estimator/src/core/backproject_ground_plane.cpp 0.00% <0.00%> (ø)
...pipeline_image_endpoints/src/ros/detector_node.cpp 0.00% <0.00%> (ø)
...tion_estimator/src/ros/position_estimator_node.cpp 0.00% <0.00%> (ø)

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

MadsJJ added 6 commits March 12, 2026 16:39
  - Pass `this` to TransformListener so TF subscriptions run on the
    node's executor instead of a hidden background thread
  - Add `reference_frame` parameter to replace hardcoded "odom" in
    published landmarks and TF lookups
  - Remove unimplemented debug image topics/params from position
    estimator README
  - Fix vcstool README: vortex-utils → vortex-msgs
  - Remove duplicate gitlens entry in devcontainer.json
  - Move default argument for LinedetectorPipe::detect() from .cpp
    definition to .hpp declaration
Comment thread .github/workflows/code-coverage.yml Outdated
Comment thread .github/workflows/industrial-ci.yml Outdated
Comment thread pipeline-filtering/include/pipeline_filters/pipeline_processing.hpp Outdated
Comment thread pipeline-filtering/src/pipeline_filtering_ros.cpp Outdated
Comment thread dependencies.repos
Comment thread vortex_pipeline_image_endpoints/src/core/detector.cpp Outdated
Comment thread vortex_pipeline_image_endpoints/src/core/furthest_points.cpp Outdated
@kluge7 kluge7 mentioned this pull request Mar 12, 2026
@MadsJJ MadsJJ requested a review from kluge7 March 12, 2026 19:12
Copy link
Copy Markdown
Contributor

@jorgenfj jorgenfj left a comment

Choose a reason for hiding this comment

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

Make nodes composable nodes.
Use snake_case for function names

Comment thread vortex_pipeline_image_endpoints/src/core/preprocessing.cpp Outdated
Comment thread vortex_pipeline_position_estimator/src/ros/position_estimator_node.cpp Outdated
Comment thread vortex_pipeline_position_estimator/src/core/backproject_ground_plane.cpp Outdated
MadsJJ added 7 commits March 20, 2026 15:51
- Remove unused #include <vector> from detector.hpp
- Make morphological kernel size a ROS param (morph_kernel_size)
- Remove frame_id from CameraIntrinsics; store as camera_frame_id_ on node
- Remove ROS types from backproject_ground_plane core library;
  accept cv::Matx33d rotation and cv::Vec3d translation instead
- Extract landmark message building into buildLandmarkMsg helper
- Add comments explaining yaw orientation calculation
Copy link
Copy Markdown
Contributor

@jorgenfj jorgenfj left a comment

Choose a reason for hiding this comment

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

lgtm

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.

5 participants