search

RobotecAI / ros2cs

A C# (.Net) implementation of ros2 client library (rcl), enabling communication between ros2 ecosystem and C#/.Net applications such as Unity3D
Apache License 2.0
91 stars 22 forks source link

further work on service PR #36

Closed Deric-W closed 1 year ago

Deric-W commented 1 year ago

Hello, this PR is based on #30 created by @miyakoshi-dev and adds improvements to the Client implementation. I wanted to contribute the changes to his fork here (contains a summary) but since he told me that he did not have enough time to handle it I created this PR.

pijaro commented 1 year ago

@Deric-W Nice work! I will look into it as soon as possible :+1:

pijaro commented 1 year ago

I like this MR, let's just resolve this wait set minor issue and I think we will be able to merge.

I will also do a quick Windows test, and @adamdbrw will take a final look at the code :+1:

pijaro commented 1 year ago

Did some more tests:

Did you try it on Ubuntu 22.04 with humble? Currently, we are using mono for 22.04, and it looks like there are some issues with building the Client.cs:

.../ros2cs_core/Client.cs(59,23): error CS1519: Unexpected symbol `<' in class, struct, or interface member declaration
.../ros2cs_core/Client.cs(59,28): error CS1519: Unexpected symbol `,' in class, struct, or interface member declaration

and so on. It looks like there are some clashes with mono :thinking:

Deric-W commented 1 year ago

I ran tests only on Windows since I dont have any Linux machines right now, I try to change that. The syntax errors seemed to be caused by tuple literals in generic type signatures, but running mcs now causes:

Mono.CSharp.InternalErrorException: src/ros2cs/ros2cs_core/Client.cs(24,11): ROS2 ---> Mono.CSharp.InternalErrorException: src/ros2cs/ros2cs_core/Client.cs(30,16): ROS2.Client<I,O> ---> System.ArgumentException: An item with the same key has already been added. Key: PendingTasksView

Out of curiosity: Why is mono being used on Ubuntu 22.04?

pijaro commented 1 year ago

We had incompatibility issues with Unity, Ubuntu 22.04 (first releases), and r2cs on .net sdk6.

However, I did a quick test with netstandard2.0 and it looks like it works now :thinking:.

Therefore, I think you can revert 366f16a, and I will migrate u22.04 to dotnet.

pijaro commented 1 year ago

@Deric-W :medal_sports: @miyakoshi-dev :medal_sports: Thanks again for your great contribution.

I adjusted the framework versions, and it should be working on dotnet on all OSes.