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

Marshaling using "out" does not work on Windows and breaks BasicType arrays #6

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

samiamlabs
Copy link
Collaborator

No description provided.

${std_msgs_ASSEMBLIES_DLL}
${test_msgs_ASSEMBLIES_DLL}
${tf2_msgs_ASSEMBLIES_DLL}
${geometry_msgs_ASSEMBLIES_DLL}
${rosidl_generator_ASSEMBLIES_DLL}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good extension

// publisher.Dispose();
// subscription.Dispose();
// }

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the status of commented tests?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests are a bit slow to run and should not be necessary if the message read/write tests are written properly. Had not decided if I should keep them all. Deleted all but one for now.

Assert.That(msg.String_value, Is.EqualTo("Turtles all the way down"));
msg.WriteNativeMessage();
msgCopy.ReadNativeMessage(msg.Handle);
Assert.That(msgCopy.String_value, Is.EqualTo("Turtles all the way down"));
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch there on not using the same messge

return ros_message->@(member.name);
@[ elif isinstance(member.type, AbstractSequence)]@
*size = ros_message->@(member.name).size;
// *size = ros_message->@(member.name).size;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is well tested I would probably remove the comments

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had not tested on Linux when I pushed this. Have done that now and the comments are sould have been removed now.

@@ -107,6 +107,9 @@ def get_builtin_dotnet_type(type_, use_primitives=True):
if type_ == 'uint8':
return 'byte' if use_primitives else 'System.Byte'

if type_ == 'byte':
return 'byte' if use_primitives else 'System.Byte'

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably missed that because I saw ROS2 byte mapped to octet

@adamdbrw
Copy link
Owner

adamdbrw commented Sep 9, 2019

Hey Sam,

Some good additions to tests and adding the byte IDL type to dictionary - well done.
I am wondering why "out" doesn't work on Windows, do you have sources that state so and explain why? Or is it purely from your experience. Can you give some background to the issue?

@samiamlabs
Copy link
Collaborator Author

samiamlabs commented Sep 30, 2019

Hey Adam,

I had the same issue with "out" not working when I moved to Windows from mostly Linux development in the last version of the generators.

The behavior on Windows 10, in this case, was that the length returned through "out" was always 0 and the main return of the function was always a null pointer if I remember correctly.

Spent some time trying to find documentation on why this does not seem to work on Windows 10 but could find any explanation.

@samiamlabs
Copy link
Collaborator Author

I finally managed to make a unitypackage that allows the same Unity project to use ROS2 both on Ubuntu 18.04 and Windows 10 without sourcing anything. (As long as you use the included scripts to start the Editor/App.)

https://github.com/DynoRobotics/UnityRos2/releases/tag/v0.0.3-beta

The unitypackage basically just contains the result of running ros2 run rcldotnet_utils init_unity_project on both Windows and Linux in the root directory of a Unity Project and a few utility scripts right now.

I also changed the name of the built Unity programs to RosApplication because I have been using it for other stuff in addition to simulation. Made a VR remote control system for Nao for example https://www.instagram.com/p/B3B0BeYo5fY/?utm_source=ig_web_copy_link

Going to try to restore my turtlebot3 with navigation2 Unity example to working order based on the current state of this project now. We have a bunch of university students that are going to try to add support for coverage planning (for lawnmowers, floor scrubbers and so on) to that navigation stack and need a simulator :)

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.

2 participants