Skip to content

Add starting_up_state parameter to Updater #354

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

Open
wants to merge 2 commits into
base: ros2
Choose a base branch
from

Conversation

redvinaa
Copy link

In the current implementation, when the Updater object is constructed, it sends off an "OK" signal. This could be problematic if we consider "Everything is running as expected" (definition of "OK" status as per REP107), that running is already initialized.

The default behavior doesn't change with this PR, so it wouldn't break anything.
Also, in the python implementation, None can be passed to skip "Node starting up" status publishing altogether.

@ct2034 ct2034 self-assigned this Jun 27, 2024
@ct2034 ct2034 added bug This is a bug in the code (and not a new feature) ros2 PR tackling a ROS2 branch labels Jun 27, 2024
@redvinaa
Copy link
Author

Hi! Can I get an update on this one?

If it wasn't merged because of this test case failing, can I get some help understanding the error? I'm sure a few minutes of someone who already understands these template-based tests can save me hours of trial and error.
After that I can also rebase to fix the conflict.

@ct2034
Copy link
Collaborator

ct2034 commented Feb 10, 2025

Hi @redvinaa.
Could we please have a quick discussion about this? I am not sure if it is worth the extra overhead. Yes,

"Everything is running as expected" (definition of "OK" status as per REP107)

On the other hand, I also would not like my system starting in an error state. Your issue could also be solved by additional state, such as proposed in ros2/common_interfaces#268. What is your opinion about that?

@ct2034 ct2034 added enhancement This tackles a new feature of the code (and not a bug) and removed bug This is a bug in the code (and not a new feature) labels May 26, 2025
@redvinaa
Copy link
Author

redvinaa commented Jun 4, 2025

Hi! Sorry for the long delay. It's possible that it would make sense to add new states, but that's a huge change and it appears to be moving very slowly. This PR on the other hand is not breaking, and I think is still useful until the new states get added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This tackles a new feature of the code (and not a bug) ros2 PR tackling a ROS2 branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants