Single-node dual MID360 driver: eliminate SDK race condition + logging/thread-safety fixes + unified livox CLI#3
Conversation
… from sub-pr-1 Co-authored-by: marc-hanheide <1153084+marc-hanheide@users.noreply.github.com> Agent-Logs-Url: https://github.com/jabasai/livox_ros_driver2/sessions/43d3b612-f959-4035-a1e5-f734d1e71bf7
|
@ibrahimhroob another alternative approach I prompted... |
|
@copilot please implement a separate tool to reboot the lidars: Yes, you can reboot/power-cycle a Livox Mid-360 programmatically using the SDK. The SDK provides a dedicated reboot API, though it's a soft reboot (firmware-triggered restart), not a physical power cut. API:
|
| Function | Effect |
|---|---|
LivoxLidarRequestReboot |
Soft reboot of the LiDAR firmware (equivalent to a power cycle) |
LivoxLidarRequestReset |
Reset the LiDAR (configuration/state reset, not a full reboot) |
⚠️ Note: This is a firmware-triggered soft reboot over the network — it does not cut and restore physical power. If you need a true power cycle (e.g., to recover from a hard hang), you'd need external hardware (e.g., a smart PDU or relay).
…evices Co-authored-by: marc-hanheide <1153084+marc-hanheide@users.noreply.github.com> Agent-Logs-Url: https://github.com/jabasai/livox_ros_driver2/sessions/72f95b72-8a82-4ee4-80cf-8dde957f7536
Done in e662c7b. Added Usage: # Reboot one or more specific devices
ros2 run livox_ros_driver2 livox_reboot 192.168.1.167
ros2 run livox_ros_driver2 livox_reboot 192.168.1.167 192.168.1.184
# Reset (config/state reset) all discovered devices
ros2 run livox_ros_driver2 livox_reboot --all --reset
# Explicit host NIC
ros2 run livox_ros_driver2 livox_reboot 192.168.1.167 --host-ip 192.168.1.10
|
|
This has been tested on bob24 after a firmware update and confirmed working |
marc-hanheide
left a comment
There was a problem hiding this comment.
I approve after I tested on Bob 24 with latest firmware (v13.18.0244).
Lesson learned, make sure firmware is up to date.
There was a problem hiding this comment.
Pull request overview
This PR restructures dual-MID360 operation to run both sensors inside a single livox_ros_driver2_node (single SDK instance) to eliminate cross-instance SDK discovery/registration races, and adds a set of thread-safety/logging improvements plus CLI utilities for device management.
Changes:
- Add ROS2 launch flows that dynamically generate per-IP
host_net_infoconfigs and (for dual MID360) run both lidars in one node with topic remaps. - Improve driver robustness: atomic connection state, allowlist-based packet filtering, watchdog + reconnect timers, and node-free logging macros.
- Add standalone CLI tools (
livox_scan,livox_set_ip,livox_reboot) and ROS2-only repo updates (launch dir migration, CI/devcontainer).
Reviewed changes
Copilot reviewed 34 out of 40 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
src/tools/livox_set_ip.cpp |
New CLI tool to set static IP on Livox-SDK2 devices. |
src/tools/livox_scan.cpp |
New CLI tool to discover devices and query internal configuration. |
src/tools/livox_reboot.cpp |
New CLI tool to reboot/reset devices by IP or --all. |
src/tools/debug.py |
New UDP diagnostics helper script. |
src/livox_ros_driver2.cpp |
Adds connect-time watchdog + periodic reconnect/race recovery logic; uses new logging macros. |
src/lds_lidar.cpp |
Switches to LIVOX_* logging and registers allowed lidar handles for packet filtering. |
src/lds.cpp |
Uses atomics for connect state and tracks first data receipt for reconnect logic. |
src/lddc.cpp |
Uses atomic connect_state reads; adds throttled warnings; switches to LIVOX_* logging. |
src/include/livox_log.h |
Introduces node-free logging macros for ROS1/ROS2/non-ROS builds. |
src/driver_node.h |
Adds watchdog and reconnect timer members. |
src/comm/pub_handler.h |
Adds per-handle allowlist storage/API. |
src/comm/pub_handler.cpp |
Implements allowlist filtering and first-packet/drop diagnostics; switches to LIVOX_* logging. |
src/comm/comm.h |
Makes connect_state atomic and adds data_received atomic. |
src/call_back/livox_lidar_callback.cpp |
Mitigates config handshake races; avoids sending commands to “foreign” lidars; switches to LIVOX_* logging. |
src/call_back/lidar_common_callback.cpp |
Switches callback logging to LIVOX_* macros. |
package.xml |
Adds ROS2 package manifest to the repo (previously ignored). |
launch_ROS1/rviz_mixed.launch |
Removes ROS1 launch sample (ROS2-only branch). |
launch_ROS1/rviz_MID360.launch |
Removes ROS1 launch sample (ROS2-only branch). |
launch_ROS1/rviz_HAP.launch |
Removes ROS1 launch sample (ROS2-only branch). |
launch_ROS1/msg_mixed.launch |
Removes ROS1 launch sample (ROS2-only branch). |
launch_ROS1/msg_MID360.launch |
Removes ROS1 launch sample (ROS2-only branch). |
launch_ROS1/msg_HAP.launch |
Removes ROS1 launch sample (ROS2-only branch). |
launch/rviz_mixed.py |
ROS2 Python launch for mixed sensors (rviz + driver). |
launch/rviz_MID360_launch.py |
ROS2 Python launch for MID360 (rviz + driver). |
launch/rviz_HAP_launch.py |
ROS2 Python launch for HAP (rviz + driver). |
launch/msg_MID360_launch.py |
ROS2 Python launch for MID360 (driver only). |
launch/msg_HAP_launch.py |
ROS2 Python launch for HAP (driver only). |
launch/mid360_launch.py |
New configurable single-MID360 launcher (dynamic config generation, env/arg resolution, cleanup). |
launch/dual_mid360_launch.py |
New dual-MID360 single-node launcher (dynamic combined config + topic remaps). |
config/MID360_config_184.json |
Adds static config example for lidar 192.168.1.184. |
config/MID360_config_167.json |
Adds static config example for lidar 192.168.1.167. |
README.md |
Updates docs to ROS2-only, documents new launchers and tools. |
CMakeLists.txt |
Auto-detects ROS version/distro and builds/installs new CLI tools; installs launch/. |
.gitignore |
Stops ignoring package.xml. |
.github/workflows/ros-ci.yml |
Adds ROS2 CI workflow (SDK2 install + build). |
.github/workflows/copilot-setup-steps.yml |
Adds workflow to provision a build environment for Copilot agent tasks. |
.github/copilot-instructions.md |
Adds repo-specific Copilot guidance (architecture, conventions). |
.devcontainer/docker-compose.yml |
Adds devcontainer setup with a bridged Livox-like network. |
.devcontainer/devcontainer.json |
Adds VS Code devcontainer configuration for ROS2 Humble. |
.devcontainer/Dockerfile |
Adds container image build for ROS2 Humble + Livox SDK2. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| | Lidar | Points topic | IMU topic | TF frame | | ||
| | --- | --- | --- | --- | | ||
| | Front | `/front_lidar/points` | `/front_lidar/imu` | `front_lidar_link` | | ||
| | Back | `/back_lidar/points` | `/back_lidar/imu` | `back_lidar_link` | | ||
|
|
There was a problem hiding this comment.
The README states that dual_mid360_launch publishes front/back lidars with different TF frames (front_lidar_link vs back_lidar_link), but the driver currently supports only a single frame_id parameter and dual_mid360_launch passes only front_frame_id. Unless per-lidar frame IDs are added in the driver, this documentation should be corrected to avoid misleading users.
| | Lidar | Points topic | IMU topic | TF frame | | |
| | --- | --- | --- | --- | | |
| | Front | `/front_lidar/points` | `/front_lidar/imu` | `front_lidar_link` | | |
| | Back | `/back_lidar/points` | `/back_lidar/imu` | `back_lidar_link` | | |
| | Lidar | Points topic | IMU topic | TF frame (current driver) | | |
| | --- | --- | --- | --- | | |
| | Front | `/front_lidar/points` | `/front_lidar/imu` | `front_lidar_link` | | |
| | Back | `/back_lidar/points` | `/back_lidar/imu` | `front_lidar_link` | | |
| > **Note:** The current driver exposes only a single `frame_id` parameter that is applied to all published point cloud and IMU messages. `dual_mid360_launch.py` therefore passes only `front_frame_id`, and both the front and back lidars use the same TF frame. Per-lidar TF frames (e.g. `front_lidar_link` vs `back_lidar_link`) are **not yet supported** by the driver. |
There was a problem hiding this comment.
@copilot ensure the lidars use the correct different frame IDs
There was a problem hiding this comment.
Fixed in f3b3a24. Lddc now carries a lidar_frame_ids_ map (handle → frame_id) populated from the new lidar_frame_ids ROS parameter ("ip1:frame1,ip2:frame2"). Every publish function — PointCloud2, CustomMsg, PCL, and IMU — calls GetFrameId(index) which looks up the per-lidar override and falls back to the global frame_id. The dual_mid360_launch.py sets lidar_frame_ids: "<front_ip>:front_lidar_link,<back_ip>:back_lidar_link" so each lidar publishes with its own TF frame. This also fixes the pre-existing bug where IMU messages had frame_id hardcoded to "livox_frame" regardless of the driver parameter.
| static DeviceEntry* FindDeviceByHandle(uint32_t handle) { | ||
| std::lock_guard<std::mutex> lock(g_scan.mtx); | ||
| for (auto& d : g_scan.devices) { | ||
| if (d.handle == handle) return &d; | ||
| } | ||
| return nullptr; | ||
| } |
There was a problem hiding this comment.
FindDeviceByHandle() returns a raw pointer to an element inside g_scan.devices (std::vector) after releasing g_scan.mtx. Since OnLidarInfoChange() can push_back concurrently, the vector can reallocate and invalidate the pointer, leading to use-after-free in the async query callbacks. Use a stable container (e.g., std::map/unordered_map keyed by handle, or std::deque), or keep the mutex held while updating the entry (return an index/handle and re-look-up under lock).
| parameters=[ | ||
| {'xfer_format': int(LaunchConfiguration('xfer_format').perform(context))}, | ||
| {'multi_topic': int(LaunchConfiguration('multi_topic').perform(context))}, | ||
| {'data_src': int(LaunchConfiguration('data_src').perform(context))}, | ||
| {'publish_freq': float(LaunchConfiguration('publish_freq').perform(context))}, | ||
| {'output_data_type': int(LaunchConfiguration('output_type').perform(context))}, | ||
| {'frame_id': front_frame_id}, | ||
| {'user_config_path': config_path}, | ||
| {'cmdline_input_bd_code': ''}, | ||
| {'connect_timeout_s': float(LaunchConfiguration('connect_timeout_s').perform(context))}, | ||
| ], |
There was a problem hiding this comment.
dual_mid360_launch.py accepts back_frame_id and prints it, but the launched node parameters only set a single 'frame_id' (front_frame_id). The back lidar will therefore publish with the front frame_id, which contradicts the launch args/README text. Either remove back_frame_id (and document that a single frame_id applies), or extend the driver to support per-lidar frame IDs and pass them through.
There was a problem hiding this comment.
Fixed in f3b3a24. The back_frame_id arg is now wired through via the lidar_frame_ids parameter: lidar_frame_ids: "<front_ip>:front_lidar_link,<back_ip>:back_lidar_link". The driver parses this map and each lidar uses its own TF frame when publishing.
| 'When empty (default): uses "<namespace>_livox_frame" if namespace is set, ' | ||
| 'otherwise "livox_frame".' |
There was a problem hiding this comment.
The frame_id launch-argument description says the default uses "_livox_frame", but _launch_setup() actually defaults to f"{namespace}_link". Align the description with the implementation (or adjust the default) to avoid confusing users and mismatched TF frames.
| 'When empty (default): uses "<namespace>_livox_frame" if namespace is set, ' | |
| 'otherwise "livox_frame".' | |
| 'When empty (default): uses "<namespace>_link" if a namespace is set ' | |
| '(for example "mid360_link" when namespace is "mid360"), otherwise "_link".' |
There was a problem hiding this comment.
Fixed in f3b3a24. The description now correctly reads "<namespace>_link" (matching the actual default logic in _launch_setup).
marc-hanheide
left a comment
There was a problem hiding this comment.
@copilot in addition to addressing the review comments also consolidate all the tools (livox_* cli tools) into one CLI tool with different verbs (reset, reboot, search, setup).
add to the new CLI tools also functionality to upgrade the firmware (verb "firmware_upgrade") as implemented here: https://github.com/Livox-SDK/Livox-SDK2/blob/master/samples/multi_lidars_upgrade/main.cpp
| parameters=[ | ||
| {'xfer_format': int(LaunchConfiguration('xfer_format').perform(context))}, | ||
| {'multi_topic': int(LaunchConfiguration('multi_topic').perform(context))}, | ||
| {'data_src': int(LaunchConfiguration('data_src').perform(context))}, | ||
| {'publish_freq': float(LaunchConfiguration('publish_freq').perform(context))}, | ||
| {'output_data_type': int(LaunchConfiguration('output_type').perform(context))}, | ||
| {'frame_id': front_frame_id}, | ||
| {'user_config_path': config_path}, | ||
| {'cmdline_input_bd_code': ''}, | ||
| {'connect_timeout_s': float(LaunchConfiguration('connect_timeout_s').perform(context))}, | ||
| ], |
| 'When empty (default): uses "<namespace>_livox_frame" if namespace is set, ' | ||
| 'otherwise "livox_frame".' |
| | Lidar | Points topic | IMU topic | TF frame | | ||
| | --- | --- | --- | --- | | ||
| | Front | `/front_lidar/points` | `/front_lidar/imu` | `front_lidar_link` | | ||
| | Back | `/back_lidar/points` | `/back_lidar/imu` | `back_lidar_link` | | ||
|
|
There was a problem hiding this comment.
@copilot ensure the lidars use the correct different frame IDs
…to livox tool with firmware_upgrade Co-authored-by: marc-hanheide <1153084+marc-hanheide@users.noreply.github.com> Agent-Logs-Url: https://github.com/jabasai/livox_ros_driver2/sessions/fd894be1-58c0-4ec6-8e86-b7f30cf25cf3
Done in f3b3a24. All three tools (
|
ibrahimhroob
left a comment
There was a problem hiding this comment.
happy with this as it is working robustly now, from the tools I have mainly tested the scan, reboot and ip assign
Two separate driver instances each initialise their own Livox SDK, which broadcasts device-discovery packets that can steal the peer lidar's host-registration and interrupt its data stream. A timing delay between instances mitigates but does not reliably eliminate this. Running both lidars in one SDK instance makes the race structurally impossible.
Core change: single-node dual-lidar launch
launch/dual_mid360_launch.pynow launches onelivox_ros_driver2_nodemanaging both MID360s. A combined config is generated at launch time with two per-IPhost_net_infoentries in array format — required to key each entry to a specific lidar IP rather than all MID360s on the subnet:Topics are remapped to
/front_lidar/points,/front_lidar/imu,/back_lidar/points,/back_lidar/imu. All connectivity params (IPs, ports, host IP) are resolvable via launch args or env vars (FRONT_LIDAR_LIVOX_IP,BACK_LIDAR_LIVOX_IP, etc.).Thread-safety & logging fixes (from
copilot/sub-pr-1)src/comm/comm.h—connect_statechanged tostd::atomic<LidarConnectState>; addedstd::atomic<bool> data_receivedsrc/comm/pub_handler—AllowHandle()allowlist drops packets from unexpected lidar handlessrc/livox_ros_driver2.cpp/src/driver_node.h— one-shot watchdog timer (shuts down for respawn if no lidar connects withinconnect_timeout_s); Race-3 reconnect timer re-asserts work-mode every 3 s for any Sampling lidar with no data flowsrc/include/livox_log.h— node-freeLIVOX_INFO/WARN/ERROR/DEBUGmacros routing toRCLCPP_*(ROS2) orROS_*(ROS1)Per-lidar TF frame IDs
The driver now supports independent TF frame IDs per lidar via the
lidar_frame_idsROS parameter ("ip1:frame1,ip2:frame2"format):src/lddc.h/src/lddc.cpp—Lddc::SetLidarFrameIds()populates a handle→frame_id map;GetFrameId(index)resolves the correct frame per lidar for all message types (PointCloud2, CustomMsg, PCL, IMU). Also fixes a pre-existing bug where IMU messages hadframe_idhardcoded to"livox_frame".src/livox_ros_driver2.cpp— declares and parseslidar_frame_idsparameter, converts IP strings to handles, and callsSetLidarFrameIds().launch/dual_mid360_launch.py— passeslidar_frame_ids: "<front_ip>:front_lidar_link,<back_ip>:back_lidar_link"so front and back lidars each publish with their own distinct TF frame.Unified
livoxCLI toolAll standalone tools are consolidated into a single
src/tools/livox.cppwith sub-commands:searchsetuprebootresetfirmware_upgradeSetLivoxLidarUpgradeFirmwarePath+UpgradeLivoxLidarsCMakeLists.txtnow builds a singlelivoxtarget (replacing the formerlivox_scan,livox_set_ip, andlivox_reboottargets). README §5 updated with documentation for all five verbs.Launch & configuration fixes
launch/mid360_launch.py— correctedframe_idargument description (was<namespace>_livox_frame, now correctly<namespace>_link)launch_ROS2/renamed tolaunch/;launch_ROS1/removed (ROS2-only branch).github/copilot-instructions.md,.github/workflows/,.devcontainer/addedCMakeLists.txtauto-detects ROS version from environment📱 Kick off Copilot coding agent tasks wherever you are with GitHub Mobile, available on iOS and Android.