Skip to content
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

Global costmap resized by static layer even if layered_costmap_->isSizeLocked() is set to true #768

Closed
ghost opened this issue Jul 25, 2018 · 6 comments

Comments

@ghost
Copy link

ghost commented Jul 25, 2018

In the function staticLayer::incomingMap(navigation/costmap_2d/plugins/static_layer.cpp line 166), the first if statement results in master map(layered_costmap) being resized even if the setting for layered_costmap_->isSizeLocked is set to true. This seems unintended to me and I was wondering the changes below should be made.

There are few other changes to be made for this to fully work, but for now I just wanted to ask whether I was correct in reading this as a bug.

The lines in question:

if (!layered_costmap_->isRolling() && (master->getSizeInCellsX() != size_x ||
master->getSizeInCellsY() != size_y ||
master->getResolution() != new_map->info.resolution ||
master->getOriginX() != new_map->info.origin.position.x ||
master->getOriginY() != new_map->info.origin.position.y ||
!layered_costmap_->isSizeLocked()))
{
// Update the size of the layered costmap (and all layers, including this one)
ROS_INFO("Resizing costmap to %d X %d at %f m/pix", size_x, size_y, new_map->info.resolution);
layered_costmap_->resizeMap(size_x, size_y, new_map->info.resolution, new_map->info.origin.position.x,
new_map->info.origin.position.y, true);
}

And the changes I'm suggesting:

-  if (!layered_costmap_->isRolling() && (master->getSizeInCellsX() != size_x ||
+  if (!layered_costmap_->isRolling() && !layered_costmap_->isSizeLocked()
+   && (master->getSizeInCellsX() != size_x ||
    master->getSizeInCellsY() != size_y ||
    master->getResolution() != new_map->info.resolution ||
    master->getOriginX() != new_map->info.origin.position.x ||
-   master->getOriginY() != new_map->info.origin.position.y ||
-   !layered_costmap_->isSizeLocked()))
+   master->getOriginY() != new_map->info.origin.position.y)) 
@moriarty
Copy link
Contributor

Sorry this comment won't answer your question. I just saw a wall of unformatted text and had to say something.

I guess there is no "how to file a bug" for this repository, but some more strict repos would require a little bit more basic information for example

FYI, if you find the lines in GitHub, you can click on them, and then select "copy permalink" which is much easier to read... Then you can also use three ` marks, to type code blocks and use diff format to show clearer what you mean.

if (!layered_costmap_->isRolling() && (master->getSizeInCellsX() != size_x ||
master->getSizeInCellsY() != size_y ||
master->getResolution() != new_map->info.resolution ||
master->getOriginX() != new_map->info.origin.position.x ||
master->getOriginY() != new_map->info.origin.position.y ||
!layered_costmap_->isSizeLocked()))
{
// Update the size of the layered costmap (and all layers, including this one)
ROS_INFO("Resizing costmap to %d X %d at %f m/pix", size_x, size_y, new_map->info.resolution);
layered_costmap_->resizeMap(size_x, size_y, new_map->info.resolution, new_map->info.origin.position.x,
new_map->info.origin.position.y, true);
}

I might have made a typo, but from reading your text and typing it again (this is easier if you try it on your machine first and just copy some of the output of git diff), I think you want to replace the lines above, with:

-  if (!layered_costmap_->isRolling() && (master->getSizeInCellsX() != size_x ||
+  if (!layered_costmap_->isRolling() && !layered_costmap_->isSizeLocked()
+   && (master->getSizeInCellsX() != size_x ||
    master->getSizeInCellsY() != size_y ||
    master->getResolution() != new_map->info.resolution ||
    master->getOriginX() != new_map->info.origin.position.x ||
-   master->getOriginY() != new_map->info.origin.position.y ||
-   !layered_costmap_->isSizeLocked()))
+   master->getOriginY() != new_map->info.origin.position.y))

A quick look at git-blame view from within GitHub shows that these lines were last modified with PR #324 which was @mikeferguson & @reinzor

@ghost
Copy link
Author

ghost commented Jul 26, 2018

Thanks for the suggestions! Will update and reopen with corrections @moriarty

@ghost ghost closed this as completed Jul 26, 2018
@moriarty
Copy link
Contributor

@jinder1s I didn’t mean be rude or scare you away, I just wanted to help pin point the issue you were trying to report

@ghost
Copy link
Author

ghost commented Jul 27, 2018

@moriarty no no, I genuinely appreciate the advice. Its just the confluence of many events yesterday did not allow me to edit this issue request till now.

@ghost ghost reopened this Jul 27, 2018
@mikeferguson
Copy link
Contributor

@jinder1s that does seem like an error, and you appear to have the correct fix -- please open a PR

@DLu
Copy link
Member

DLu commented Jun 18, 2019

Closed via #792

@DLu DLu closed this as completed Jun 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants