Skip to content

Conversation

@HunterM321
Copy link

Added specific functions for radar cropping and fixed issues odom plotting behavior.

@HunterM321 HunterM321 requested review from Copilot and desifisker July 11, 2025 17:22
@HunterM321 HunterM321 self-assigned this Jul 11, 2025
Copy link

Copilot AI left a 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 refactors odometry plotting into a reusable function, enhances the sample teach‐submaps script with save options for point clouds, and adds a helper for radar‐based point cloud filtering.

  • Extracts and centralizes 3D odometry path plotting into plot_odometry_path.
  • Introduces --save_pc/--save_dir arguments and point‐cloud saving (individual & accumulated) in plot_teach_submaps.py.
  • Adds pix4d2radar.py to filter point clouds based on a radar’s conic field of view.

Reviewed Changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/vtr_odom_extractor/relative_export_odometry_path.py Extract plot_odometry_path and replace duplicated inline odometry plotting
samples/plot_teach_submaps.py Add CLI flags for saving point clouds and implement per‐cloud and accumulated writes
helpers/pix4d2radar.py New script to load, filter, and save radar‐cropped point clouds based on angle & range
Comments suppressed due to low confidence (2)

src/vtr_odom_extractor/relative_export_odometry_path.py:12

  • [nitpick] The newly introduced 'plot_odometry_path' function lacks unit or integration tests. Consider adding tests to validate the plotted boundary calculations and ensure it handles edge cases.
def plot_odometry_path(positions):

src/vtr_odom_extractor/relative_export_odometry_path.py:20

  • The function uses 'np' without importing numpy in this module, which will cause a NameError. Please add 'import numpy as np' at the top of the file.
    max_range = np.array([positions[:,0].max()-positions[:,0].min(), 


# Accumulate all points for this major_id
if args.save_pc:
accumulated_points.append(new_points.T)
Copy link

Copilot AI Jul 11, 2025

Choose a reason for hiding this comment

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

Appending 'new_points.T' transposes each point set to shape (3, N), which will misalign when vstacking later. Use 'new_points' (shape N×3) instead of its transpose.

Suggested change
accumulated_points.append(new_points.T)
accumulated_points.append(new_points)

Copilot uses AI. Check for mistakes.
Comment on lines +3 to +6

# Input and output file paths
input_ply = "point_clouds/pin_4_crop_sub.ply"
output_ply = "point_clouds/pin_4_filtered.ply"
Copy link

Copilot AI Jul 11, 2025

Choose a reason for hiding this comment

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

[nitpick] The script uses hardcoded file paths and parameters (e.g., input_ply, output_ply, base heights). Consider adding argparse support to make these values configurable and remove magic numbers for better flexibility.

Suggested change
# Input and output file paths
input_ply = "point_clouds/pin_4_crop_sub.ply"
output_ply = "point_clouds/pin_4_filtered.ply"
import argparse
# Parse command-line arguments
parser = argparse.ArgumentParser(description="Filter point cloud based on radar parameters.")
parser.add_argument("--input_ply", type=str, default="point_clouds/pin_4_crop_sub.ply", help="Path to input PLY file.")
parser.add_argument("--output_ply", type=str, default="point_clouds/pin_4_filtered.ply", help="Path to output PLY file.")
parser.add_argument("--pix4d_pc_base_height", type=float, default=-12, help="Base height of the Pix4D point cloud.")
parser.add_argument("--radar_height", type=float, default=1.5, help="Height of the radar above the base.")
parser.add_argument("--radar_angle", type=float, default=2, help="Radar angle in degrees.")
parser.add_argument("--radar_range", type=float, default=20, help="Radar range in meters.")
args = parser.parse_args()
# Input and output file paths
input_ply = args.input_ply
output_ply = args.output_ply

Copilot uses AI. Check for mistakes.
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.

2 participants