-
Notifications
You must be signed in to change notification settings - Fork 914
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
[windows] Workaround for Python 2 xmlrpc performance issues. #1872
Conversation
@mikepurvis This is ready for review and merge. Thanks! |
set ROS_MASTER_URI=http://127.0.0.1:11311 | ||
) | ||
|
||
if "%ROS_IP%" == "" ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't take into account if ROS_HOSTNAME
is set. And it is discourage to set both - ROS_IP
and ROS_HOSTNAME
at the same time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dirk-thomas Thanks. I edited the check to make sure ROS_IP
is only set when both don't exist in the environments.
Thanks for the patch. |
This PR ros#1872 made windows act differently than linux because of a specific python 2 delay doing unnecessary DNS checks for localhost. The underlying code in python is different for python3 so the hack is no longer necessary. Given that, its better to reunify the codebase
This PR #1872 made windows act differently than linux because of a specific python 2 delay doing unnecessary DNS checks for localhost. The underlying code in python is different for python3 so the hack is no longer necessary. Given that, its better to reunify the codebase
There is a ROS performance slowness issue that we root caused to an existing issue for Python 2. One discussion can be found here: https://stackoverflow.com/questions/2617615/slow-python-http-server-on-localhost
Instead of asking Windows developers to manually override the
ROS_MASTER_URI
andROS_IP
to workaround it, I am proposing that, by default, make them to be IP address directly.This fixes ms-iot/ROSOnWindows#6.