feat(autoware_lanelet2_extension): replace remaining lanelet2_extension utilities functions - bpp packages#12085
Conversation
Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com>
Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com>
(autoware_behavior_path_goal_planner_module) Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com>
(autoware_behavior_path_start_planner_module) Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com>
Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com>
Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com>
Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com>
Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com>
|
Thank you for contributing to the Autoware project! 🚧 If your pull request is in progress, switch it to draft mode. Please ensure:
|
|
Documentation URL: https://autowarefoundation.github.io/autoware_universe/pr-12085/ |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #12085 +/- ##
==========================================
+ Coverage 18.13% 23.52% +5.39%
==========================================
Files 1839 1873 +34
Lines 126417 127990 +1573
Branches 44329 46910 +2581
==========================================
+ Hits 22922 30110 +7188
- Misses 84399 85206 +807
+ Partials 19096 12674 -6422
*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:
|
| const auto combine_lanelet = lanelet::utils::combineLaneletsShape(concat_lanelets); | ||
| // concat_lanelets is already confirmed to have at least one lanelet (nearest lanelet) | ||
| const auto combine_lanelet = | ||
| *autoware::experimental::lanelet2_utils::combine_lanelets_shape(concat_lanelets); |
There was a problem hiding this comment.
Do not dereference optional without null-check ! In this case you should return false if it was nullopt
| // store combined lane and its front lane | ||
| const auto & combined_and_first = std::pair<lanelet::ConstLanelet, lanelet::ConstLanelet>( | ||
| lanelet::utils::combineLaneletsShape(combined_lane_elems), combined_lane_elems.front()); | ||
| *autoware::experimental::lanelet2_utils::combine_lanelets_shape(combined_lane_elems), |
There was a problem hiding this comment.
I'd prefer
- (0) do not check combine_lane_elems
- (1) obtain
combine_lanelets_shapeof optional type directly - (2) do the process
Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com>
Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com>
Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com>
| const auto combined_target_lane = lanelet::utils::combineLaneletsShape(target_lanes); | ||
| const auto combined_target_lane_opt = | ||
| autoware::experimental::lanelet2_utils::combine_lanelets_shape(target_lanes); | ||
| if (!combined_target_lane_opt.has_value()) { |
There was a problem hiding this comment.
| if (!combined_target_lane_opt.has_value()) { | |
| if (!combined_target_lane_opt) { |
There was a problem hiding this comment.
@zulfaqar-azmi-t4
Thank you for your review and comments!
In this PR (and other related PRs), I used the same expression for the consistency.
Is there any performance issue with using has_value() instead of implicitly converting opt to bool (only opt)?
Is it okay to keep has_value() for consistency?
There was a problem hiding this comment.
It is just about style, there is no performance issue since operator bool() is called internally
There was a problem hiding this comment.
Thank you for the explanation. Within lane change module, normally we're using if (!combined_target_lane_opt) { directly. Personally I would like to keep the consistency that way. But then it is also okay, so I'll leave it to you,
There was a problem hiding this comment.
Thank you for the suggestion. I’ve updated it to keep consistency with the lane change module (I found one more usage of has_value, so I removed it there too).
…ets opt Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com>
Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com>
…on utilities functions - bpp packages (autowarefoundation#12085) * replace getArcCoordinates in bpp packages Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com> * replace getArcCoordinates usage in bpp packages (2) Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com> * replace getArcCoordinates in bpp package (3) (autoware_behavior_path_goal_planner_module) Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com> * replace getArcCoordinates in bpp package(4) (autoware_behavior_path_start_planner_module) Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com> * replace getLateralDistanceToClosestLanelet in bpp package Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com> * replace getExpandedLanelet(s) in bpp package Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com> * replace combineLaneletsShape in bpp packages Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com> * fix wrong condition in bpp Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com> * style(pre-commit): autofix * remove directly dereference opt for combine_lanelet Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com> * remove log if lanelet is empty Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com> * bind reference to optional value for combine_lanelets_shape Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com> * remove log and simply return outside else in get_dirty_expanded_lanelets opt Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com> * remove .has_value in lane_change_module for consistency Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com> --------- Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com> Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Co-authored-by: Mamoru Sobue <hilo.soblin@gmail.com>
…on utilities functions - bpp packages (autowarefoundation#12085) * replace getArcCoordinates in bpp packages Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com> * replace getArcCoordinates usage in bpp packages (2) Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com> * replace getArcCoordinates in bpp package (3) (autoware_behavior_path_goal_planner_module) Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com> * replace getArcCoordinates in bpp package(4) (autoware_behavior_path_start_planner_module) Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com> * replace getLateralDistanceToClosestLanelet in bpp package Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com> * replace getExpandedLanelet(s) in bpp package Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com> * replace combineLaneletsShape in bpp packages Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com> * fix wrong condition in bpp Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com> * style(pre-commit): autofix * remove directly dereference opt for combine_lanelet Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com> * remove log if lanelet is empty Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com> * bind reference to optional value for combine_lanelets_shape Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com> * remove log and simply return outside else in get_dirty_expanded_lanelets opt Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com> * remove .has_value in lane_change_module for consistency Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com> --------- Signed-off-by: Sarun Mukdapitak <sarun.mukda@gmail.com> Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Co-authored-by: Mamoru Sobue <hilo.soblin@gmail.com>
Description
This PR is a small PR separated from #12014 for
behavior_path_plannerpackages in planning componentThese functions in
autoware_lanelet2_extensionthat are ported in #760, and will be deprecated inautoware_lanelet2_extensionPR #95.This PR will replace their usage:
Target functions:
getArcCoordinatesgetLateralDistanceToCenterline❌getLateralDistancetoClosestLanelet->get_lateral_distance_to_centerline❌combineLaneletsShape=========================================================
getCenterlineWithOffsetgetRightBoundWithOffset❌getLeftBoundWithOffset❌==========================================================
getExpandedLanelet-> portedget_dirty_expanded_lanelet❌getExpandedLanelets-> portedget_dirty_expanded_lanelets❌Note:
❌: does not exist in
autoware_universe- bpp packagesRelated links
autoware_corePR #842autoware_lanelet2_extensionPR #95planning #12083
bvp #12084
evaluator #12086
Parent Issue:
How was this PR tested?
Pass CI and Evaluator (CommonScenario_Basic)
Notes for reviewers
None.
Interface changes
None.
Effects on system behavior
None.