search

ros-geographic-info / geographic_info

ROS packages for geographic information
http://ros.org/wiki/geographic_info
66 stars 62 forks source link

ROS2 port of geographic_msgs and geographic_info for robot_localization #28

Closed SteveMacenski closed 5 years ago

SteveMacenski commented 5 years ago

Also includes a vague port of geodesy, which I just did the minimal porting of code, but the dependency unique_id hasn't been ported in ROS2 with the unique_identifier_msgs port from uuid_msgs

Its a full port of msgs and info with the code changes for geodesy but I don't have a concrete case to fully port that over with the missing unique_id package

SteveMacenski commented 5 years ago

Pinging back @jack-oquin @bmagyar. This is a blocker on getting nav_sat_transform up and running in ROS2. They're just a few message and service files.

IanTheEngineer commented 5 years ago

Looks like this may supersede https://github.com/ros-geographic-info/geographic_info/pull/26 . @bmagyar, are you the only active maintainer of this package?

SteveMacenski commented 5 years ago

I'm also motivated to get this in, so knit pick away, but this is blocking alot of development across the board so I'd like whatever issues there are to be brought up so they can be squashed. I don't think waiting is a course of action anymore since its blocking r_l ports to LTS ROS2

SteveMacenski commented 5 years ago

Also @bmagyar @IanTheEngineer I'm more than happy to maintain this and help out with the ROS2 items across the board in the ros-geographic-info organization if you add me to the org/let me know if/when there are meetings. My new job is 100% open source and I can easily make a business case for keeping this all up to date in ROS2

IanTheEngineer commented 5 years ago

@SteveMacenski if you were a maintainer, would you be willing to keep the ROS1 master branch in sync with a ROS2 dashing branch? This repo sees relatively little activity in general, so I doubt it would be much additional work.

@mikepurvis & @bmagyar, do either of you have admin privileges to add maintainers to @ros-geographic-info? @SteveMacenski seems like a good candidate for helping with maintainership.

SteveMacenski commented 5 years ago

@IanTheEngineer for msgs? Yeah sure, that's a no-biggy. The headers there's been no PRs for what looks like 5 years so I'm pretty sure that's also very low effort ;-)

SteveMacenski commented 5 years ago

@IanTheEngineer @bmagyar can we move forward with this?

I've posted on discourse as well to take over maintainership https://discourse.ros.org/t/claiming-maintainership-for-geographic-info-in-ros2/10620

IanTheEngineer commented 5 years ago

@SteveMacenski, I don't have any membership/ownership in this org, but I think you've gone through the right channels by posting here and on Discourse. It would be great if @mikepurvis, @jack-oquin, or @bmagyar could add you to the org and add you as an additional maintainer here, rather than another fork

SteveMacenski commented 5 years ago

Does anyone object to anything here?

IanTheEngineer commented 5 years ago

@SteveMacenski this PR should be re-targeted at a ROS2 branch rather than directly into the ROS1 master.

SteveMacenski commented 5 years ago

@IanTheEngineer I updated it, can you take a look see and approve so I'm not going cowboy :cowboy_hat_face:

After merge I'll push a release for dashing

SteveMacenski commented 5 years ago

@IanTheEngineer @tfoote @clalancette Any of you want to take a look and give me the OK to merge? Doesn't seem like I'm going to get anyone else to engage on this and I'd like to have a second pair of eyes give me a "looks good"

SteveMacenski commented 5 years ago

OK I resolved the issues and also fully ported over geodesy with the unique_id no longer required. Merging and releasing for dashing.