-
Notifications
You must be signed in to change notification settings - Fork 88
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
planner_cspace: publish internally used maps as OccupancyGrid #396
Conversation
This comment has been minimized.
This comment has been minimized.
Codecov Report
@@ Coverage Diff @@
## master #396 +/- ##
==========================================
+ Coverage 73.95% 74.68% +0.72%
==========================================
Files 58 58
Lines 4251 4258 +7
==========================================
+ Hits 3144 3180 +36
+ Misses 1107 1078 -29
Continue to review full report at Codecov.
|
adb93db
to
5052cd2
Compare
This comment has been minimized.
This comment has been minimized.
[#1130] PASSED on kineticAll tests passed
|
[#1130] PASSED on melodicAll tests passed
|
@DaikiMaekawa please take a look. |
remember_updates: true | ||
hist_ignore_range: 0.0 | ||
hist_ignore_range_max: 1.5 | ||
</rosparam> |
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.
I know this works fine, but shouldn't these parameters be located inside <node pkg="planner_cspace" type="planner_3d" name="planner_3d">
?
planner_3d
namespace is not constant.
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.
navigation.launch
was originally a skeleton code for real usage, and demo.launch
is for demonstration which should not be in navigation.launch
.
However, currently navigation.launch
is much specific for demonstration. It maybe better to directly put it into neonavigation_launch/config/navigate.yaml
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.
I'm going to move these parameters to config/navigation.yaml
for now and reorganize neonavigation_launch
in #398.
continue; | ||
if (cost_estim_cache_[p] == FLT_MAX) | ||
continue; | ||
point.z = cost_estim_cache_[p] / 500; |
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.
where does 500
comes from?
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.
It's a magic parameter of the visualization used in the master branch. In this PR, it just keep compatibility with the previous behaviour.
[#1132] PASSED on kineticAll tests passed
[#1132] PASSED on melodicAll tests passed
|
[#1133] PASSED on kineticAll tests passed
[#1133] PASSED on melodicAll tests passed
|
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.
LGTM
fix #395