-
Notifications
You must be signed in to change notification settings - Fork 44
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
remove the padding member from structs which should be empty #73
Conversation
Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
I think this spot at line 379 in rosidl_python/rosidl_generator_py/resource/_msg.py.em Lines 372 to 382 in 000cf6b
|
Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
Very good point. Fixed in d97cc46. I am curious if the running CI builds will complain about it 🤔 Update: and the tests did catch this 😥 http://build.ros2.org/view/Eci/job/Eci__overlay_ubuntu_bionic_amd64/23/testReport/junit/rosidl_runtime_py.test.rosidl_runtime_py/test_set_message/test_set_message_fields_none/ |
* remove the padding member from structs which should be empty Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com> * update __eq__ logic Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
IDL and some programming languages like C require a struct to have at least one member. Python though doesn't have that restriction and can define a class without any slots.
Therefore this patch removes the member
structure_needs_at_least_one_member
only used to "pad" empty structures from the Python data type. As a consequence printing the message won't show the member as well as the user won't see a getter and setter for the member anymore.In the conversion from Python to C the member variable present in C is just set to
0
in order to not be uninitialized. In the other direction the value in the C struct is just ignored and not transferred to the Python object.Fixes ros2/ros2cli#297. Requires ros2/rosidl#389.