Add docs for Denoise Layer plugin (costmap_2d) #209

ryzhikovas · 2021-09-17T08:23:24Z

Docs for this feature.

AlexeyMerzlyakov

This is pretty large plugin being adding into Nav2 stack. So, please add new "New Plugins" section to "Galactic to Humble" migration guide where to make a short description and reference to the Denoise Layer plugin there. You can find similar sections in Galactic and Foxy migration guides as an example.

Also, there are some minor fixes from below before one can go into mainstream.

configuration/packages/configuring-costmaps.rst

configuration/packages/costmap-plugins/denoise.rst

plugins/index.rst

plugin_tutorials/docs/filtering_of_noise-induced_obstacles.rst

ryzhikovas · 2022-04-15T08:01:15Z

I have fixed the issues mentioned. If at the same time I did not introduce new shortcomings, then I am ready to synchronize (fetch + rebase) whis the parent branch.

AlexeyMerzlyakov

Thank you for the thorough work there! Some small changes are required and we could go.

plugin_tutorials/docs/filtering_of_noise-induced_obstacles.rst

plugins/index.rst

migration/Galactic.rst

ryzhikovas · 2022-04-19T19:13:37Z

Fixed in these last two commits [1], [2]. In addition, performed the rebase.

AlexeyMerzlyakov

LGTM

plugin_tutorials/index.rst

configuration/packages/costmap-plugins/denoise.rst

migration/Galactic.rst

SteveMacenski · 2022-04-28T18:19:55Z

plugin_tutorials/docs/filtering_of_noise-induced_obstacles.rst

+1. Enable Denoise Layer
+-----------------------
+
+Denoise Layer is Costmap2D plugin. You can enable the ``DenoiseLayer`` plugin in Costmap2D by adding ``denoise_layer`` to the ``plugins`` parameter in ``nav2_params.yaml``. You can place it in the ``global_costmap`` and (or) ``local_costmap`` to filter noise on a global or local map. The DenoiseLayer plugin should have the following parameter defined:


I would add a note here that this is typically most usable in the local costmap, though usable in the global costmap as well. Since the local costmap updates are much smaller, it will have less impact on run-time performance. I'm concerned that if users generally use this in the global costmap, it will cause performance issues when paired with typically-high-range lidars of 20+ meters which the update window may be massive.

Another reason is that since this works only in the update window, if you have salt and pepper noise in the local costmap, it disappears and is not remembered when going back into that region since the local costmap doesn't store data in areas after it has left.

The global costmap does though, so if we leave an area and the salt / pepper noise was being handled by this later, it would still be there when we came back. This isn't necessarily a problem, but makes me want to make the point to users that you can do this, but there are some real things to think about if doing so.

Done. If I misunderstood your thought or described it poorly, please help me with a clear statement of this warning.

Done in 68827cc after rebase

AlexeyMerzlyakov

Please re-base over latest main branch; and check Steve and my comments

migration/Galactic.rst

plugin_tutorials/docs/filtering_of_noise-induced_obstacles.rst

AlexeyMerzlyakov

The documentation updates seems to be already ironed in previous reviews. There are a few strokes and I see no problem to go.

tutorials/docs/filtering_of_noise-induced_obstacles.rst

AlexeyMerzlyakov

One comment (please restore back clarification). After it, LGTM.
@SteveMacenski, could you also please take a look on the PR?

tutorials/docs/filtering_of_noise-induced_obstacles.rst

ryzhikovas · 2023-03-21T21:18:08Z

@SteveMacenski, @AlexeyMerzlyakov, does this pull request still have a chance of being accepted? If yes, I will keep this PR relevant for the main branch.

SteveMacenski · 2023-03-22T15:54:46Z

Yes, Alexey messaged me last week about reviewing this after he's given his approvals and its my plan to do it the week after next (unfortunately, I'm no longer at Samsung Research so my time for reviewing large scale changes is limited between other responsibilities, but I'm committed to having your work represented in Nav2 given how much time you've put into it).

@AlexeyMerzlyakov what would help me greatly is if you reviewed your review comments and "Resolve" the ones that are completed to your satisfaction already so that they're no longer expanded in the thread. That way, I can look at only the ones that are not yet complete while I read through the full thread in detail as I'm reviewing the code for context. It would be good to know with certainty which are "done" and which might have been forgotten about. Ditto for the nav2 repo PR.

