diff --git a/rosidl_generator_py/CMakeLists.txt b/rosidl_generator_py/CMakeLists.txt index e5ff834f..60ef3d43 100644 --- a/rosidl_generator_py/CMakeLists.txt +++ b/rosidl_generator_py/CMakeLists.txt @@ -68,6 +68,12 @@ if(BUILD_TESTING) WORKING_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}/rosidl_generator_py" ) + ament_add_pytest_test(test_convert_round_trip_py "test/test_convert_round_trip.py" + APPEND_ENV "PYTHONPATH=${pythonpath}" + APPEND_LIBRARY_DIRS "${_append_library_dirs}" + WORKING_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}/rosidl_generator_py" + ) + ament_add_pytest_test(test_cli_extension test/test_cli_extension.py) ament_add_pytest_test(test_property_py test/test_property.py diff --git a/rosidl_generator_py/resource/_msg_support.c.em b/rosidl_generator_py/resource/_msg_support.c.em index 62941b29..7bb5511a 100644 --- a/rosidl_generator_py/resource/_msg_support.c.em +++ b/rosidl_generator_py/resource/_msg_support.c.em @@ -152,6 +152,9 @@ nested_header += '__functions.h' msg_typename = '__'.join(message.structure.namespaced_type.namespaced_name()) }@ @ +@{ +declared_types = set() +}@ @[for member in message.structure.members]@ @{ type_ = member.type @@ -159,14 +162,21 @@ if isinstance(type_, AbstractNestedType): type_ = type_.value_type }@ @[ if isinstance(type_, NamespacedType)]@ -@[ if type_.namespaces[0] != package_name]@ +@{ +type_key = tuple(type_.namespaced_name()) +already_declared = type_key in declared_types +declared_types.add(type_key) +}@ +@[ if not already_declared]@ +@[ if type_.namespaces[0] != package_name]@ ROSIDL_GENERATOR_C_IMPORT -@[ end if]@ +@[ end if]@ bool @('__'.join(type_.namespaces + [convert_camel_case_to_lower_case_underscore(type_.name)]))__convert_from_py(PyObject * _pymsg, void * _ros_message); -@[ if type_.namespaces[0] != package_name]@ +@[ if type_.namespaces[0] != package_name]@ ROSIDL_GENERATOR_C_IMPORT -@[ end if]@ +@[ end if]@ PyObject * @('__'.join(type_.namespaces + [convert_camel_case_to_lower_case_underscore(type_.name)]))__convert_to_py(void * raw_ros_message); +@[ end if]@ @[ end if]@ @[end for]@ @@ -179,30 +189,57 @@ ROSIDL_GENERATOR_C_EXPORT bool @('__'.join(message.structure.namespaced_type.namespaces + [convert_camel_case_to_lower_case_underscore(message.structure.namespaced_type.name)]))__convert_from_py(PyObject * _pymsg, void * _ros_message) { // check that the passed message is of the expected Python class + static PyObject * cached_expected_class = NULL; + if (cached_expected_class == NULL) { + PyObject * pymessage_module = PyImport_ImportModule("@('.'.join(message.structure.namespaced_type.namespaces)).@(module_name)"); + if (pymessage_module == NULL) { + return false; + } + cached_expected_class = PyObject_GetAttrString(pymessage_module, "@(message.structure.namespaced_type.name)"); + Py_DECREF(pymessage_module); + if (cached_expected_class == NULL) { + return false; + } + } { - PyObject * class_attr = PyObject_GetAttrString(_pymsg, "__class__"); - if (class_attr == NULL) { + int is_instance = PyObject_IsInstance(_pymsg, cached_expected_class); + if (is_instance < 0) { return false; } - PyObject * name_attr = PyObject_GetAttrString(class_attr, "__name__"); - if (name_attr == NULL) { - Py_DECREF(class_attr); + if (is_instance == 0) { + PyErr_Format( + PyExc_TypeError, "expected an instance of '@(class_module).@(namespaced_type)', got '%s'", + Py_TYPE(_pymsg)->tp_name); return false; } - PyObject * module_attr = PyObject_GetAttrString(class_attr, "__module__"); - if (module_attr == NULL) { - Py_DECREF(name_attr); - Py_DECREF(class_attr); + } + // Cache interned attribute names for faster attribute access. The private slot + // names are used rather than the public property names so that reading a field + // is a plain slot access instead of a call into the generated property getter. + // The isinstance check above guarantees that the slots exist. +@[for i, member in enumerate(message.structure.members)]@ +@[ if len(message.structure.members) == 1 and member.name == EMPTY_STRUCTURE_REQUIRED_MEMBER_NAME]@ +@[ continue]@ +@[ end if]@ + static PyObject * cached_attr_@(member.name) = NULL; +@[end for]@ + static bool cached_attrs_initialized = false; + if (!cached_attrs_initialized) { +@[for member in message.structure.members]@ +@[ if len(message.structure.members) == 1 and member.name == EMPTY_STRUCTURE_REQUIRED_MEMBER_NAME]@ +@[ continue]@ +@[ end if]@ + cached_attr_@(member.name) = PyUnicode_InternFromString("_@(member.name)"); +@[end for]@ +@[for member in message.structure.members]@ +@[ if len(message.structure.members) == 1 and member.name == EMPTY_STRUCTURE_REQUIRED_MEMBER_NAME]@ +@[ continue]@ +@[ end if]@ + if (cached_attr_@(member.name) == NULL) { return false; } - - // PyUnicode_1BYTE_DATA is just a cast - assert(strncmp("@(class_module)", (char *)PyUnicode_1BYTE_DATA(module_attr), @(len(class_module))) == 0); - assert(strncmp("@(namespaced_type)", (char *)PyUnicode_1BYTE_DATA(name_attr), @(len(namespaced_type))) == 0); - - Py_DECREF(module_attr); - Py_DECREF(name_attr); - Py_DECREF(class_attr); +@[end for]@ + cached_attrs_initialized = true; } @(msg_typename) * ros_message = _ros_message; @[for member in message.structure.members]@ @@ -216,7 +253,7 @@ if isinstance(type_, AbstractNestedType): type_ = type_.value_type }@ { // @(member.name) - PyObject * field = PyObject_GetAttrString(_pymsg, "@(member.name)"); + PyObject * field = PyObject_GetAttr(_pymsg, cached_attr_@(member.name)); if (!field) { return false; } @@ -231,12 +268,7 @@ nested_type = '__'.join(type_.namespaced_name()) return false; } @[ if isinstance(member.type, AbstractSequence)]@ - Py_ssize_t size = PySequence_Size(field); - if (-1 == size) { - Py_DECREF(seq_field); - Py_DECREF(field); - return false; - } + Py_ssize_t size = PySequence_Fast_GET_SIZE(seq_field); if (!@(nested_type)__Sequence__init(&(ros_message->@(member.name)), size)) { PyErr_SetString(PyExc_RuntimeError, "unable to create @(nested_type)__Sequence ros_message"); Py_DECREF(seq_field); @@ -345,12 +377,7 @@ nested_type = '__'.join(type_.namespaced_name()) } @[ end if]@ @[ if isinstance(member.type, AbstractSequence)]@ - Py_ssize_t size = PySequence_Size(field); - if (-1 == size) { - Py_DECREF(seq_field); - Py_DECREF(field); - return false; - } + Py_ssize_t size = PySequence_Fast_GET_SIZE(seq_field); @[ if isinstance(member.type.value_type, AbstractString)]@ if (!rosidl_runtime_c__String__Sequence__init(&(ros_message->@(member.name)), size)) { PyErr_SetString(PyExc_RuntimeError, "unable to create String__Sequence ros_message"); @@ -410,7 +437,7 @@ nested_type = '__'.join(type_.namespaced_name()) Py_DECREF(field); return false; } - rosidl_runtime_c__String__assign(&dest[i], PyBytes_AS_STRING(encoded_item)); + rosidl_runtime_c__String__assignn(&dest[i], PyBytes_AS_STRING(encoded_item), PyBytes_GET_SIZE(encoded_item)); Py_DECREF(encoded_item); @[ elif isinstance(member.type.value_type, AbstractWString)]@ assert(PyUnicode_Check(item)); @@ -498,7 +525,7 @@ nested_type = '__'.join(type_.namespaced_name()) Py_DECREF(field); return false; } - rosidl_runtime_c__String__assign(&ros_message->@(member.name), PyBytes_AS_STRING(encoded_field)); + rosidl_runtime_c__String__assignn(&ros_message->@(member.name), PyBytes_AS_STRING(encoded_field), PyBytes_GET_SIZE(encoded_field)); Py_DECREF(encoded_field); @[ elif isinstance(member.type, AbstractWString)]@ assert(PyUnicode_Check(field)); @@ -576,19 +603,157 @@ nested_type = '__'.join(type_.namespaced_name()) ROSIDL_GENERATOR_C_EXPORT PyObject * @('__'.join(message.structure.namespaced_type.namespaces + [convert_camel_case_to_lower_case_underscore(message.structure.namespaced_type.name)]))__convert_to_py(void * raw_ros_message) { - /* NOTE(esteve): Call constructor of @(message.structure.namespaced_type.name) */ + static PyObject * cached_pymessage_class = NULL; + static PyObject * cached_py_attr__check_fields = NULL; + static PyObject * cached_check_fields_default = NULL; PyObject * _pymessage = NULL; - { + if (cached_pymessage_class == NULL) { PyObject * pymessage_module = PyImport_ImportModule("@('.'.join(message.structure.namespaced_type.namespaces)).@(module_name)"); assert(pymessage_module); - PyObject * pymessage_class = PyObject_GetAttrString(pymessage_module, "@(message.structure.namespaced_type.name)"); - assert(pymessage_class); + if (pymessage_module == NULL) { + return NULL; + } + // Mirror the value __init__ would store in the _check_fields slot. The module + // global is read rather than the environment variable directly because Python + // evaluates it once at import time and it must not change afterwards. + PyObject * check_fields_setting = PyObject_GetAttrString(pymessage_module, "ros_python_check_fields"); + assert(check_fields_setting); + if (check_fields_setting == NULL) { + Py_DECREF(pymessage_module); + return NULL; + } + cached_check_fields_default = PyBool_FromLong( + PyUnicode_Check(check_fields_setting) && + PyUnicode_CompareWithASCIIString(check_fields_setting, "1") == 0); + Py_DECREF(check_fields_setting); + cached_py_attr__check_fields = PyUnicode_InternFromString("_check_fields"); + assert(cached_py_attr__check_fields); + assert(cached_check_fields_default); + if (cached_check_fields_default == NULL || cached_py_attr__check_fields == NULL) { + Py_DECREF(pymessage_module); + return NULL; + } + cached_pymessage_class = PyObject_GetAttrString(pymessage_module, "@(message.structure.namespaced_type.name)"); Py_DECREF(pymessage_module); - _pymessage = PyObject_CallObject(pymessage_class, NULL); - Py_DECREF(pymessage_class); + assert(cached_pymessage_class); + if (cached_pymessage_class == NULL) { + return NULL; + } + } + // Cache interned attribute names for faster attribute access. The private slot + // names are used rather than the public property names so that assigning a field + // is a plain slot write instead of a call into the generated property setter. + // The values below are built by this function and are correct by construction, + // so the type assertions those setters perform would be pure overhead. + // The numpy and array.array helpers used to build the special nested basic type + // fields are cached alongside. All of it is initialized before the instance is + // allocated so that no partially built message has to be cleaned up on failure, + // and the flag is set last so that a failed attempt is retried from scratch. +@[for member in message.structure.members]@ +@[ if len(message.structure.members) == 1 and member.name == EMPTY_STRUCTURE_REQUIRED_MEMBER_NAME]@ +@[ continue]@ +@[ end if]@ + static PyObject * cached_py_attr_@(member.name) = NULL; +@[ if isinstance(member.type, AbstractNestedType) and isinstance(member.type.value_type, BasicType) and member.type.value_type.typename in SPECIAL_NESTED_BASIC_TYPES]@ +@[ if isinstance(member.type, Array)]@ + static PyObject * cached_numpy_empty_@(member.name) = NULL; + static PyObject * cached_numpy_args_@(member.name) = NULL; + static PyObject * cached_numpy_kwargs_@(member.name) = NULL; +@[ elif isinstance(member.type, AbstractSequence)]@ + static PyObject * cached_array_type_@(member.name) = NULL; + static PyObject * cached_array_typecode_@(member.name) = NULL; +@[ end if]@ +@[ end if]@ +@[end for]@ + static bool cached_py_attrs_initialized = false; + if (!cached_py_attrs_initialized) { +@[for member in message.structure.members]@ +@[ if len(message.structure.members) == 1 and member.name == EMPTY_STRUCTURE_REQUIRED_MEMBER_NAME]@ +@[ continue]@ +@[ end if]@ + if (cached_py_attr_@(member.name) == NULL) { + cached_py_attr_@(member.name) = PyUnicode_InternFromString("_@(member.name)"); + assert(cached_py_attr_@(member.name)); + if (cached_py_attr_@(member.name) == NULL) { + return NULL; + } + } +@[ if isinstance(member.type, AbstractNestedType) and isinstance(member.type.value_type, BasicType) and member.type.value_type.typename in SPECIAL_NESTED_BASIC_TYPES]@ +@[ if isinstance(member.type, Array)]@ + // The numpy C API table is never initialized in this translation unit, so + // PyArray_SimpleNew is not available and numpy.empty is called instead. + if (cached_numpy_empty_@(member.name) == NULL) { + PyObject * numpy_module = PyImport_ImportModule("numpy"); + assert(numpy_module); + if (numpy_module == NULL) { + return NULL; + } + PyObject * dtype = PyObject_GetAttrString(numpy_module, "@(SPECIAL_NESTED_BASIC_TYPES[member.type.value_type.typename]['dtype'].replace('numpy.', ''))"); + PyObject * numpy_empty = PyObject_GetAttrString(numpy_module, "empty"); + Py_DECREF(numpy_module); + assert(dtype); + assert(numpy_empty); + if (dtype == NULL || numpy_empty == NULL) { + Py_XDECREF(dtype); + Py_XDECREF(numpy_empty); + return NULL; + } + PyObject * numpy_args = Py_BuildValue("(n)", (Py_ssize_t)@(member.type.size)); + PyObject * numpy_kwargs = Py_BuildValue("{s:O}", "dtype", dtype); + Py_DECREF(dtype); + assert(numpy_args); + assert(numpy_kwargs); + if (numpy_args == NULL || numpy_kwargs == NULL) { + Py_XDECREF(numpy_args); + Py_XDECREF(numpy_kwargs); + Py_DECREF(numpy_empty); + return NULL; + } + cached_numpy_args_@(member.name) = numpy_args; + cached_numpy_kwargs_@(member.name) = numpy_kwargs; + // assigned last as it guards the block + cached_numpy_empty_@(member.name) = numpy_empty; + } +@[ elif isinstance(member.type, AbstractSequence)]@ + if (cached_array_type_@(member.name) == NULL) { + PyObject * array_module = PyImport_ImportModule("array"); + assert(array_module); + if (array_module == NULL) { + return NULL; + } + PyObject * array_type = PyObject_GetAttrString(array_module, "array"); + Py_DECREF(array_module); + PyObject * array_typecode = PyUnicode_InternFromString("@(SPECIAL_NESTED_BASIC_TYPES[member.type.value_type.typename]['type_code'])"); + assert(array_type); + assert(array_typecode); + if (array_type == NULL || array_typecode == NULL) { + Py_XDECREF(array_type); + Py_XDECREF(array_typecode); + return NULL; + } + cached_array_typecode_@(member.name) = array_typecode; + // assigned last as it guards the block + cached_array_type_@(member.name) = array_type; + } +@[ end if]@ +@[ end if]@ +@[end for]@ + cached_py_attrs_initialized = true; + } + // Allocate the instance without running __init__, which would recursively build + // a default value for every field only for all of them to be overwritten below. + // The message classes define __slots__ and have no __dict__, so every slot has + // to be assigned here, including _check_fields. + { + PyTypeObject * message_type = (PyTypeObject *)cached_pymessage_class; + _pymessage = message_type->tp_alloc(message_type, 0); if (!_pymessage) { return NULL; } + if (PyObject_SetAttr(_pymessage, cached_py_attr__check_fields, cached_check_fields_default)) { + Py_DECREF(_pymessage); + return NULL; + } } @[if len(message.structure.members) == 1 and member.name == EMPTY_STRUCTURE_REQUIRED_MEMBER_NAME]@ (void)raw_ros_message; @@ -608,8 +773,14 @@ if isinstance(type_, AbstractNestedType): PyObject * field = NULL; @[ if isinstance(member.type, AbstractNestedType) and isinstance(member.type.value_type, BasicType) and member.type.value_type.typename in SPECIAL_NESTED_BASIC_TYPES]@ @[ if isinstance(member.type, Array)]@ - field = PyObject_GetAttrString(_pymessage, "@(member.name)"); + // Create the numpy array that __init__ used to provide. Since the whole array + // is overwritten below there is no need to zero it first. + field = PyObject_Call( + cached_numpy_empty_@(member.name), + cached_numpy_args_@(member.name), + cached_numpy_kwargs_@(member.name)); if (!field) { + Py_DECREF(_pymessage); return NULL; } assert(field->ob_type != NULL); @@ -622,7 +793,14 @@ if isinstance(type_, AbstractNestedType): @(SPECIAL_NESTED_BASIC_TYPES[member.type.value_type.typename]['dtype'].replace('numpy.', 'npy_')) * dst = (@(SPECIAL_NESTED_BASIC_TYPES[member.type.value_type.typename]['dtype'].replace('numpy.', 'npy_')) *)PyArray_GETPTR1(seq_field, 0); @primitive_msg_type_to_c(member.type.value_type) * src = &(ros_message->@(member.name)[0]); memcpy(dst, src, @(member.type.size) * sizeof(@primitive_msg_type_to_c(member.type.value_type))); - Py_DECREF(field); + { + int rc = PyObject_SetAttr(_pymessage, cached_py_attr_@(member.name), field); + Py_DECREF(field); + if (rc) { + Py_DECREF(_pymessage); + return NULL; + } + } @[ elif isinstance(member.type, AbstractSequence)]@ @[ if isinstance(member.type, UnboundedSequence) and member.type.value_type.typename == 'uint8']@ if (ros_message->@(member.name).is_rosidl_buffer) { @@ -650,71 +828,51 @@ if isinstance(type_, AbstractNestedType): Py_DECREF(rosidl_buffer_internal); } if (field == NULL) { + Py_DECREF(_pymessage); return NULL; } // Set the Buffer on the Python message object - if (PyObject_SetAttrString(_pymessage, "@(member.name)", field) == -1) { + if (PyObject_SetAttr(_pymessage, cached_py_attr_@(member.name), field) == -1) { Py_DECREF(field); + Py_DECREF(_pymessage); return NULL; } Py_DECREF(field); } else { @[ end if]@ @{bi = ' ' if (isinstance(member.type, UnboundedSequence) and member.type.value_type.typename == 'uint8') else ''}@ -@(bi) field = PyObject_GetAttrString(_pymessage, "@(member.name)"); -@(bi) if (!field) { -@(bi) return NULL; -@(bi) } -@(bi) assert(field->ob_type != NULL); -@(bi) assert(field->ob_type->tp_name != NULL); -@(bi) assert(strcmp(field->ob_type->tp_name, "array.array") == 0); -@(bi) // ensure that itemsize matches the sizeof of the ROS message field -@(bi) PyObject * itemsize_attr = PyObject_GetAttrString(field, "itemsize"); -@(bi) assert(itemsize_attr != NULL); -@(bi) size_t itemsize = PyLong_AsSize_t(itemsize_attr); -@(bi) Py_DECREF(itemsize_attr); -@(bi) if (itemsize != sizeof(@primitive_msg_type_to_c(member.type.value_type))) { -@(bi) PyErr_SetString(PyExc_RuntimeError, "itemsize doesn't match expectation"); -@(bi) Py_DECREF(field); -@(bi) return NULL; -@(bi) } -@(bi) // clear the array, poor approach to remove potential default values -@(bi) Py_ssize_t length = PyObject_Length(field); -@(bi) if (-1 == length) { -@(bi) Py_DECREF(field); -@(bi) return NULL; -@(bi) } -@(bi) if (length > 0) { -@(bi) PyObject * pop = PyObject_GetAttrString(field, "pop"); -@(bi) assert(pop != NULL); -@(bi) for (Py_ssize_t i = 0; i < length; ++i) { -@(bi) PyObject * ret = PyObject_CallFunctionObjArgs(pop, NULL); -@(bi) if (!ret) { -@(bi) Py_DECREF(pop); -@(bi) Py_DECREF(field); -@(bi) return NULL; -@(bi) } -@(bi) Py_DECREF(ret); +@(bi) // Create the array.array that __init__ used to provide. Building it straight +@(bi) // from the raw bytes replaces the previous sequence of a getattr, an itemsize +@(bi) // check, a pop() loop to discard the default values and a frombytes() call. +@(bi) // array.array interprets a bytes initializer as machine values, so the +@(bi) // itemsize of the typecode has to match the C type for the length to be right. +@(bi) { +@(bi) // the data pointer may be NULL for an empty sequence, which is fine here +@(bi) // because only a zero length is derived from it in that case +@(bi) PyObject * data = PyBytes_FromStringAndSize( +@(bi) (const char *)ros_message->@(member.name).data, +@(bi) ros_message->@(member.name).size * sizeof(@primitive_msg_type_to_c(member.type.value_type))); +@(bi) if (!data) { +@(bi) Py_DECREF(_pymessage); +@(bi) return NULL; @(bi) } -@(bi) Py_DECREF(pop); -@(bi) } -@(bi) if (ros_message->@(member.name).size > 0) { -@(bi) // populating the array.array using the frombytes method -@(bi) PyObject * frombytes = PyObject_GetAttrString(field, "frombytes"); -@(bi) assert(frombytes != NULL); -@(bi) @primitive_msg_type_to_c(member.type.value_type) * src = &(ros_message->@(member.name).data[0]); -@(bi) PyObject * data = PyBytes_FromStringAndSize((const char *)src, ros_message->@(member.name).size * sizeof(@primitive_msg_type_to_c(member.type.value_type))); -@(bi) assert(data != NULL); -@(bi) PyObject * ret = PyObject_CallFunctionObjArgs(frombytes, data, NULL); +@(bi) field = PyObject_CallFunctionObjArgs( +@(bi) cached_array_type_@(member.name), cached_array_typecode_@(member.name), data, NULL); @(bi) Py_DECREF(data); -@(bi) Py_DECREF(frombytes); -@(bi) if (!ret) { -@(bi) Py_DECREF(field); +@(bi) if (!field) { +@(bi) Py_DECREF(_pymessage); +@(bi) return NULL; +@(bi) } +@(bi) assert(PySequence_Size(field) == (Py_ssize_t)ros_message->@(member.name).size); +@(bi) } +@(bi) { +@(bi) int rc = PyObject_SetAttr(_pymessage, cached_py_attr_@(member.name), field); +@(bi) Py_DECREF(field); +@(bi) if (rc) { +@(bi) Py_DECREF(_pymessage); @(bi) return NULL; @(bi) } -@(bi) Py_DECREF(ret); @(bi) } -@(bi) Py_DECREF(field); @[ if isinstance(member.type, UnboundedSequence) and member.type.value_type.typename == 'uint8']@ } // end else (non-buffer path) @[ end if]@ @@ -732,6 +890,7 @@ nested_type = '__'.join(type_.namespaced_name()) @[ end if]@ field = PyList_New(size); if (!field) { + Py_DECREF(_pymessage); return NULL; } @(nested_type) * item; @@ -744,6 +903,7 @@ nested_type = '__'.join(type_.namespaced_name()) PyObject * pyitem = @('__'.join(type_.namespaces + [convert_camel_case_to_lower_case_underscore(type_.name)]))__convert_to_py(item); if (!pyitem) { Py_DECREF(field); + Py_DECREF(_pymessage); return NULL; } int rc = PyList_SetItem(field, i, pyitem); @@ -754,6 +914,7 @@ nested_type = '__'.join(type_.namespaced_name()) @[ else]@ field = @('__'.join(type_.namespaces + [convert_camel_case_to_lower_case_underscore(type_.name)]))__convert_to_py(&ros_message->@(member.name)); if (!field) { + Py_DECREF(_pymessage); return NULL; } @[ end if]@ @@ -767,6 +928,7 @@ nested_type = '__'.join(type_.namespaced_name()) @[ end if]@ field = PyList_New(size); if (!field) { + Py_DECREF(_pymessage); return NULL; } for (size_t i = 0; i < size; ++i) { @@ -779,8 +941,10 @@ nested_type = '__'.join(type_.namespaced_name()) (void)rc; assert(rc == 0); @[ elif isinstance(member.type.value_type, AbstractString)]@ - PyObject * decoded_item = PyUnicode_DecodeUTF8(src[i].data, strlen(src[i].data), "replace"); + PyObject * decoded_item = PyUnicode_DecodeUTF8(src[i].data, src[i].size, "replace"); if (!decoded_item) { + Py_DECREF(field); + Py_DECREF(_pymessage); return NULL; } int rc = PyList_SetItem(field, i, decoded_item); @@ -790,6 +954,8 @@ nested_type = '__'.join(type_.namespaced_name()) int byteorder = 0; PyObject * decoded_item = PyUnicode_DecodeUTF16((const char *)src[i].data, src[i].size * sizeof(uint16_t), NULL, &byteorder); if (!decoded_item) { + Py_DECREF(field); + Py_DECREF(_pymessage); return NULL; } int rc = PyList_SetItem(field, i, decoded_item); @@ -833,31 +999,19 @@ nested_type = '__'.join(type_.namespaced_name()) assert(PySequence_Check(field)); @[ elif isinstance(member.type, BasicType) and member.type.typename == 'char']@ field = Py_BuildValue("C", ros_message->@(member.name)); - if (!field) { - return NULL; - } @[ elif isinstance(member.type, BasicType) and member.type.typename == 'octet']@ field = PyBytes_FromStringAndSize((const char *)&ros_message->@(member.name), 1); - if (!field) { - return NULL; - } @[ elif isinstance(member.type, AbstractString)]@ field = PyUnicode_DecodeUTF8( ros_message->@(member.name).data, - strlen(ros_message->@(member.name).data), + ros_message->@(member.name).size, "replace"); - if (!field) { - return NULL; - } @[ elif isinstance(member.type, AbstractWString)]@ int byteorder = 0; field = PyUnicode_DecodeUTF16( (const char *)ros_message->@(member.name).data, ros_message->@(member.name).size * sizeof(uint16_t), NULL, &byteorder); - if (!field) { - return NULL; - } @[ elif isinstance(member.type, BasicType) and member.type.typename == 'boolean']@ @# using PyBool_FromLong allows treating the variable uniformly by calling Py_DECREF on it later field = PyBool_FromLong(ros_message->@(member.name) ? 1 : 0); @@ -882,10 +1036,15 @@ nested_type = '__'.join(type_.namespaced_name()) @[ else]@ assert(false); @[ end if]@ + if (!field) { + Py_DECREF(_pymessage); + return NULL; + } { - int rc = PyObject_SetAttrString(_pymessage, "@(member.name)", field); + int rc = PyObject_SetAttr(_pymessage, cached_py_attr_@(member.name), field); Py_DECREF(field); if (rc) { + Py_DECREF(_pymessage); return NULL; } } diff --git a/rosidl_generator_py/test/test_convert_round_trip.py b/rosidl_generator_py/test/test_convert_round_trip.py new file mode 100644 index 00000000..e5e0ec80 --- /dev/null +++ b/rosidl_generator_py/test/test_convert_round_trip.py @@ -0,0 +1,328 @@ +# Copyright 2026 Cellumation GmbH +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +""" +Round trip tests for the generated convert_from_py / convert_to_py functions. + +convert_to_py allocates the Python message with tp_alloc and writes straight to the +``_field`` slots instead of calling ``__init__`` and going through the property +setters. That makes it responsible for producing every slot, and for producing the +exact container types (``list`` / ``array.array`` / ``numpy.ndarray``) that +``__init__`` used to create. These tests exercise the full matrix of field kinds +against those two properties. + +The conversion functions are reached the same way rclpy reaches them, by unwrapping +the PyCapsules on the message metaclass. ``PYFUNCTYPE`` is required rather than +``CFUNCTYPE`` because the latter releases the GIL, which crashes immediately. +""" + +import array +import ctypes +import importlib +import os +from typing import Any, List, Optional + +import numpy +import pytest + +from rosidl_generator_py.msg import Arrays +from rosidl_generator_py.msg import BasicTypes +from rosidl_generator_py.msg import BoundedPlainSequences +from rosidl_generator_py.msg import BoundedSequences +from rosidl_generator_py.msg import BuiltinTypeSequencesIdl +from rosidl_generator_py.msg import Constants +from rosidl_generator_py.msg import Defaults +from rosidl_generator_py.msg import Empty +from rosidl_generator_py.msg import MultiNested +from rosidl_generator_py.msg import Nested +from rosidl_generator_py.msg import StringArrays +from rosidl_generator_py.msg import Strings +from rosidl_generator_py.msg import UnboundedSequences +from rosidl_generator_py.msg import WStrings + +from rosidl_parser.definition import AbstractSequence +from rosidl_parser.definition import AbstractString +from rosidl_parser.definition import AbstractWString +from rosidl_parser.definition import Array +from rosidl_parser.definition import BasicType +from rosidl_parser.definition import BoundedSequence +from rosidl_parser.definition import NamespacedType + +MESSAGE_TYPES = [ + Arrays, + BasicTypes, + BoundedPlainSequences, + BoundedSequences, + BuiltinTypeSequencesIdl, + Constants, + Defaults, + Empty, + MultiNested, + Nested, + StringArrays, + Strings, + UnboundedSequences, + WStrings, +] + +# Kept small so that it fits the tightest bound used by the test interfaces. +SEQUENCE_LENGTH = 2 + +# Basic types that are stored in an array.array when in a sequence and in a +# numpy.ndarray when in a fixed size array, mapped to the typecode and dtype the +# generated code has to produce. +# +# Spelled out here rather than imported from generate_py_impl on purpose. It makes +# the test an independent oracle for the container types instead of comparing the +# generator against itself, and the generated test interfaces shadow the generator +# package on sys.path anyway. +SPECIAL_NESTED_BASIC_TYPES = { + 'float': ('f', numpy.float32), + 'double': ('d', numpy.float64), + 'int8': ('b', numpy.int8), + 'uint8': ('B', numpy.uint8), + 'int16': ('h', numpy.int16), + 'uint16': ('H', numpy.uint16), + 'int32': ('i', numpy.int32), + 'uint32': ('I', numpy.uint32), + 'int64': ('q', numpy.int64), + 'uint64': ('Q', numpy.uint64), +} + +# What __init__ would store in the _check_fields slot. +EXPECTED_CHECK_FIELDS = os.getenv('ROS_PYTHON_CHECK_FIELDS', default='') == '1' + +_PyCapsule_GetPointer = ctypes.pythonapi.PyCapsule_GetPointer +_PyCapsule_GetPointer.restype = ctypes.c_void_p +_PyCapsule_GetPointer.argtypes = [ctypes.py_object, ctypes.c_char_p] + + +def _capsule(message_type: type, function: str) -> Any: + """ + Look up one of the conversion capsules of a message type. + + ``__import_type_support__()`` cannot be used here. It runs + ``from rosidl_generator_py import import_type_support``, but the test interfaces + of this package are generated into a Python package that is *also* called + ``rosidl_generator_py`` and shadows the generator package on ``sys.path``. The + generated helper swallows the resulting ImportError and leaves the capsules + unset, so the typesupport extension module is imported directly instead, the + same way ``rosidl_generator_py.import_type_support`` would. + """ + package, *middle, module = message_type.__module__.split('.') + suffix = '__'.join(middle + [module[1:]]) + typesupport = importlib.import_module( + '.{}_s__rosidl_typesupport_c'.format(package), package=package) + return getattr(typesupport, '{}_msg__{}'.format(function, suffix)) + + +def _round_trip(message: Any) -> Any: + """Send a message through convert_from_py and convert_to_py and back.""" + message_type = type(message) + + def pointer(name: str) -> int: + return _PyCapsule_GetPointer(_capsule(message_type, name), None) + + # PYFUNCTYPE keeps the GIL held, which the conversion functions require. + create = ctypes.PYFUNCTYPE(ctypes.c_void_p)(pointer('create_ros_message')) + destroy = ctypes.PYFUNCTYPE(None, ctypes.c_void_p)(pointer('destroy_ros_message')) + convert_from_py = ctypes.PYFUNCTYPE( + ctypes.c_bool, ctypes.py_object, ctypes.c_void_p)(pointer('convert_from_py')) + convert_to_py = ctypes.PYFUNCTYPE( + ctypes.py_object, ctypes.c_void_p)(pointer('convert_to_py')) + + ros_message = create() + assert ros_message, 'failed to allocate the ROS message' + try: + assert convert_from_py(message, ros_message), 'convert_from_py failed' + return convert_to_py(ros_message) + finally: + destroy(ros_message) + + +def _scalar_value(type_: Any, seed: int) -> Any: + """Build a value for a single element, small enough to fit every basic type.""" + if isinstance(type_, NamespacedType): + module = __import__('.'.join(type_.namespaces), fromlist=[type_.name]) + return _populate(getattr(module, type_.name)()) + if isinstance(type_, (AbstractString, AbstractWString)): + # Short enough for the bounded string fields of the test interfaces. + return 'v%d' % seed + assert isinstance(type_, BasicType), type_ + if type_.typename == 'boolean': + return bool(seed % 2) + if type_.typename == 'octet': + return bytes([seed]) + if type_.typename == 'char': + # Only reachable from a .idl file. 'char' in a .msg interface is mapped to + # uint8 before it reaches the generator, but a real IDL char is a str. + return chr(seed) + if type_.typename in ('float', 'double'): + # Exactly representable as float32, so float32 fields survive the round trip. + return seed + 0.5 + # Fits in int8, the narrowest signed integer type. + return seed + + +def _field_value(slot_type: Any, seed: int) -> Any: + """Build a field value using the same container type that __init__ would.""" + if not isinstance(slot_type, (Array, AbstractSequence)): + return _scalar_value(slot_type, seed) + + if isinstance(slot_type, Array): + length = slot_type.size + elif isinstance(slot_type, BoundedSequence): + length = min(SEQUENCE_LENGTH, slot_type.maximum_size) + else: + length = SEQUENCE_LENGTH + value_type = slot_type.value_type + values = [_scalar_value(value_type, seed + i) for i in range(length)] + + if isinstance(value_type, BasicType) and \ + value_type.typename in SPECIAL_NESTED_BASIC_TYPES: + type_code, dtype = SPECIAL_NESTED_BASIC_TYPES[value_type.typename] + if isinstance(slot_type, Array): + return numpy.array(values, dtype=dtype) + return array.array(type_code, values) + return values + + +def _populate(message: Any) -> Any: + """Assign a deterministic value to every field of a message.""" + for seed, (name, slot_type) in enumerate( + zip(message.get_fields_and_field_types(), message.SLOT_TYPES)): + setattr(message, name, _field_value(slot_type, seed + 1)) + return message + + +def _assert_same_shape(original: Any, converted: Any, path: str) -> None: + """Assert the converted value has exactly the container types of the original.""" + assert type(original) is type(converted), \ + '%s: expected %s, got %s' % (path, type(original), type(converted)) + if isinstance(original, array.array): + assert original.typecode == converted.typecode, \ + '%s: typecode %s != %s' % (path, original.typecode, converted.typecode) + elif isinstance(original, numpy.ndarray): + assert original.dtype == converted.dtype, \ + '%s: dtype %s != %s' % (path, original.dtype, converted.dtype) + elif isinstance(original, list): + assert len(original) == len(converted), '%s: length differs' % path + for index, (left, right) in enumerate(zip(original, converted)): + _assert_same_shape(left, right, '%s[%d]' % (path, index)) + elif hasattr(original, 'get_fields_and_field_types'): + _assert_message_invariants(original, converted, path) + + +def _assert_message_invariants(original: Any, converted: Any, path: str) -> None: + """Check the slots of a converted message, recursively.""" + # tp_alloc leaves every slot NULL, so a slot the C code forgets to assign only + # shows up as an AttributeError at the point of use. Reading them all here turns + # that into a test failure instead. + assert converted._check_fields == EXPECTED_CHECK_FIELDS, \ + '%s: _check_fields was not set the way __init__ would set it' % path + for name in original.get_fields_and_field_types(): + _assert_same_shape( + getattr(original, name), getattr(converted, name), '%s.%s' % (path, name)) + + +@pytest.mark.parametrize( + 'message_type', MESSAGE_TYPES, ids=[t.__name__ for t in MESSAGE_TYPES]) +def test_round_trip_populated(message_type: type) -> None: + """A fully populated message survives a round trip unchanged.""" + original = _populate(message_type()) + converted = _round_trip(original) + + assert converted == original + assert type(converted) is message_type + _assert_message_invariants(original, converted, message_type.__name__) + # Touches every slot through the public properties, so a slot left unset by the + # C code raises AttributeError here rather than in user code later on. + assert repr(converted) == repr(original) + + +@pytest.mark.parametrize( + 'message_type', MESSAGE_TYPES, ids=[t.__name__ for t in MESSAGE_TYPES]) +def test_round_trip_default(message_type: type) -> None: + """A default constructed message survives a round trip unchanged.""" + original = message_type() + converted = _round_trip(original) + + assert converted == original + _assert_message_invariants(original, converted, message_type.__name__) + assert repr(converted) == repr(original) + + +def test_round_trip_empty_sequences() -> None: + """Empty sequences round trip, covering the zero length array.array path.""" + original = UnboundedSequences() + converted = _round_trip(original) + + assert converted == original + assert converted.uint8_values == array.array('B', []) + assert converted.string_values == [] + + +def test_converted_message_is_usable() -> None: + """A converted message behaves like a normally constructed one.""" + original = _populate(Nested()) + converted = _round_trip(original) + + # Assignment still runs the property setters, including their validation. + converted.basic_types_value.int32_value = 42 + assert converted.basic_types_value.int32_value == 42 + with pytest.raises(AssertionError): + converted.basic_types_value._check_fields = True + converted.basic_types_value.int32_value = 'not an int' + + +def test_wrong_message_type_raises() -> None: + """convert_from_py rejects a message of another type with a TypeError.""" + convert_from_py = ctypes.PYFUNCTYPE( + ctypes.c_bool, ctypes.py_object, ctypes.c_void_p)( + _PyCapsule_GetPointer(_capsule(Nested, 'convert_from_py'), None)) + create = ctypes.PYFUNCTYPE(ctypes.c_void_p)( + _PyCapsule_GetPointer(_capsule(Nested, 'create_ros_message'), None)) + destroy = ctypes.PYFUNCTYPE(None, ctypes.c_void_p)( + _PyCapsule_GetPointer(_capsule(Nested, 'destroy_ros_message'), None)) + + ros_message = create() + try: + # PYFUNCTYPE re-raises the exception that the C function left set. + with pytest.raises(TypeError, match='Nested'): + convert_from_py(BasicTypes(), ros_message) + finally: + destroy(ros_message) + + +def test_no_reference_leak() -> None: + """Repeated conversions do not accumulate references to the message class.""" + import sys + original = _populate(Arrays()) + _round_trip(original) + before = sys.getrefcount(Arrays) + for _ in range(100): + _round_trip(original) + assert sys.getrefcount(Arrays) == before + + +def _unset_slots(message: Any) -> Optional[List[str]]: + return [name for name in message.__slots__ if not hasattr(message, name)] + + +@pytest.mark.parametrize( + 'message_type', MESSAGE_TYPES, ids=[t.__name__ for t in MESSAGE_TYPES]) +def test_every_slot_is_assigned(message_type: type) -> None: + """Every declared slot is populated, including _check_fields.""" + converted = _round_trip(_populate(message_type())) + assert _unset_slots(converted) == []