Added teleop_legged_robots for noetic

[tarasborov]

Added teleop_legged_robots for noetic

[clalancette]

There are a few problems with the source package that I saw.

First, I don't think that this package is going to actually work on Noetic. In particular, https://github.com/SoftServeSAG/teleop_legged_robots/blob/66efb1b9a751db2283402274de796bf01f215840/nodes/teleop_legged_keyboard.py#L1 doesn't work; that needs to be /usr/bin/env python3 to work properly.

Second, this package uses tf, which is long deprecated; it really should be using tf2.

Third, https://github.com/SoftServeSAG/teleop_legged_robots/blob/main/src/teleop_legged_robots/__init__.py isn't actually used as far as I can tell; I would remove that.

Only the first issue is blocking, but it would be good to fix the other two issues as well.

Also, why did you not use bloom to open these PRs?

[mikeferguson]

@clalancette it looks like the package uses catkin_install_programs (https://github.com/SoftServeSAG/teleop_legged_robots/blob/66efb1b9a751db2283402274de796bf01f215840/CMakeLists.txt#L12) which, by my understanding actually rewrites the shebangs to python3 when that is the environment, right? (I've been using that elsewhere).

[sloretz]

that needs to be /usr/bin/env python3 to work properly.

@clalancette it looks like the package uses catkin_install_programs (https://github.com/SoftServeSAG/teleop_legged_robots/blob/66efb1b9a751db2283402274de796bf01f215840/CMakeLists.txt#L12)

@mikeferguson is correct, catkin_install_python() plus a shebang that says #!/usr/bin/ev python is correct.

[clalancette]

Thanks for the heads up about the shebang rewriting. I couldn't remember if that was part of what we ended up deploying to Noetic; I guess it is. In that case, the other questions are still outstanding (though they aren't really blocking, it would still be nice to clean them up).

[mikeferguson]

Second, this package uses tf, which is long deprecated; it really should be using tf2.

When did this become a requirement for listing a package in rosdistro? Like, if that's a requirement, we probably gotta delist 25-50% of the things in rosdistro (for example, http://wiki.ros.org/tf shows 290 melodic packages and 66 noetic packages that depend directly on tf. That doesn't even count the transitive dependencies - for instance pcl_ros has an additional 20 packages that depend on it in noetic).

[tarasborov]

Thanks for the comments. I have committed the updated code to the GitHub

1. I double checked how the code works in all three distributives. Everything runs fine, so I guess that shebangs did the trick, as @mikeferguson and @sloretz pointed out. Therefore, I did not do any changes to the code.

2. Initially I expected that tf should be enough, but I got curious how it will work with tf2 and updated the code.

3. Great catch @clalancette I simplified the package structure

4. I had strange issues with bloom while entering git credentials to create a PR, thus, I created it manually. Perhaps SSH authentication would help, but I didn’t try it.

[sloretz]

holding for the upcoming Noetic sync

[ivanpauno]

The comments seem to have been addressed, and based on a previous comment this was already good to go, so merging!