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

Fixing warehouse services #474

Merged
merged 2 commits into from Mar 22, 2017

Conversation

beatrizleon
Copy link

@beatrizleon beatrizleon commented Mar 21, 2017

Description

When launching the warehouse services, I got an error connecting to the warehouse which was due to not having the appropriate try/catch and also missing the conn->connect() call. With this pr, services are advertised correctly.

Checklist

  • Required: Code is auto formatted using clang-format
  • I tried on Kinetic but I think it should be cherry-picked to other current ROS branches (Indigo, Jade)

Thank you!

ROS_INFO("Connecting to warehouse on %s:%d", host.c_str(), port);
if (!conn->connect())
{
ROS_ERROR("Failed to connect to DB");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be nice to print the host and port on this error message. I'm surprised it ever worked without the call to connect()

@beatrizleon
Copy link
Author

@jrgnicho thanks for reviewing it. I added info to the comment.

@v4hn v4hn merged commit 601beb6 into moveit:kinetic-devel Mar 22, 2017
@v4hn
Copy link
Contributor

v4hn commented Mar 22, 2017

Thanks for the contribution @beatrizleon !
The current state of the warehouse is not too nice I suppose..

@beatrizleon
Copy link
Author

@v4hn thanks for the merge! Do you have an estimate on when these changes will be released to the deb package?

@beatrizleon beatrizleon deleted the F_warehouse_services branch March 22, 2017 12:31
@v4hn
Copy link
Contributor

v4hn commented Mar 23, 2017 via email

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.

None yet

3 participants