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

Update to Foxy, Support Services, QoS, Tf2 via rclcpp and Hololens #84

Closed
wants to merge 16 commits into from

Conversation

ooeygui
Copy link
Member

@ooeygui ooeygui commented Oct 16, 2021

No description provided.

Camelron and others added 13 commits October 16, 2021 09:11
* First commit

Signed-off-by: Francisco Martin Rico <fmrico@gmail.com>

* Support for collections (except string)

Signed-off-by: Ubuntu <ubuntu@ros2-eloquent.lxd>

* Support for collections (except string)

Signed-off-by: Francisco Martin Rico <fmrico@gmail.com>

* add return values

* first attempt altering Array to work with Sequence; style=poor

* update msg.h.em

* primitive sequence functionality

* Support BasicType Sequences

Co-authored-by: Francisco Martin Rico <fmrico@gmail.com>
Co-authored-by: Ubuntu <ubuntu@ros2-eloquent.lxd>
Co-authored-by: Lou Amadio <lamadio@microsoft.com>
* add return values

* Plumb QoS profiles through to C# layer

* Debug QoS plumbing

* Document profiles

* Plumb QoS profiles through to C# layer

* Debug QoS plumbing

* Document profiles

* conventions refactor

Co-authored-by: Lou Amadio <lamadio@microsoft.com>
* add return values

* (broken) in progress nested type sequences

* Add support for sequences of namespaced types

Co-authored-by: Lou Amadio <lamadio@microsoft.com>
* add return values

* (broken) in progress nested type sequences

* Add support for sequences of namespaced types

* Expose TransformListener interface from tf2 to C#

* C# portion of tf interface

* clean up interface and document

Co-authored-by: Lou Amadio <lamadio@microsoft.com>
* Adding Service support.  WIP.

* Changes compiling.  Beginning testing.

* Able to roundtrip service request and response.

* End to end client with server working.

* Cleaning up some error handling.

* Adding shutdown cleanup logic.

* Copyright notices.  Fixes for add of shutdown logic.

Co-authored-by: Stuart Schaefer <stscha@microsoft.com>
@esteve
Copy link
Member

esteve commented Oct 16, 2021

@ooeygui thanks for the PR. I had a brief look at it, but it's really difficult to review a PR this big which touches so many things at the same time and adds multiple features, could you break it down to individual PRs? Thanks.

Also, depending on rclcpp is a huge departure from the original goal of ros2-dotnet, I honestly don't see any major reason for doing so, client libraries in ROS2 are meant to be built upon rcl

@ooeygui
Copy link
Member Author

ooeygui commented Oct 16, 2021

As many of these are interconnected; I'll see what we can do to split it out.

@ooeygui
Copy link
Member Author

ooeygui commented Oct 16, 2021

I'd be happy to schedule an virtual live review if that would help get through this PR.

@esteve
Copy link
Member

esteve commented Oct 17, 2021

As many of these are interconnected; I'll see what we can do to split it out.

@ooeygui these features were added as separate PR in your fork, so it should be straightforward to replicate that onto upstream. For example, QoS was introduced in ms-iot#2, why not just resubmit that PR onto this repository? It's ok if PRs depend on each other, as long as they have a clear goal and can be discussed in isolation.

There are over 3000 additions in this PR, it's a massive diff, I'm quite sure if someone in your team sent a PR this big it wouldn't go through easily.

I'd be happy to schedule an virtual live review if that would help get through this PR.

Thanks, but let's try going the usual route of using GitHub, a live review of over 3000 additions is gonna be quite intense.

@ooeygui
Copy link
Member Author

ooeygui commented Nov 22, 2021

@esteve Please forgive my miscommunication about the license. Microsoft by default will contribute new code under MIT, but I have verified that Apache is fine as well. As I break up the original code drop in #84, I'll rewrite the license headers so that it is not mentioned. I have left PR 84 for the conversation history; however, if you want to close it to keep the noise down in the repo, that's fine too.
I will remove the rclcpp binary as part of breaking up #84.

I'm planning on addressing the CI issues independently first; I did see you had made progress on the Linux side; I'll build off that for Windows and UWP.

Thank you

moving to #84 as requested in #89

@ooeygui ooeygui marked this pull request as draft April 19, 2022 17:22
@ooeygui
Copy link
Member Author

ooeygui commented Apr 19, 2022

Converted this to a draft so it isn't accidentally pulled.

@pabloinigoblasco
Copy link

Hello.
What is the state of this today?

@hoffmann-stefan
Copy link
Member

hoffmann-stefan commented Apr 22, 2023

Closing this as it is out of sync with the main branch by merging my two PRs:

QoS is already implemented in #94 if someone want's to checkout this branch, should be split out into a seperate PR in the next weeks.

For TF2 we have a separate repository that binds to the c++ classes. TF2 should not be part of the core rcldotnet library, but rather be it's own library that builds on top of rcldotnet. Think this will get moved to ros2_dotnet organization at some time.

Thanks for your contributions non the less, even if it didn't get merged!

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

Successfully merging this pull request may close these issues.

None yet

6 participants