fix(ad_api_adaptors): fix to merge waypoint #2215
Conversation
isamu-takagi
requested review from
mitsudome-r,
TakaHoribe and
takayuki5168
as code owners
Codecov ReportBase: 11.09% // Head: 11.09% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #2215 +/- ##
==========================================
- Coverage 11.09% 11.09% -0.01%
==========================================
Files 1202 1202
Lines 86169 86219 +50
Branches 20682 20682
==========================================
Hits 9564 9564
- Misses 66523 66573 +50
Partials 10082 10082
*This pull request uses carry forward flags. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| this, get_clock(), rate.period(), std::bind(&RoutingAdaptor::on_timer, this)); | ||
|
|
||
| state_ = RouteState::Message::UNKNOWN; | ||
| route_ = std::make_shared<SetRoutePoints::Service::Request>(); |
There was a problem hiding this comment.
Setting nullptr first sounds better to me.
If you don't think it's good, just ignore please.
There was a problem hiding this comment.
This value is always used and does not need to be nullptr.
There was a problem hiding this comment.
Then why do you make route_ shared pointer?
There was a problem hiding this comment.
Because the async_send_request function only supports shared pointer.
| constexpr int delay_count = 3; | ||
| if (0 < request_control_ && request_control_ < delay_count) { | ||
| ++request_control_; | ||
| } | ||
| if (request_control_ != delay_count) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
I don't understand this algorithm well.
Why 3 is chosen?
What does merge waypoints mean.
request_control_ seems to have additional meaning, not only what the variable name expresses.
Could you write a comment of what request_control_ means.
Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>
* fix(ad_api_adaptors): fix to merge waypoint * fix(ad_api_adaptors): update comments and variable name Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp> Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp> Signed-off-by: Kotaro Yoshimoto <pythagora.yoshimoto@gmail.com>
* fix(ad_api_adaptors): fix to merge waypoint * fix(ad_api_adaptors): update comments and variable name Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp> Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp> Signed-off-by: yoshiri <yoshiyoshidetteiu@gmail.com>
Description
Fix the bug where consecutive waypoints would be ignored.
Pre-review checklist for the PR author
The PR author must check the checkboxes below when creating the PR.
In-review checklist for the PR reviewers
The PR reviewers must check the checkboxes below before approval.
Post-review checklist for the PR author
The PR author must check the checkboxes below before merging.
After all checkboxes are checked, anyone who has write access can merge the PR.