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

segbot needs proper find_package kdl in the cmake #21

Closed
tfoote opened this issue Dec 9, 2013 · 19 comments
Closed

segbot needs proper find_package kdl in the cmake #21

tfoote opened this issue Dec 9, 2013 · 19 comments
Assignees

Comments

@tfoote
Copy link

tfoote commented Dec 9, 2013

This same thing was fixed in robot_state_publisher here: https://github.com/ros/robot_state_publisher/pull/10/files

It's currently regressed on the buildfarm.
http://jenkins.ros.org/view/HbinP64/job/ros-hydro-segbot-bringup_binarydeb_precise_amd64/197/console

@jack-oquin
Copy link
Member

@piyushk: Please let me know if you'd like me to fix this. Otherwise, I'll leave it to you.

@piyushk
Copy link
Member

piyushk commented Dec 10, 2013

@jack-oquin Thanks!

Since ros/robot_state_publisher#2 has been pulled in and released, we no longer need segbot_state_publisher. All references to segbot_state_publisher in the launch files should be switched back to robot_state_publisher.

You should be able to run a test to check if the tf_prefix problem has indeed gone away by running:

roslaunch segbot_gazebo multiple_segbots.launch

If you can make the necessary commits, I'll also run a quick test and re-release.

@jack-oquin
Copy link
Member

The proposed solution is to back out the work-around introduced for #16 in a2262d0, now that we can use the standard robot_state_publisher.

@jack-oquin
Copy link
Member

I backed out the #16 changes. Testing multiple_segbots with the released robot_state_publisher-1.9.9 fails. I tried version 1.9.10 in ros-shadow-fixed, but that fails, too. It looks like the tf_prefix is not getting handled as expected.

There is no CHANGELOG for robot_state_publisher, but the hydro-devel commit history suggests that the required updates should be in 1.9.10.

So, I am not sure how to proceed. Either I messed up the changes, the new robot_state_publisher still does not work for us, or there is something wrong in my test environment. The obvious alternative is to keep maintaining our own segbot_state_publisher.

Given my confusion, I decided to commit my updates in a new robot_state_publisher segbot branch, rather than the standard devel branch. @piyushk: Please take a look at it, try it, and let me know what you think we should do.

@piyushk
Copy link
Member

piyushk commented Dec 11, 2013

Thanks Jack. I wasn't able to look at this today. I will look at it first thing tomorrow morning.

@piyushk
Copy link
Member

piyushk commented Dec 13, 2013

Apologies for the delay.

@jack-oquin This worked out of the box for me. I installed v1.9.10 of robot_state_publisher from source, and your updated code works fine. What was the problem you were experiencing?

Note that with multiple segbots, rviz does not display the robot model. It would have taken a bit more work to get that working, and it was not important enough. I display an odometry arrow.

@jack-oquin
Copy link
Member

Rviz displays a global status error: "fixed frame [map] does not exist". Both robots display status error for both Odometry and LaserScan messages.

@piyushk
Copy link
Member

piyushk commented Dec 14, 2013

I got that from the system deb for robot_state_publisher as well. Reported upstream: ros/robot_state_publisher#11

If you compile robot_state_publisher from source, things work correctly. I am merging this into devel. Thanks for the fix Jack!

@jack-oquin
Copy link
Member

The problem is: we can't release Ubuntu binary packages with this fix until ros/robot_state_publisher#11 is fixed, because our packages will be broken and everyone will have to build from source.

But, we need to make a new release to fix the build breakage reported here.

Shouldn't we back out that change and make our own fix for the KDL mix-up? Then we can release again when robot_state_publisher is working again.

@piyushk
Copy link
Member

piyushk commented Dec 14, 2013

Ideally, yes. However, I don't expect anyone else to be be updating code before Jan 7.

I was planning on skipping the overhead as I expect the robot_state_publisher issue to be resolved fairly soon. After your change, we only have a launch dependency on robot_state_publisher. I am fine releasing right now (the only reason being to remove buildfarm warnings), and updating the package.xml with a gte flag and re-releasing once a fix is produced upstream.

What do you think?

@jack-oquin
Copy link
Member

Releasing what we have now will resolve the build problem. That is clearly the easiest fix for us. The devel build did succeed after you merged my updated branch.

I worry that others using this package without our knowledge may experience breakage. I would hate to cause that. I suppose they will let us know if they do and we can recommend building robot_state_publisher from source as a work-around.

@jack-oquin
Copy link
Member

I am fine releasing right now (the only reason being to remove buildfarm warnings), and updating the package.xml with a gte flag and re-releasing once a fix is produced upstream.

Won't the ros/robot_state_publisher#11 fix make it work again without us needing to re-release?

@piyushk
Copy link
Member

piyushk commented Dec 14, 2013

Yes, it is not necessary. However, we should correctly specify the correct versions in the package.xml file. If somebody simply uses apt-get install ros-hydro-segbot, the new version of robot-state-publisher will not be pulled in. I suspect we don't have the correct package versions for other dependencies as well.

@jack-oquin
Copy link
Member

OK

I wonder what would happen if we released it now with a minimal <run_depend> of robot_state_publisher >= 1.9.11?

I suppose it would build but fail to install.

@tfoote
Copy link
Author

tfoote commented Dec 14, 2013

@jack-oquin I think that's the behavior you'll see.

@jack-oquin
Copy link
Member

@tfoote: Is that a good idea, or will it cause trouble on the build farm?

it might prevent people from installing a broken version before the next robot_state_publisher release.

@piyushk
Copy link
Member

piyushk commented Dec 15, 2013

@tfoote Thanks for the info. I'll wait a couple more days on the upstream issue before re-releasing.

@jack-oquin
Copy link
Member

Running a pre-release test now. Will release soon.

@jack-oquin
Copy link
Member

After some initial hiccoughs, the build farm seems happy with segbot 0.1.9. New package versions have been propagated to ros-shadow-fixed, except for segbot-gazebo-plugins, which is marked obsolete.

Since that fixes the original problem, I am closing this issue.

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

No branches or pull requests

3 participants