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

RobotState rviz previewer: First message from e.g. latching publishers is not applied to robot state correctly #596

Merged
merged 3 commits into from Aug 31, 2017
Merged

Conversation

axelschroth
Copy link
Contributor

Fixes #595

Note: I am not familiar with the problems that leaded to the delayed loading of the robot model so please review this solution especially regarding thread contexts of the methods..

axelschroth added 2 commits August 18, 2017 14:05
@jrgnicho
Copy link
Contributor

@axelschroth thanks for submitting this. Your change seems logical but before accepting I'd like to test it out. Would you happen to have a repository with a test setup?

@axelschroth
Copy link
Contributor Author

I'm afraid I don't, sorry.. The test setup I am working with is part of a big software infrastructure. You just have to use a latching publisher, publish a robot state and disable then enable the RobotState display.

Is there any repository already set up using a 6DOF robot which I could use to create a test repository?

@v4hn
Copy link
Contributor

v4hn commented Aug 21, 2017 via email

@jrgnicho
Copy link
Contributor

@axelschroth there are several moveit_config packages for some abb robots here https://github.com/ros-industrial/abb

@v4hn
Copy link
Contributor

v4hn commented Aug 27, 2017

Ok, I can't reproduce the error you face, but it's a racing condition, so that doesn't matter too much in this case.
I looked through the code and can verify the logic problem you describe in #595 .

I see no problem with changing the threading context of the problematic call to changedRobotStateTopic as you proposed.
Everything runs fine with this patch (just as before in my case...).

Tested and verified.

I believe we should backport this to indigo. I'll leave that to the merging maintainer.

@v4hn v4hn added the awaits 2nd review one maintainer approved this request label Aug 27, 2017
@jrgnicho jrgnicho merged commit dfdff08 into moveit:kinetic-devel Aug 31, 2017
jrgnicho pushed a commit to jrgnicho/moveit that referenced this pull request Aug 31, 2017
…s is not applied to robot state correctly (moveit#596)

* Subscribe to topic after loading robot model -> processing first message from e.g. latching publishers correctly

* Formatting only

* Formatting only
jrgnicho pushed a commit to jrgnicho/moveit that referenced this pull request Aug 31, 2017
…s is not applied to robot state correctly (moveit#596)

* Subscribe to topic after loading robot model -> processing first message from e.g. latching publishers correctly

* Formatting only

* Formatting only
@axelschroth axelschroth deleted the robot-state-latched-msg-fix branch September 8, 2017 08:26
jrgnicho added a commit to jrgnicho/moveit that referenced this pull request Oct 17, 2017
jrgnicho added a commit to jrgnicho/moveit that referenced this pull request Oct 17, 2017
v4hn pushed a commit that referenced this pull request Oct 18, 2017
JafarAbdi pushed a commit to JafarAbdi/moveit that referenced this pull request Mar 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaits 2nd review one maintainer approved this request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants