ros2_java icon indicating copy to clipboard operation
ros2_java copied to clipboard

Sync with work done in fork

Open ivanpauno opened this issue 5 years ago • 9 comments

There has been some work in https://github.com/osrf/ros2_java to add support for extra ros2 features:

  • Improved parameters API
  • NodeOptions
  • Improvements in Time (see https://github.com/ros2-java/ros2_java/issues/122)
  • Support for rcl events (in progress)
  • Update QoS profile (todo)
  • Support for actions (todo)

I know there are already a bunch of PRs opened to add to support for newer ros2 versions (dashing, eloquent, foxy), to update documentation, and to fix android support.

When that's done, I think we can start contributing this work upstream.

I'm opening this so the work that has been done in the fork is visible from maintainers here, and it's not lost in the void. P.S.: I'm not quite sure what's blocking the opened PRs. If I can somehow help, please let me know.

ivanpauno avatar Aug 21 '20 13:08 ivanpauno

If I can somehow help, please let me know.

@ivanpauno that's awesome, thanks for offer!

I wasn't aware of fork, thanks for letting me know. @jacobperron is a committer here as well, so he can review any contributions you submit to rcljava. Those new features are much needed, it'd be great if they could be integrated into upstream.

Right now, my priority is getting the dashing branch merged into master, but in order to do that, I'd prefer not to break any of the users' workflow and continue supporting all the existing platforms (both JDK and Android).

So far I've managed to fix the Android issues in https://github.com/ros2-java/ros2_java/pull/125, but one of the tests fails with JDK https://github.com/ros2-java/ros2_java/pull/125/checks?check_run_id=975516440

Thanks again for helping out!

esteve avatar Aug 24 '20 10:08 esteve

@esteve

FWIW, I'll be reviewing the work done in the fork. I'll give it another once over when we start opening PRs here, but we can wait for your sign-off if you prefer.

jacobperron avatar Aug 24 '20 20:08 jacobperron

@jacobperron I think I'm missing some context here. So does this mean you'll stop contributing to ros2_java and all the work will be done in the OSRF fork? That'd be unfortunate, but I guess OSRF has its own agenda. I think your contributions were very valuable and you had become co-maintainer of ros2_java, so if you want to contribute to ros2_java again in the future, your contributions will be more than welcome :-)

esteve avatar Aug 25 '20 13:08 esteve

Not at all! We plan to upstream all of our patches here.

We're just pushing forward on our fork so we can use the changes for one of our projects. You can expect a truck-load of pull requests once #117 is merged :wink:

jacobperron avatar Aug 25 '20 18:08 jacobperron

@esteve any updates on #117 because that review looks like the blocker from getting the changes from the fork back into this repo.

moriarty avatar Sep 29 '20 18:09 moriarty

@moriarty the Gradle plugins don't seem to work with the latest version of colcon, so I'll skip those for the sake of getting #117 merged. My priority was to ensure that all the platforms that are supported in master would continue to be supported after #117 was merged (i.e. JRE and Android). However, @jacobperron has write access to this repo, so any changes in the fork can be merged to the dashing branch here now, there's no need to wait for dashing being merged to master, I don't see how #117 is blocking patches being merged into the dashing branch here, to be honest.

esteve avatar Oct 06 '20 08:10 esteve

Much of the work we've done in the fork is only compatible with Foxy and/or Galactic. I'd like to merge those changes to master. Alternatively, we could start new branches for foxy and galactic and start landing changes here. @esteve I'm not sure what strategy you'd like to follow for supporting multiple ROS distros. Maintaining separate branches for each distro seems reasonable to me.

jacobperron avatar Oct 06 '20 22:10 jacobperron

@jacobperron yeah, you're right about the branches. At first I thought there wouldn't be that many incompatibilities between distros, but I see now that it's rather cumbersome to have only one branch, it'd be good to eventually consolidate all that into one and detect at runtime what ROS distro is running, though. Anyway, if it's more convenient to separate branches for now, it's ok. I've sent @ivanpauno an invite to join the ros2-java org. Thank you both so much for all the work!

esteve avatar Oct 07 '20 11:10 esteve

To update here: due to resource limitations, we've decided to only support the latest ROS 2 distro (currently Galactic). The main branch is currently supporting Galactic and we will continue to migrate patches from our fork there.

jacobperron avatar Nov 13 '21 02:11 jacobperron