Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Chootin opened this issue Oct 17, 2023 · 5 comments
Open

Comments

@Chootin
Copy link
Contributor

Chootin commented Oct 17, 2023

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:

  • Exposed clock and time messages for users
  • Parameter services and handling
  • use_sim_time parameter and time handling
  • Timers and the time jump callback
  • Fetching of native default QoS Profiles
  • Overriding parameters for ros2 launch and ros2 run

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
Copy link
Member

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
Copy link
Contributor Author

Chootin commented Oct 22, 2023

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
Copy link
Contributor Author

Chootin commented Oct 23, 2023

@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
Copy link
Member

@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
Copy link
Contributor Author

Chootin commented Oct 23, 2023

PR is now available #124

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants