search

ros2-dotnet / ros2_dotnet

.NET bindings for ROS2
Apache License 2.0
139 stars 57 forks source link

Substantial Number of additional ROS2 Features Implemented on My Fork #123

Open Chootin opened 11 months ago

Chootin commented 11 months ago

As mentioned in a comment in my now merged pull request #122, I have implemented the following ROS2 features (as of writing) on my fork:

These features have all been tested on two machines which are both running Ubuntu 20.04 and ROS2 Foxy. I have also been using them as part of the development of a Unity engine simulation toolkit here at QUT.

My Chootin/ros2_dotnet/dev branch (which contains all these features) is currently up-to-date with ros2_dotnet/main and can be automatically merged, so there should be two ways to contribute the code to the main repository.

  1. Make a single pull-request with all the features (this is preferable as there are fixes which are not in-order as I've found issues with my original implementations)
  2. Do some cherry picking attempting to isolate features to create sequential pull requests to review one at a time

Any thoughts @hoffmann-stefan or anyone else currently managing this repository?

Cheers, Alec.

hoffmann-stefan commented 11 months ago

Sorry, was a busy week. Hope I get the time to look at your branch next week.

35 changed files with 2427 additions and 277 deletions seems a lot for a singe PR. I will look at the commits myself and try to do some interactive rebase and cherry-picking if you don't mind. Maybe there is some way to at least separate some of that branch out to it's own PR.

Chootin commented 11 months ago

I have a lot of the work already split out into branches, it might be easier if I try to put some of the later fixes into those. I'll have a go on my end. Feel free to have a go at your end as well and see if we can't find a solution that works well.

Chootin commented 11 months ago

@hoffmann-stefan What do you think of Chootin/ros2_dotnet/upstream_pr1?

It may still be too much; however this branch contains just the features relating to clock, timer, and the related fixes which I think should come first.

Possibly what makes this look a little bigger than it is, is that I added RegisterNativeFunction to DllLoadUtils as I grew tired of making mistakes copy-pasting boilerplate code as I was developing this.

hoffmann-stefan commented 11 months ago

@hoffmann-stefan What do you think of Chootin/ros2_dotnet/upstream_pr1?

@Chootin Jup this branch looks good for the next PR, Thanks :) Could you open it?

Chootin commented 11 months ago

PR is now available #124