test(traffic_light_map_based_detector): add integration test#12396
Conversation
Signed-off-by: Takahisa.Ishikawa <takahisa.ishikawa@tier4.jp>
|
Thank you for contributing to the Autoware project! 🚧 If your pull request is in progress, switch it to draft mode. Please ensure:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12396 +/- ##
==========================================
+ Coverage 18.46% 19.00% +0.53%
==========================================
Files 1888 1894 +6
Lines 129807 130625 +818
Branches 45966 48149 +2183
==========================================
+ Hits 23968 24822 +854
- Misses 86102 86570 +468
+ Partials 19737 19233 -504
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
LGTM!
I have a comment for the code which not that much important.
Also, for the PR comment's figure, it seems it has some typo.
The linestring for a traffic light should be placed at bottom of actual traffic light position.
So the traffic light's height is from z=3.5 to z=4.5:
traffic light
/
───┌─┐ ^
│●│ |
│●│ | 1 m
│●│ |
└─┘ v _____
^
| 3.5 m
z = 0 __________v
| EXPECT_EQ(rois[0].traffic_light_id, traffic_light_linestring_id_); | ||
| EXPECT_NEAR(rois[0].roi.x_offset, 301, 1); | ||
| EXPECT_NEAR(rois[0].roi.y_offset, 140, 1); | ||
| EXPECT_NEAR(rois[0].roi.width, 37, 1); | ||
| EXPECT_NEAR(rois[0].roi.height, 39, 1); | ||
|
|
||
| // expect/rois has zero vibration offset, so exact integer values from geometry | ||
| auto expect_rois = getExpectRois(); | ||
| ASSERT_EQ(expect_rois.size(), 1u); | ||
| EXPECT_EQ(expect_rois[0].traffic_light_id, traffic_light_linestring_id_); | ||
| EXPECT_EQ(expect_rois[0].roi.x_offset, 310u); | ||
| EXPECT_EQ(expect_rois[0].roi.y_offset, 150u); | ||
| EXPECT_EQ(expect_rois[0].roi.width, 20u); | ||
| EXPECT_EQ(expect_rois[0].roi.height, 20u); |
There was a problem hiding this comment.
Following comment may be too much for just checking the code, but I will leave it here.
We can leave the value as for some reference to do some sanity check (detect the logical or numerical changes), but we can compute the actual value quite easily for an ideal camera like following. So it would be better to leave some comments for the (likely a) magic numbers.
camera at (0, 0, 0), looking at +x
// world -> camera
// downfward is y+ in camera coordinate
top left : (20.0, 0.5, 3.5 + 1.0) -> (-0.5, -4.5, 20.0)
right bottom: (20.0, -0.5, 3.5) -> ( 0.5, -3.5, 20.0)
for ideal camera
c_x = 320
c_y = 240
u = f_x * (x_opt / z_opt) + c_x
v = f_y * (y_opt / z_opt) + c_y
------------------------------------------------------------------------------
without margin (expect ROI)
top left point:
u_top_left = f_x * (x_opt / z_opt) + c_x = 400 * (-0.5 / 20) + 320 = 310
v_top_left = f_y * (y_opt / z_opt) + c_y = 400 * (-4.5 / 20) + 240 = 150
bottom right point
u_bottom_right = f_x * (x_opt / z_opt) + c_x = 400 * (0.5 / 20) + 320 = 330
v_bottom_right = f_y * (y_opt / z_opt) + c_y = 400 * (-3.5 / 20) + 240 = 170
(x, y, w, h) -> (310, 150, 20, 20)
------------------------------------------------------------------------------
with margin (rough ROI)
margin def.:
margin_x = (margin_yaw / 2) * depth + margin_width / 2
margin_y = (margin_pitch / 2) * depth + margin_height / 2
margin_z = margin_depth / 2
margin_x = (0.01745 / 2) * 20.0 + (0.5 / 2) = 0.4245
margin_y = (0.01745 / 2) * 20.0 + (0.5 / 2) = 0.4245
margin_z = 0.5 / 2 = 0.25
top left point (tl_camera_optical - margin):
u_top_left = 400 * ((-0.5 - 0.4245) / (20 - 0.25)) + 320 ≒ 301.276
v_top_left = 400 * ((-4.5 - 0.4245) / (20 - 0.25)) + 240 ≒ 140.263
bottom right point (tl_camera_optical + margin):
u_bottom_right = 400 * ((0.5 + 0.4245) / (20 + 0.25)) + 320 ≒ 338.262
v_bottom_right = 400 * ((-3.5 + 0.4245) / (20 + 0.25)) + 240 ≒ 179.249
(x, y, w, h) -> (301.276, 140.263, 36.986 38.986) ≒ (301, 140, 37, 39)
…ion comments to integration test Signed-off-by: Takahisa.Ishikawa <takahisa.ishikawa@tier4.jp>
|
@a-maumau |
Description
Add an integration test for
autoware_traffic_light_map_based_detectornode (MapBasedDetector).This test verifies the end-to-end pipeline: publishing a lanelet map with a traffic light regulatory element, a route, TF, and camera info, then asserting that the node correctly produces ROIs (both
output/roiswith vibration margin andexpect/roiswithout).The following diagram illustrates the test setup — camera placement, traffic light position, and the expected ROI projection:
Related links
Parent Issue:
How was this PR tested?
1. All tests pass
2. Test coverage
Coverage was measured using
lcovwith a Debug build instrumented with--coverageflags.Summary
traffic_light_map_based_detector_node.cpptraffic_light_map_based_detector_process.cppNotes
traffic_light_map_based_detector_process.cpp(utility functions) achieves 96.4% line coverage, with the existingtest_utilscovering the remaining utility pathstraffic_light_map_based_detector_node.cppreaches 82.1% line coverage — the uncovered lines are primarily error handling paths (e.g., TF lookup failure, empty camera info) and theall_traffic_lightsfallback path when no route is availableNotes for reviewers
This is a specification test added before refactoring to prevent regressions. It captures the current behavior of the node as-is, so that any unintended changes during refactoring are caught immediately. This test is intended to be removed after the refactoring is complete and replaced by unit tests for the extracted logic classes.
Interface changes
None.
Effects on system behavior
None. This PR only adds tests.