AlexeyMerzlyakov · 2023-03-23T07:12:18Z

@AlexeyMerzlyakov what would help me greatly is if you reviewed your review comments and "Resolve" the ones that are completed to your satisfaction already

It looks like I have no access or rights to do that in this repository. The PR review items do not have "Resolve" button anymore:

SteveMacenski · 2023-03-23T17:43:13Z

Ask and ye shall be given (you should receive an email for getting write access). You should see them once you accept

AlexeyMerzlyakov · 2023-04-07T08:05:10Z

@SteveMacenski, thank you for the support!
I've closed all resolved items. One is remained to check that this is exactly what you expect.

tutorials/docs/filtering_of_noise-induced_obstacles.rst

SteveMacenski · 2023-04-18T16:27:27Z

Remove those 3 lines (and maybe reword the "Firstly" since now there's only 1 item?) and this LGTM after fixing the merge conflict

Great job on the tutorial, its rare I read one where I have no substantive remarks. I had some wording nitpicks but I took care of those myself, at least those that I could from the github UI.

Signed-off-by: ryzhikovas <ryzhikovas@gmail.com>

ryzhikovas · 2023-04-25T08:52:15Z

Thank you for the exact wording. The small number of edits is the merit of Alexey, who helped a lot with the documentation at an early stage.

SteveMacenski

Approved pending code review itself

ryzhikovas mentioned this pull request Sep 17, 2021

Impl noise filtering layer in the costmap_2d ros-navigation/navigation2#2567

Merged

6 tasks

AlexeyMerzlyakov reviewed Mar 31, 2022

View reviewed changes

AlexeyMerzlyakov reviewed Apr 19, 2022

View reviewed changes

ryzhikovas force-pushed the feature-costmap2d-denoise branch from 59b9f7f to 6513710 Compare April 19, 2022 19:04

AlexeyMerzlyakov approved these changes Apr 20, 2022

View reviewed changes

SteveMacenski reviewed Apr 28, 2022

View reviewed changes

AlexeyMerzlyakov reviewed Dec 26, 2022

View reviewed changes

migration/Galactic.rst Outdated Show resolved Hide resolved

plugin_tutorials/docs/filtering_of_noise-induced_obstacles.rst Outdated Show resolved Hide resolved

ryzhikovas force-pushed the feature-costmap2d-denoise branch from 6513710 to 279ed83 Compare January 30, 2023 15:23

AlexeyMerzlyakov reviewed Feb 1, 2023

View reviewed changes

tutorials/docs/filtering_of_noise-induced_obstacles.rst Outdated Show resolved Hide resolved

tutorials/docs/filtering_of_noise-induced_obstacles.rst Outdated Show resolved Hide resolved

AlexeyMerzlyakov approved these changes Feb 3, 2023

View reviewed changes

tutorials/docs/filtering_of_noise-induced_obstacles.rst Outdated Show resolved Hide resolved

ryzhikovas force-pushed the feature-costmap2d-denoise branch from 4ac0320 to 4ca7a45 Compare March 21, 2023 21:17

SteveMacenski requested changes Apr 18, 2023

View reviewed changes

ryzhikovas added 10 commits April 25, 2023 11:09

Add docs for Denoise Layer plugin (costmap_2d)

92c2da5

Signed-off-by: ryzhikovas <ryzhikovas@gmail.com>

Move config example to plugin params page

68dd94c

Remove unnecessary extra spaces

fcd0bd6

Minor improvements

d8a8a0b

Add a short description to the migration guide

f69d529

Signed-off-by: ryzhikovas <ryzhikovas@gmail.com>

Improves the tutorial

11c35a6

Fix markup

3b27923

Improve content and fix errors

52bad2c

Move to general tutorials list

8ef62f0

Default config only includes the local_costmap

f2df439

ryzhikovas and others added 14 commits April 25, 2023 11:09

Fix headline in migration notes

a5b3dc0

Add a warning about the global costmap

dcf6118

Move to actual migration guide

f6ae123

Fix launch command

ff3291e

Fix typo

c3ca185

Fix interpretation of cells

f8850e5

Restore clarification

e863523

Update tutorials/docs/filtering_of_noise-induced_obstacles.rst

9e70835

Update tutorials/docs/filtering_of_noise-induced_obstacles.rst

dadb4d2

Update tutorials/docs/filtering_of_noise-induced_obstacles.rst

157fe58

Update tutorials/docs/filtering_of_noise-induced_obstacles.rst

32681d7

Update tutorials/docs/filtering_of_noise-induced_obstacles.rst

f330416

Update tutorials/docs/filtering_of_noise-induced_obstacles.rst

3a8056c

Remove some technical details

3cd2571

ryzhikovas force-pushed the feature-costmap2d-denoise branch from 7175bd7 to 3cd2571 Compare April 25, 2023 08:14

SteveMacenski approved these changes Apr 26, 2023

View reviewed changes

Merge branch 'master' into feature-costmap2d-denoise

451a6cf

AlexeyMerzlyakov merged commit adb5213 into ros-navigation:master May 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add docs for Denoise Layer plugin (costmap_2d) #209

Add docs for Denoise Layer plugin (costmap_2d) #209

ryzhikovas commented Sep 17, 2021 •

edited

Loading

AlexeyMerzlyakov left a comment •

edited

Loading

ryzhikovas commented Apr 15, 2022

AlexeyMerzlyakov left a comment •

edited

Loading

ryzhikovas commented Apr 19, 2022

AlexeyMerzlyakov left a comment

SteveMacenski Apr 28, 2022

ryzhikovas Jan 30, 2023

AlexeyMerzlyakov Apr 7, 2023 •

edited

Loading

AlexeyMerzlyakov left a comment

AlexeyMerzlyakov left a comment

AlexeyMerzlyakov left a comment

ryzhikovas commented Mar 21, 2023

SteveMacenski commented Mar 22, 2023 •

edited

Loading

AlexeyMerzlyakov commented Mar 23, 2023 •

edited

Loading

SteveMacenski commented Mar 23, 2023

AlexeyMerzlyakov commented Apr 7, 2023

SteveMacenski commented Apr 18, 2023 •

edited

Loading

ryzhikovas commented Apr 25, 2023

SteveMacenski left a comment

Add docs for Denoise Layer plugin (costmap_2d) #209

Add docs for Denoise Layer plugin (costmap_2d) #209

Conversation

ryzhikovas commented Sep 17, 2021 • edited Loading

AlexeyMerzlyakov left a comment • edited Loading

Choose a reason for hiding this comment

ryzhikovas commented Apr 15, 2022

AlexeyMerzlyakov left a comment • edited Loading

Choose a reason for hiding this comment

ryzhikovas commented Apr 19, 2022

AlexeyMerzlyakov left a comment

Choose a reason for hiding this comment

SteveMacenski Apr 28, 2022

Choose a reason for hiding this comment

ryzhikovas Jan 30, 2023

Choose a reason for hiding this comment

AlexeyMerzlyakov Apr 7, 2023 • edited Loading

Choose a reason for hiding this comment

AlexeyMerzlyakov left a comment

Choose a reason for hiding this comment

AlexeyMerzlyakov left a comment

Choose a reason for hiding this comment

AlexeyMerzlyakov left a comment

Choose a reason for hiding this comment

ryzhikovas commented Mar 21, 2023

SteveMacenski commented Mar 22, 2023 • edited Loading

AlexeyMerzlyakov commented Mar 23, 2023 • edited Loading

SteveMacenski commented Mar 23, 2023

AlexeyMerzlyakov commented Apr 7, 2023

SteveMacenski commented Apr 18, 2023 • edited Loading

ryzhikovas commented Apr 25, 2023

SteveMacenski left a comment

Choose a reason for hiding this comment

ryzhikovas commented Sep 17, 2021 •

edited

Loading

AlexeyMerzlyakov left a comment •

edited

Loading

AlexeyMerzlyakov left a comment •

edited

Loading

AlexeyMerzlyakov Apr 7, 2023 •

edited

Loading

SteveMacenski commented Mar 22, 2023 •

edited

Loading

AlexeyMerzlyakov commented Mar 23, 2023 •

edited

Loading

SteveMacenski commented Apr 18, 2023 •

edited

Loading