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

fix the bug (similar with #3972) in humble fork #3999

Closed
wants to merge 2 commits into from
Closed

fix the bug (similar with #3972) in humble fork #3999

wants to merge 2 commits into from

Conversation

GoesM
Copy link
Contributor

@GoesM GoesM commented Dec 4, 2023


Basic Info

Info Please fill out this column
Ticket(s) this addresses #3958 #3972
Primary OS tested on Ubuntu22.04
Robotic platform tested on turtlebot
Does this PR contain AI generated software? No

Description of contribution in a few bullet points

it seems that bug fix work in #3972 has not been pulled into fork nav2-humble.

Here's our modified humble-fork born from our previous work process.

Perhaps these ready-made commits would do help for you ^_^

Copy link
Contributor

mergify bot commented Dec 4, 2023

@GoesM, all pull requests must be targeted towards the main development branch.
Once merged into main, it is possible to backport to @humble, but it must be in main
to have these changes reflected into new distributions.

Copy link
Member

@SteveMacenski SteveMacenski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please directly cherry-pick the other PR, do not just modify a few lines

* Costmap2DROS may be launched by another Lifecycle Node as a composed module
* If composed, its parents will handle the shutdown, which includes this module
*/
void on_rcl_preshutdown() override
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You did not modify the constructor for setting this to false.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean we should modify the constructor in humble to look like in main branch?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm saying that you should directly git cherry-pick the main commit to the humble branch and push that to this PR 😄 That way they're exactly the same. In fact, it might be wise to close this PR in general and reopen one with only the cherry picked commit.

Otherwise, if you wait like ~3 weeks, I'll be running a Humble sync and I'll get to this myself 😄

@GoesM GoesM closed this Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants