fix failure transferring sensor data from carla to carma #19
Conversation
|
|
||
| /*Lidar*/ | ||
| if (!carla_lidar_stream_enabled) { | ||
| ROS_ERROR_STREAM("CARLA LIDAR data stream is disabled"); |
There was a problem hiding this comment.
I think error is probably the wrong log level for this, debug seems more appropriate. Nothing has necessarily gone wrong if we've disabled the simulated LIDAR.
| ROS_ERROR_STREAM("CARLA LIDAR data stream is disabled"); | ||
| } else if (localization_stream_enabled && object_detection_stream_enabled) { | ||
| throw std::invalid_argument("CARLA LIDAR sensor and both ground truth data streams cannot be enabled at the same time"); | ||
| } else { |
There was a problem hiding this comment.
Do we need a catch-all somewhere to check if we've got a minimal set of data to run with? If we turn off simulated lidar/gnss AND the localization truth data, we've got nothing to work with at all. I imagine CARMA Platform downstream of these nodes would complain in that case, but the error would be further away from the source than if we were to check that somewhere here.
| ros::CARMANodeHandle::spin(); | ||
| /*Camera*/ | ||
| if (!carla_camera_stream_enabled) { | ||
| ROS_ERROR_STREAM("CARLA camera data stream is disabled"); |
There was a problem hiding this comment.
Similar to above RE: debug level
| /*Camera*/ | ||
| if (!carla_camera_stream_enabled) { | ||
| ROS_ERROR_STREAM("CARLA camera data stream is disabled"); | ||
| } else if (carla_camera_stream_enabled && object_detection_stream_enabled) { |
There was a problem hiding this comment.
Minor: The check here again on carla_camera_stream_enabled is redundant and can be removed, since we're already in the else-branch of an if (!carla_camera_stream_enabled) ...
| } | ||
| /*GNSS*/ | ||
| if (!carla_gnss_stream_enabled) { | ||
| ROS_ERROR_STREAM("CARLA camera data stream is disabled"); |
| /*GNSS*/ | ||
| if (!carla_gnss_stream_enabled) { | ||
| ROS_ERROR_STREAM("CARLA camera data stream is disabled"); | ||
| } else if (carla_gnss_stream_enabled && localization_stream_enabled) { |
There was a problem hiding this comment.
first logical term redundant
| <arg name='loc_ground_truth_enabled' value="false"/> | ||
| <remap from="/gt_localization_stream" to="~loc_ground_truth_enabled"/> | ||
| <group if="$(eval loc_ground_truth_enabled == 'true')"> | ||
| <group if="$(eval arg('localization_stream') == 'true')"> |
There was a problem hiding this comment.
Is this really the right syntax? I'm a bit unfamiliar, but anything == true reads as weird to me. Is this doing raw string comparison instead of converting the arg into a boolean? I'd have to look through the launch files elsewhere to see if this pattern is used, or somewhere in the roslaunch docs.
Not a huge deal if it works, but just a bit weird.
|
Kudos, SonarCloud Quality Gate passed!
|
2 similar comments
|
Kudos, SonarCloud Quality Gate passed!
|
|
Kudos, SonarCloud Quality Gate passed!
|
PR Details
Description
Related Issue
Motivation and Context
How Has This Been Tested?
Types of changes
Checklist:
CARMA Contributing Guide