search

clearpathrobotics / robot_upstart

ROS package of helper functions related to bringing up roslaunch on system startup.
BSD 3-Clause "New" or "Revised" License
192 stars 94 forks source link

Selectable ROS_HOSTNAME? #61

Open 130s opened 6 years ago

130s commented 6 years ago

ROS_HOSTNAME is set automatically when service starts. This can cause communication issue between clients when they can't resolve the robot's pc with the value in ROS_HOSTNAME.

Does it make sense to add an option to not explicitly set ROS_HOSTNAME (implying I might open a PR)?

Environment variables regarding ROS networking are tricky to me (IMO they are for many users...) so I might be wrong though.

130s commented 6 years ago

I opened a kind of related suggestion https://github.com/ros/ros_comm/issues/1251. Maybe depending on the progression of that ticket, we may shift to ROS_HOSTNAME and could drop official support of ROS_IP. I'm not sure about this yet.

130s commented 6 years ago

@mikepurvis I can try opening a PR, but first I'd like you or anyone to confirm my concern is valid.

Timple commented 6 years ago

I would like to have this option as well!

But since it is a environment variable, I get the feeling creating a PR to set any ROS variable would make sense.

130s commented 6 years ago

My team is in need for this improvement anyways, so I'll start work on this.

since it is a environment variable, I get the feeling creating a PR to set any ROS variable would make sense.

I'm not sure what you mean. Do you mean a feature to be able to selectively pass all ROS environment variables?

Timple commented 6 years ago

Exactly, maybe somebody has the need to pass ROS_MASTER_URI or something else as well

mikepurvis commented 6 years ago

For research products like Husky, we were never able to assume functional DNS in customers' networks, so it's always just used the ROS_IP branch of that template.

I'm fine to have some scheme for overriding the ROS_HOSTNAME that gets set, possibly similar to how the master_uri check works a line or two lower.

Another option would be to make it only set ROS_HOSTNAME there if it isn't already set, which would allow it to be overridden in a global /etc/ros/setup.bash file, as suggested in previous discussions, such as https://github.com/clearpathrobotics/robot_upstart/pull/44#issuecomment-261583868.

magnus-rovco commented 4 years ago

I've been running into an issue where I needed the ROS_HOSTNAME to be the HOSTNAME appended with .local suffix. So adding the option to specify the ROS_HOSTNAME seemed like a good solution.

LarsJanssenTUe commented 2 years ago

I ran into exactly the same issue as @magnus-rovco, so for now I hacked this together using some local changes, if this is the direction the maintainers want to go in I can clean the code up and open a PR.