search

clearpathrobotics / robot_upstart

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

Melodic compatibility: rewrite getifip to make use of iproute2 #83

Closed Rayman closed 4 years ago

Rayman commented 5 years ago

ifconfig is deprecated in favor of iproute2. So I rewrote the script to parse the json output of the ip route addr show <INTERFACE> command.

This should also work for lower versions of ubuntu, but I did not test those.

Rayman commented 5 years ago

Thanks you for the review. I've addressed all comments.

Rayman commented 5 years ago

This patch only seems to work on melodic because iproute2 should support the -json option. Can you make a melodic-devel branch then I'll retarget my PR.

mikepurvis commented 5 years ago

I'm not over the moon about needing a distro branch in this repo, just for this change. Would it be possible to support both? Like, perhaps by testing for ip addr supporting the -json flag, and if not, falling back on the other ifconfig invocation?

For the purposes of the fallback, I'd be fine if you just wanted to shell out to a bash -c rather than re-implementing the pipes using a bunch of tortured subprocess stuff.

Rayman commented 5 years ago

I've done another try with this PR: https://github.com/clearpathrobotics/robot_upstart/pull/88. This keeps a single branch for all distros.