Feature/comm plate scan revisit v2

[piroo8]

curious draft pull request

[Copilot]

Pull request overview

This PR revisits the comm-based parking/plate-scan workflow by moving plate scanning and scan-state gating into the comm stack, adding runtime enable/disable control for heavier perception/planning nodes, and updating the tmux setup to launch the new plate reader node.

Changes:

• Add scan-state gating topics (*/enabled) to idle perception/occupancy/planner work when not needed, and introduce a scan FSM + mission event logging in comm_node.
• Add a comm plate reader node entrypoint and wire it into the runtime workflow (tmux + CommNode gating via /plate_reader/enabled and /plate_reader/ready).
• Remove the legacy parking_enforcement package (mission, costmap, stereo depth, launch files, docs) in favor of the consolidated comm approach.

Reviewed changes

Copilot reviewed 21 out of 23 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
setup.sh	Adds a new tmux pane for comm plate_reader_node and shifts bag recording to a new pane.
ros2_ws/src/perception/perception/depth_node.py	Adds an enable gate (/perception/depth_enabled) to stop stereo compute when disabled.
ros2_ws/src/comm/setup.py	Registers plate_reader_node as a new comm console script.
ros2_ws/src/comm/package.xml	Adds runtime exec dependencies for comm nodes.
ros2_ws/src/comm/comm/plate_reader_node.py	Refactors plate reader to be comm-local with new /plate_reader/ready signaling and hardcoded config.
ros2_ws/src/comm/comm/occupancy_node.py	Adds an enable gate (/occupancy_node/enabled) to stop occupancy compute when disabled.
ros2_ws/src/comm/comm/local_planner_node.py	Adds an enable gate (/local_planner/enabled) to stop planning when disabled.
ros2_ws/src/comm/comm/comm_node.py	Introduces scan FSM (transit/descend/scan/ascend), stack gating, freshness interlocks, and JSON mission logging.
ros2_ws/src/parking_enforcement_package/parking_enforcement/setup.py	Deleted (removes legacy parking_enforcement packaging).
ros2_ws/src/parking_enforcement_package/parking_enforcement/setup.cfg	Deleted (removes legacy parking_enforcement packaging config).
ros2_ws/src/parking_enforcement_package/parking_enforcement/README.md	Deleted (removes legacy parking_enforcement documentation).
ros2_ws/src/parking_enforcement_package/parking_enforcement/parking_enforcement/stereo_depth_node.py	Deleted (removes legacy stereo depth implementation).
ros2_ws/src/parking_enforcement_package/parking_enforcement/parking_enforcement/parking_revisit_node.py	Deleted (removes legacy revisit mission node).
ros2_ws/src/parking_enforcement_package/parking_enforcement/parking_enforcement/parking_mission_node.py	Deleted (removes legacy first-pass mission node).
ros2_ws/src/parking_enforcement_package/parking_enforcement/parking_enforcement/mission_utils.py	Deleted (removes legacy mission utilities).
ros2_ws/src/parking_enforcement_package/parking_enforcement/parking_enforcement/local_costmap_node.py	Deleted (removes legacy local costmap node).
ros2_ws/src/parking_enforcement_package/parking_enforcement/parking_enforcement/config.py	Deleted (removes legacy shared config).
ros2_ws/src/parking_enforcement_package/parking_enforcement/package.xml	Deleted (removes legacy package manifest).
ros2_ws/src/parking_enforcement_package/parking_enforcement/launch/parking_revisit.launch.py	Deleted (removes legacy revisit launch).
ros2_ws/src/parking_enforcement_package/parking_enforcement/launch/parking_first_pass.launch.py	Deleted (removes legacy first-pass launch).
ros2_ws/src/parking_enforcement_package/parking_enforcement/launch/parking_demo.launch.py	Deleted (removes legacy demo launch alias).

Comments suppressed due to low confidence (5)

ros2_ws/src/comm/comm/plate_reader_node.py:13

• This module imports tensorrt/pycuda unconditionally. On machines without the Jetson/TensorRT runtime (including dev/CI), the node will crash at import time. Prefer a guarded import with a clear runtime error and a clean shutdown (or a models_ready=False degraded mode) so the rest of the stack can still run.
  ros2_ws/src/comm/comm/plate_reader_node.py:133
