-
Notifications
You must be signed in to change notification settings - Fork 88
feat(planning_test_manager): subscription callback #563
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat(planning_test_manager): subscription callback #563
Conversation
Thank you for contributing to the Autoware project! 🚧 If your pull request is in progress, switch it to draft mode. Please ensure:
|
2bb0934
to
c79246d
Compare
There was a problem hiding this 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 introduces a new overload for PlanningInterfaceTestManager::subscribeOutput
that accepts a custom callback and adds a test verifying that subscription callbacks receive the expected messages.
- Added a templated
subscribeOutput
overload taking a user-provided callback - Retained the original
subscribeOutput
behavior via an internal call to the new overload - Added
CallbackSubscriptionTest
to ensure messages are received and payloads match
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
testing/autoware_planning_test_manager/include/autoware/planning_test_manager/autoware_planning_test_manager.hpp | Added generic callback-based subscribeOutput and refactored existing overload |
testing/autoware_planning_test_manager/test/test_planning_test_manager.cpp | Added CallbackSubscriptionTest to verify the new subscription behavior |
Comments suppressed due to low confidence (1)
testing/autoware_planning_test_manager/include/autoware/planning_test_manager/autoware_planning_test_manager.hpp:81
- In a void function, you cannot return an expression; remove the
return
keyword and simply callsubscribeOutput<OutputT>(...)
.
return subscribeOutput<OutputT>(
testing/autoware_planning_test_manager/test/test_planning_test_manager.cpp
Outdated
Show resolved
Hide resolved
testing/autoware_planning_test_manager/test/test_planning_test_manager.cpp
Outdated
Show resolved
Hide resolved
103f1b7
to
ad9aec7
Compare
Signed-off-by: Grzegorz Głowacki <[email protected]>
bb715ef
to
13083b4
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #563 +/- ##
==========================================
- Coverage 47.79% 47.44% -0.35%
==========================================
Files 313 313
Lines 20076 20086 +10
Branches 8733 8704 -29
==========================================
- Hits 9595 9530 -65
- Misses 9660 9742 +82
+ Partials 821 814 -7
*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:
|
Thank you for your contribution! Now I'm doing tests by using TIER IV internal scenario evaluator. |
Sorry for being late, due to some my mistakes the tests are delayed. Let me do the tests again. Apologies for making you wait. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution. Only minor comments I have. This PR looks good 👍
Scenario Simulation on TIER IV Internal Tool (can be seen only by TIER IV Internal developer):
testing/autoware_planning_test_manager/test/test_planning_test_manager.cpp
Outdated
Show resolved
Hide resolved
testing/autoware_planning_test_manager/test/test_planning_test_manager.cpp
Outdated
Show resolved
Hide resolved
Signed-off-by: Grzegorz Głowacki <[email protected]>
d6576bc
to
45a4f4a
Compare
@sasakisasaki I've renamed the nodes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution! Though I put some minor comments, I agree with merging this PR 👍
auto test_manager = | ||
std::make_shared<autoware::planning_test_manager::PlanningInterfaceTestManager>(); | ||
|
||
auto test_target_node = std::make_shared<rclcpp::Node>("target_node_subscription_test"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
auto test_target_node = std::make_shared<rclcpp::Node>("target_node_subscription_test"); | |
auto test_target_node = std::make_shared<rclcpp::Node>("target_node_for_subscription_test"); |
auto test_manager = | ||
std::make_shared<autoware::planning_test_manager::PlanningInterfaceTestManager>(); | ||
|
||
auto test_target_node = std::make_shared<rclcpp::Node>("target_node_msg_validation_test"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
auto test_target_node = std::make_shared<rclcpp::Node>("target_node_msg_validation_test"); | |
auto test_target_node = std::make_shared<rclcpp::Node>("target_node_for_msg_validation_test"); |
Description
This small change allow checking the value of the subscribed message
Modifications
Added new overload PlanningInterfaceTestManager::subscribeOutput
Effects on system behavior
None.