implemented 'update costmap on request' option#3297
implemented 'update costmap on request' option#3297JohnTGZ wants to merge 1 commit intoros-navigation:humblefrom
Conversation
|
@JohnTGZ, please properly fill in PR template in the future. @SteveMacenski, use this instead.
|
AlexeyMerzlyakov
left a comment
There was a problem hiding this comment.
@JohnTGZ, Thank you for taking care on this issue. Please refer to the review items. And then make a PR to a Nav2 main branch instead of humble (since we are working with main branch in Nav2 first).
| stop_updates_ = false; | ||
| map_update_thread_shutdown_ = false; | ||
|
|
||
| updateMapOnRequest(); |
There was a problem hiding this comment.
Despite it is never guaranteed (that is really deserves a separate bug ticket), plugin->activate() should be called once before main updating loop with plugin->updateBounds(), plugin->updateCosts() calls. Therefore, updateMapOnRequest() can not be called before the start(), to not cause to plugin->updateCosts() to be executed before plugin is being initialized.
Please note, that to ensure that start() to work well without parallel execution with main update loop, initialized_ variable should be set to true.
There was a problem hiding this comment.
Thanks for pointing this out, I studied the code in more detail and this is an important detail to take note of
|
@AlexeyMerzlyakov Thanks for the feedback, I have studied the code in more detail to understand the implications behind the code Im changing. I have addressed these comments in another branch that merges with |
Yep. Once you will resolve all review items from this PR, please open a new one for |
|
This PR has been shifted to #3300 as this PR is for the wrong branch. |
Basic Info
This PR targets #3121, adding the 'update costmap on request' option via the parameter
update_on_requestwhich can be specified on the costmap layer, as shown in nav2_bringup/params/nav2_params.yaml.Description of contribution in a few bullet points
update_on_requestis set toTRUE, the value ofupdate_frequencyis set to0.0, therefore disabling the costmap update loop on the thread.waitForCostmap()instead of the costmap being updated on a fixed loop.computeControl().update_on_requestisFALSE, the behaviour is as default.Description of documentation updates required from your changes
Will need to update the documentation for the
update_on_requestparameter and how setting it toTRUEwill affect the value ofupdate_frequencyinternally.Future work that may be required in bullet points
If
update_on_requestis set toTRUE, it is possible to disable initializing the thread for the map update loop as it won't be utilised.For Maintainers: