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

Create subscriber on constructor #4859

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Tacha-S
Copy link

@Tacha-S Tacha-S commented Jan 16, 2025


Basic Info

Info Please fill out this column
Ticket(s) this addresses #4802
Primary OS tested on Ubuntu
Robotic platform tested on gazebo simulation of Tally
Does this PR contain AI generated software? No

Description of contribution in a few bullet points

  • I modified it to create a subscriber during initialization while maintaining the functionality to switch topics based on the input port status.

Description of documentation updates required from your changes

  • There are no changes to the inputs, outputs, parameters, or overall behavior, so there is no need to update the documentation.

Description of how this change was tested

  • Create a tree with IsBatteryLow as the top-level condition and invoke the tree when the battery level is low.

Future work that may be required in bullet points

For Maintainers:

  • Check that any new parameters added are updated in docs.nav2.org
  • Check that any significant change is added to the migration guide
  • Check that any new features OR changes to existing behaviors are reflected in the tuning guide
  • Check that any new functions have Doxygen added
  • Check that any new features have test coverage
  • Check that any new plugins is added to the plugins page
  • If BT Node, Additionally: add to BT's XML index of nodes for groot, BT package's readme table, and BT library lists

@Tacha-S Tacha-S force-pushed the feature/create-subscriber-on-init branch from 229c5a1 to a7475eb Compare January 16, 2025 01:54
Signed-off-by: Tatsuro Sakaguchi <[email protected]>
@Tacha-S Tacha-S force-pushed the feature/create-subscriber-on-init branch from a7475eb to 7eb4915 Compare January 17, 2025 01:15
Copy link
Contributor

mergify bot commented Jan 17, 2025

@Tacha-S, your PR has failed to build. Please check CI outputs and resolve issues.
You may need to rebase or pull in main due to API changes (or your contribution genuinely fails).

Signed-off-by: Tatsuro Sakaguchi <[email protected]>
@Tacha-S Tacha-S force-pushed the feature/create-subscriber-on-init branch from 7eb4915 to c65bdfe Compare January 17, 2025 01:20
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.

I approve of these changes, but I think this should be made for all BT nodes so that this behavior is consistent across the board. Plus, I'm sure you'll use other BT nodes and want this done for them anyway, so may as well save yourself the headache and batch it out now :-)

@Tacha-S
Copy link
Author

Tacha-S commented Jan 21, 2025

Understood. I will check and fix the other BT nodes as well.

@Tacha-S Tacha-S marked this pull request as draft January 21, 2025 23:46
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.

3 participants