ros2-dotnet / ros2_dotnet

.NET bindings for ROS2
Apache License 2.0
136 stars 54 forks source link

Reconsider property naming convention in generated messages #91

Closed hoffmann-stefan closed 1 year ago

hoffmann-stefan commented 2 years ago

While programming using this library I regulary stumble upon the generated names of properties in the message types. For example in the header message the field frame_id gets converted to Header.Frame_id.

Frame_id is neither left nor right:

Posible alternatives to choose:

My favorite would be CamelCase, the next would be don't change the name at all and use what is defined in the *.msg file.

Maybe the namespaces of the generated messages should be prefixed and changed to another naming convention as well?

Are there other alternatives or things to consider?

Related to this would be the names of the request/response classes of services (SetBool_Request/SetBool_Response) and the general naming convention of the public namespaces/types/members in this library (RCLDotnet vs. RclDotnet), but i think this should be seperate issues.

esteve commented 2 years ago

@hoffmann-stefan that's a great idea! I prefer 2 as well, PascalCase seems the most idiomatic for C# Would you be interested in submitting a PR for this? Thanks.

hoffmann-stefan commented 2 years ago

@esteve Yes, I would like to submit a PR for this. Altough this should be done after the other larger open PRs are merged, as this would result in somewhat huge merge conflicts if done before those are merged.

shschaefer commented 2 years ago

I would suggest option #1 as it is consistent with message identifiers across all platforms. It is important that the C# language interfaces are idiomatic, but will cause debugging inconsistencies at the messaging layer when this convention is changed.

hoffmann-stefan commented 2 years ago

Seems like ros2_java is using CamelCase as well: https://github.com/ros2-java/ros2_java/blob/434e6f55253bfe2cb9ce34799fe548bbf4998d0e/rosidl_generator_java/rosidl_generator_java/__init__.py#L28-L30 Opend a PR for this: #93