-
Notifications
You must be signed in to change notification settings - Fork 0
Refactor #16
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
base: main
Are you sure you want to change the base?
Conversation
…d x vector chosen to be the vector 90 degrees on z vector and rotated by angle about z vector
…publisher outside the sync checks
…orks well in good ligting, but when it is covered by a shadow the edge detection does not detect shit roflcopter
for more information, see https://pre-commit.ci
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #16 +/- ##
======================================
Coverage 0.00% 0.00%
======================================
Files 2 8 +6
Lines 256 704 +448
Branches 21 62 +41
======================================
- Misses 256 704 +448
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a new testing and development node (ValvePoseNode) for estimating the 6DOF pose of valves using various sensors and techniques. The refactoring replaces the legacy ValveDetectionNode with a more modular architecture that separates concerns into dedicated detector classes and utility functions.
Key changes:
- Replaces monolithic node implementation with modular
ValveDetectorandAngleDetectorclasses - Adds support for both depth images and point clouds with optional color image input
- Implements flexible message synchronization supporting multiple input combinations
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/valve_pose_ros.cpp |
New ROS node implementation with message filtering and templated callbacks |
src/valve_detector.cpp |
Core valve detection logic including plane segmentation and pose estimation |
src/angle_detector.cpp |
Valve handle angle detection using Canny edge detection and Hough transform |
src/depth_image_processing.cpp |
Depth image to point cloud conversion utilities |
src/pointcloud_processing.cpp |
Point cloud processing for annulus extraction |
include/valve_detection_ros/valve_pose_ros.hpp |
ROS node header with sync policies and templated callback |
include/valve_detection/valve_detector.hpp |
Valve detector class with templated pose computation |
include/valve_detection/angle_detector.hpp |
Angle detector interface |
include/valve_detection/types.hpp |
Common type definitions and parameter structs |
include/valve_detection/depth_image_processing.hpp |
Depth processing function declarations |
include/valve_detection/pointcloud_processing.hpp |
Point cloud processing function declarations |
launch/valve_detection.launch.py |
Updated launch file for composable node container |
config/valve_detection_params.yaml |
Expanded parameter configuration |
README.md |
Enhanced documentation with usage and troubleshooting |
CMakeLists.txt |
Updated build configuration for new architecture |
package.xml |
Added tf2_geometry_msgs dependency |
src/valve_detection.cpp |
Removed legacy implementation |
include/valve_detection/valve_detection.hpp |
Removed legacy header |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| int temp_x = longest_line[0]; | ||
| int temp_y = longest_line[1]; | ||
| longest_line[0] = longest_line[2]; | ||
| longest_line[1] = longest_line[3]; |
Copilot
AI
Oct 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After swapping the x-coordinates, temp_x and temp_y are declared but never used to complete the swap. The y-coordinates should also be swapped: longest_line[2] = temp_x; longest_line[3] = temp_y; should be added after line 64.
| longest_line[1] = longest_line[3]; | |
| longest_line[1] = longest_line[3]; | |
| longest_line[2] = temp_x; | |
| longest_line[3] = temp_y; |
README.md
Outdated
|
|
||
| | Issue | Possible Cause | Fix | | ||
| | ---------------------------- | ---------------------------------------------------------------------------- | --- | | ||
| | No poses published | Missing plane segmentation (adjust `plane_ransac_threshold` or annulus size) | | | | |
Copilot
AI
Oct 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The table row contains extra empty cells at the end. Remove the trailing | delimiters and empty columns to maintain clean table formatting.
This introduces a new node meant for testing and development using various sensor and techniques for estimating the 6DOF of the valve. Once sensors and algorithms are decided a more production oriented node can be made