• Engine deserialization / context creation can fail (missing file, incompatible engine, CUDA init issues), but initialization is not wrapped in try/except and will crash the node. Catch exceptions during model load, log a helpful error (including the engine path), and ensure /plate_reader/ready stays false so the mission FSM can handle the failure gracefully.
  ros2_ws/src/comm/comm/plate_reader_node.py:54
• TRTModel.__init__ assumes deserialize_cuda_engine() and create_execution_context() succeed; if either returns None, later code will fail with a less actionable exception. Add explicit checks and raise a clear error when the engine/context cannot be created.
  ros2_ws/src/comm/comm/plate_reader_node.py:192
• There is trailing whitespace on the blank line after current_plate = plate_text, which will fail ament_flake8 (W293). Remove the whitespace-only line to keep the linter passing.
  ros2_ws/src/comm/comm/plate_reader_node.py:168
• The node always makes an overlay = frame.copy() and draws rectangles/text, but the debug image publisher is removed and the overlay is never published. This adds avoidable per-frame CPU cost during scanning; either remove the overlay work or gate it behind a parameter / a debug publisher subscription check.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[piroo8]

@copilot want you to do this "comm_node.py — home_pose initialization
self.home_pose was initialized as PoseStamped(), so the is None guard in callback_launch was never true — the pose was never captured at takeoff. RTL would always target (0,0,0).

Before — guard always False, home never set

self.home_pose = PoseStamped()
...
if self.home_pose is None: # never executes
self.home_pose.pose = ...

After

self.home_pose = None
...
if self.home_pose is None:
self.home_pose = PoseStamped() # allocate first
self.home_pose.pose = self.current_pos.pose". a bug that we had even from flight test 3.

[Copilot]

Pull request overview

Copilot reviewed 21 out of 23 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (3)

ros2_ws/src/comm/comm/plate_reader_node.py:13

• plate_reader_node.py now imports TensorRT/PyCUDA at module import time with no fallback. On systems without these Jetson-specific dependencies (or in CI), this will raise ImportError before the node can even start, making the package harder to develop/test. Consider restoring a guarded import (try/except) and having the node log a clear error and stay idle (publishing ready=False) when TRT/PyCUDA aren't available.
  ros2_ws/src/comm/comm/plate_reader_node.py:54
• TRTModel.__init__ no longer checks whether deserialize_cuda_engine() or create_execution_context() returned None. If engine deserialization fails (corrupt/unsupported engine, wrong TensorRT version), the code will error later with a less actionable exception (e.g., iterating over None). Add explicit validation and raise a RuntimeError with the engine path so failures are diagnosable.
  ros2_ws/src/comm/comm/plate_reader_node.py:192
• There is trailing whitespace on the blank line after current_plate = plate_text (visible before the if len(plate_text) == REQUIRED_PLATE_LEN: block). This will typically be flagged by ament_flake8 as W293/W291 and fail the linter tests. Remove the whitespace (or the blank line).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

dist, xy_dist, dz, yaw = ... assigns xy_dist and dz but they are never used in _update_transit. With ament_flake8 enabled for this package, this is likely to raise F841 (assigned to but never used). Use _ placeholders for unused return values or use the values in logic/logging.

Suggested change

	dist, xy_dist, dz, yaw = self.update_waypoint_navigation(self.current_wp_index, TRANSIT_ALT)
	dist, _, _, yaw = self.update_waypoint_navigation(self.current_wp_index, TRANSIT_ALT)

[Copilot]

dist, xy_dist, dz, yaw = ... assigns dist, xy_dist, and yaw but only dz is used in _update_descend. This will likely trigger ament_flake8/pyflakes F841 unused-variable errors. Consider unpacking into _ for the values you don't use.

[Copilot]

dist, xy_dist, dz, yaw = ... assigns dist, xy_dist, dz, and yaw but none of them are used in _update_scan (the function only uses timing/confirmed_plate). This is likely to fail ament_flake8 with F841 unused-variable errors. Use _ placeholders or remove the unpacking if the values aren't needed.

Suggested change

	dist, xy_dist, dz, yaw = self.update_waypoint_navigation(self.scan_wp_index, SCAN_ALT, self.scan_hold_orientation)
	self.update_waypoint_navigation(self.scan_wp_index, SCAN_ALT, self.scan_hold_orientation)

[Copilot]

dist, xy_dist, dz, yaw = ... assigns dist, xy_dist, and yaw but only dz is used in _update_ascend. With ament_flake8, this commonly triggers F841 unused-variable errors. Use _ placeholders for unused values (or log them if useful).