added repitition to robot_description reading #53
Conversation
| @@ -62,6 +62,12 @@ class JointStateListener { | |||
| /// Destructor | |||
| ~JointStateListener(); | |||
|
|
|||
| // Number of tries for reading robot_description from parameter server | |||
| static const size_t description_read_repitions_; | |||
There was a problem hiding this comment.
Change to description_read_repititions_?
|
I think that the best implementation of this feature would be to add description_read_repetitions and description_read_delay as ROS parameters with default values if the parameters are not set on the server. |
|
@jacquelinekay Makes sense. I changed it accordingly. I am not sure about default values though. |
|
I would consider making the default value for @ros/ros_team and @ros/robot_model, do you have any feedback on this new workaround for nondeterministic startup behavior? |
|
@jacquelinekay changed the default repetitions to 0. |
|
Hi @jacquelinekay just checking if this PR can be merged or there is some fundamental disagreement on the content. If it's going to be merged we can resolve the conflicts. |
…tween them are controlled by parameters. It defaults to the current behaviour.
toliver
force-pushed
the
F#52_double_check_for_robot_description
branch
from
672158a
to
2a0d5b2
Compare
|
This has been rebased and the checks are passing. Do you think it can be merged? |
|
Hi @toliver, thanks for the contribution. If I understand correctly this enables robot_state_publisher to wait for the parameter "robot_description" to be set for a fixed amount of time. Would you mind adding a test to make sure it does not wait by default, and a test to make sure it can wait when the two parameters are set? |
| @@ -62,6 +62,12 @@ class JointStateListener { | |||
| /// Destructor | |||
| ~JointStateListener(); | |||
|
|
|||
| // Default value for number of attempts to read robot_description from parameter server | |||
| static const int default_description_read_repetitions_; | |||
There was a problem hiding this comment.
I think adding these two constants to the JointStateListener class breaks ABI, which would block merging into kinetic-devel. Since these are only used in main(), how about they be local variables there?
| JointStateListener::default_description_read_delay_ << " seconds will be used."); | ||
| description_read_delay = JointStateListener::default_description_read_delay_; | ||
| } | ||
| for (size_t i = 0; i < description_read_repetitions; ++i) { |
There was a problem hiding this comment.
Please add a test for this code.
|
@clalancette Thoughts? |
|
Sorry to nose in, are there plans to move ahead with this pull request? It would be useful for my application as well. I can help with the requested changes if need be. |
|
I'm wondering if there is a nicer way to do this ,rather than just polling for a set amount of time. That is, it seems to me like it might be better for robot_state_publisher to have a configurable topic that would get published to once the robot description is available. That way, it would just wait around until that event happened, rather than polling the parameter server. Thoughts about an approach like that? |
|
This PR is targeting an EOL ROS distro, closing this. Feel free to reopen this PR in an active ROS distro if the PR is relevant |
fixes #52