Remove compilation warning with InstanceHandle_t (#262)

richiware · richiware · commit aff976d3418c · 2025-10-03T11:56:30.000+02:00
* Refs #23753. Take into account dependencies for rebuild Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev> * Refs #23753. Fix warning Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev> * Refs #23753. Remove swig warnings Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev> * Refs #23753. Add unit tests Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev> * Refs #23753. Apply suggestions Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev> --------- Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev> (cherry picked from commit 6ee4ca5)
diff --git a/fastdds_python/src/swig/CMakeLists.txt b/fastdds_python/src/swig/CMakeLists.txt
@@ -27,6 +27,7 @@ set(${PROJECT_NAME}_MODULE
 set(${PROJECT_NAME}_FILES
     ${${PROJECT_NAME}_MODULE}.i
     )
+set_property(SOURCE ${${PROJECT_NAME}_FILE} PROPERTY USE_SWIG_DEPENDENCIES TRUE)
 
 SET_SOURCE_FILES_PROPERTIES(
     ${${PROJECT_NAME}_FILES}
diff --git a/fastdds_python/src/swig/fastdds/rtps/common/InstanceHandle.i b/fastdds_python/src/swig/fastdds/rtps/common/InstanceHandle.i
@@ -13,6 +13,9 @@
 // limitations under the License.
 
 %{
+#include <stdexcept>
+#include <vector>
+
 #include "fastdds/rtps/common/InstanceHandle.h"
 
 // Define a hash method in global scope for InstanceHandle_t types
@@ -26,60 +29,119 @@ long hash(const eprosima::fastrtps::rtps::InstanceHandle_t& handle)
     }
     return ret;
 }
-
 %}
 
 // SWIG does not support type conversion operators correctly unless converted to a normal method
 %rename(get_guid) eprosima::fastrtps::rtps::InstanceHandle_t::operator const GUID_t&;
 
-%ignore eprosima::fastrtps::rtps::InstanceHandleValue_t::operator [] const;
-%ignore eprosima::fastrtps::rtps::operator <<(std::ostream&, const InstanceHandle_t&);
-%ignore eprosima::fastrtps::rtps::operator >>(std::istream&, InstanceHandle_t&);
-%rename(read_pointer_cast) eprosima::fastrtps::rtps::InstanceHandleValue_t::operator const octet* () const;
-%rename(write_pointer_cast) eprosima::fastrtps::rtps::InstanceHandleValue_t::operator octet* ();
-
-%typemap(in) eprosima::fastrtps::rtps::InstanceHandleValue_t*(eprosima::fastrtps::rtps::InstanceHandleValue_t temp)
-{
-    if (PyTuple_Check($input))
-    {
-        eprosima::fastrtps::rtps::octet* buf = temp;
-        if (!PyArg_ParseTuple($input, "BBBBBBBBBBBBBBBB",
-                    buf, buf+1, buf+2, buf+3, buf+4, buf+5, buf+6, buf+7, buf+8,
-                    buf+9, buf+10, buf+11, buf+12, buf+13, buf+14, buf+15))
+%ignore eprosima::fastdds::rtps::InstanceHandleValue_t::operator [] const;
+%ignore eprosima::fastdds::rtps::InstanceHandleValue_t::operator [];
+%ignore eprosima::fastdds::rtps::operator <<(std::ostream&, const InstanceHandle_t&);
+%ignore eprosima::fastdds::rtps::operator >>(std::istream&, InstanceHandle_t&);
+%ignore eprosima::fastdds::rtps::InstanceHandleValue_t::operator const octet* () const;
+%ignore eprosima::fastdds::rtps::InstanceHandleValue_t::operator octet* ();
+
+%extend eprosima::fastdds::rtps::InstanceHandleValue_t {
+
+    // Constructor from a sequence of 16 bytes (tuple/list/bytes/bytearray)
+    InstanceHandleValue_t(PyObject* seq) {
+        eprosima::fastdds::rtps::InstanceHandleValue_t* self = new eprosima::fastdds::rtps::InstanceHandleValue_t();
+        SWIG_PYTHON_THREAD_BEGIN_BLOCK;
+
+        // Fast-path: bytes
+        if (PyBytes_Check(seq)) {
+            if (PyBytes_GET_SIZE(seq) == 16)
+            {
+                const char* b = PyBytes_AS_STRING(seq);
+                for (int i = 0; i < 16; ++i) (*self)[i] = (uint8_t)(unsigned char)b[i];
+            }
+            else
+            {
+                delete self;
+                self = nullptr;
+                PyErr_SetString(PyExc_ValueError, "Expected 16 bytes");
+            }
+        }
+        // Fast-path: bytearray
+        else if (PyByteArray_Check(seq))
         {
-            PyErr_SetString(PyExc_TypeError, "tuple must have 16 elements");
-            SWIG_fail;
+            if (PyByteArray_GET_SIZE(seq) == 16)
+            {
+                const char* b = PyByteArray_AS_STRING(seq);
+                for (int i = 0; i < 16; ++i) (*self)[i] = (uint8_t)(unsigned char)b[i];
+            }
+            else
+            {
+                delete self;
+                self = nullptr;
+                PyErr_SetString(PyExc_ValueError, "Expected 16 bytes");
+            }
         }
-        $1 = &temp;
-    }
-    else
-    {
-        PyErr_SetString(PyExc_TypeError, "expected a tuple.");
-        SWIG_fail;
+        else
+        {
+            // Generic fallback: iterable from 16 ints 0..255
+            PyObject* it = PyObject_GetIter(seq);
+            size_t count {0};
+            if (it)
+            {
+                PyObject* item {nullptr};
+                while ((item = PyIter_Next(it)) && count < 16)
+                {
+                    long val = PyLong_AsLong(item);
+                    Py_DECREF(item);
+                    if (val == -1 && PyErr_Occurred())
+                    {
+                        delete self;
+                        self = nullptr;
+                        PyErr_SetString(PyExc_TypeError, "Sequence must contain integers");
+                        break;
+                    }
+                    else if (val < 0 || val > 255)
+                    {
+                        delete self;
+                        self = nullptr;
+                        PyErr_SetString(PyExc_ValueError, "Each value must be in 0..255");
+                        break;
+                    }
+
+                    (*self)[count] = static_cast<uint8_t>(val);
+                    ++count;
+                }
+                Py_DECREF(it);
+                if ((nullptr != item || count != 16) && nullptr != self)
+                {
+                    delete self;
+                    self = nullptr;
+                    PyErr_SetString(PyExc_ValueError, "Expected 16 elements");
+                }
+            }
+            else
+            {
+                delete self;
+                self = nullptr;
+                PyErr_SetString(PyExc_TypeError, "Expected a sequence of 16 integers (0..255) or 16-byte object");
+            }
+        }
+
+        SWIG_PYTHON_THREAD_END_BLOCK;
+
+        return self;
     }
-}
 
-%typemap(out) eprosima::fastrtps::rtps::InstanceHandleValue_t*
-{
-    constexpr size_t ih_size = std::tuple_size<eprosima::fastrtps::rtps::KeyHash_t>::value;
-    PyObject* python_tuple = PyTuple_New(ih_size);
+    size_t __len__() const { return 16; }
 
-    if (python_tuple)
-    {
-        for(size_t count = 0; count < ih_size; ++count)
-        {
-            PyTuple_SetItem(python_tuple, count, PyInt_FromLong((*$1)[count]));
-        }
+    uint8_t __getitem__(size_t i) const {
+        if (i >= 16) throw std::out_of_range("index out of range");
+        return $self->operator[](i);
     }
 
-    $result = python_tuple;
+    void __setitem__(size_t i, uint8_t v) {
+        if (i >= 16) throw std::out_of_range("index out of range");
+        $self->operator[](i) = v;
+    }
 }
 
-// Template for std::vector<InstanceHandle_t>
-%template(InstanceHandleVector) std::vector<eprosima::fastrtps::rtps::InstanceHandle_t>;
-%typemap(doctype) std::vector<eprosima::fastrtps::rtps::InstanceHandle_t>"InstanceHandleVector";
-
-%include "fastdds/rtps/common/InstanceHandle.h"
+%ignore eprosima::fastdds::rtps::InstanceHandle_t::value;
 
 // Declare the comparison operators as internal to the class
 %extend eprosima::fastrtps::rtps::InstanceHandle_t {
@@ -106,4 +168,51 @@ long hash(const eprosima::fastrtps::rtps::InstanceHandle_t& handle)
     {
         return hash(*$self);
     }
+
+    // Setter from sequence (tuple/list/bytes/bytearray)
+    void from_sequence(PyObject* seq) {
+        // Reuse the constructor to validate and copy
+        eprosima::fastdds::rtps::InstanceHandleValue_t* tmp = new_eprosima_fastdds_rtps_InstanceHandleValue_t(seq);
+        if (nullptr != tmp)
+        {
+            for (int i = 0; i < 16; ++i) $self->value[i] = (*tmp)[i];
+            delete tmp; // avoid memory leak
+        }
+    }
+
+    // Getter: return a tuple of 16 ints (0..255)
+    PyObject* to_sequence() const {
+        SWIG_PYTHON_THREAD_BEGIN_BLOCK;
+
+        PyObject* python_tuple = PyTuple_New(16);
+
+        if (python_tuple)
+        {
+            for(size_t count = 0; count < 16; ++count)
+            {
+                PyTuple_SetItem(python_tuple, count, PyInt_FromLong($self->value[count]));
+            }
+        }
+
+        SWIG_PYTHON_THREAD_END_BLOCK;
+
+        return python_tuple;
+    }
 }
+
+// Template for std::vector<InstanceHandle_t>
+%template(InstanceHandleVector) std::vector<eprosima::fastdds::rtps::InstanceHandle_t>;
+%typemap(doctype) std::vector<eprosima::fastdds::rtps::InstanceHandle_t>"InstanceHandleVector";
+
+%include "fastdds/rtps/common/InstanceHandle.h"
+
+%pythoncode %{
+def _ihv_get_value(self):
+    return self.to_sequence()
+
+def _ihv_set_value(self, seq):
+    self.from_sequence(seq)
+
+InstanceHandle_t.value = property(_ihv_get_value, _ihv_set_value,
+                                       doc="16-byte value as list/tuple/bytes/bytearray")
+%}
diff --git a/fastdds_python/test/CMakeLists.txt b/fastdds_python/test/CMakeLists.txt
@@ -20,7 +20,17 @@ if (${fastcdr_VERSION_MAJOR} EQUAL 1)
     set(fastcdr_version_argument "v1")
 endif()
 
+# Unit tests
+add_test(NAME unit_tests
+    COMMAND
+        ${Python3_EXECUTABLE}
+        -m pytest
+        -vrP
+    WORKING_DIRECTORY
+        ${CMAKE_CURRENT_SOURCE_DIR}/unittest
+    )
 
+# DDS Api tests
 add_test(NAME api_tests
     COMMAND
         ${Python3_EXECUTABLE}
diff --git a/fastdds_python/test/unittest/dds/common/test_InstanceHandle.py b/fastdds_python/test/unittest/dds/common/test_InstanceHandle.py
@@ -0,0 +1,141 @@
+import fastdds
+import pytest
+import sys
+
+
+def test_create_instance_handle_from_bytes():
+    ih = fastdds.InstanceHandle_t()
+    ih.value = b"\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f\x10"
+    assert (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16) == ih.value
+
+
+def test_create_instance_handle_from_bytearray():
+    ih = fastdds.InstanceHandle_t()
+    ih.value = bytearray([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16])
+    assert (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16) == ih.value
+
+
+def test_create_instance_handle_from_tuple():
+    ih = fastdds.InstanceHandle_t()
+    ih.value = (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16)
+    assert (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16) == ih.value
+
+
+def test_create_instance_handle_from_list():
+    ih = fastdds.InstanceHandle_t()
+    ih.value = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16]
+    assert (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16) == ih.value
+
+
+def test_create_instance_handle_from_bytes_with_less_elements():
+    ih = fastdds.InstanceHandle_t()
+    with pytest.raises(SystemError) as exception:
+        ih.value = b"\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e"
+    repr = exception.getrepr()
+    assert str(repr).split("\n")[0] == "ValueError: Expected 16 bytes"
+
+
+def test_create_instance_handle_from_bytes_with_more_elements():
+    ih = fastdds.InstanceHandle_t()
+    with pytest.raises(SystemError) as exception:
+        ih.value = (
+            b"\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f\x10\x11\x12"
+        )
+    repr = exception.getrepr()
+    assert str(repr).split("\n")[0] == "ValueError: Expected 16 bytes"
+
+
+def test_create_instance_handle_from_bytearray_with_less_elements():
+    ih = fastdds.InstanceHandle_t()
+    with pytest.raises(SystemError) as exception:
+        ih.value = bytearray([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14])
+    repr = exception.getrepr()
+    assert str(repr).split("\n")[0] == "ValueError: Expected 16 bytes"
+
+
+def test_create_instance_handle_from_bytearray_with_more_elements():
+    ih = fastdds.InstanceHandle_t()
+    with pytest.raises(SystemError) as exception:
+        ih.value = bytearray(
+            [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18]
+        )
+    repr = exception.getrepr()
+    assert str(repr).split("\n")[0] == "ValueError: Expected 16 bytes"
+
+
+def test_create_instance_handle_from_tuple_with_less_elements():
+    ih = fastdds.InstanceHandle_t()
+    with pytest.raises(SystemError) as exception:
+        ih.value = (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14)
+    repr = exception.getrepr()
+    assert str(repr).split("\n")[0] == "ValueError: Expected 16 elements"
+
+
+def test_create_instance_handle_from_tuple_with_more_elements():
+    ih = fastdds.InstanceHandle_t()
+    with pytest.raises(SystemError) as exception:
+        ih.value = (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18)
+    repr = exception.getrepr()
+    assert str(repr).split("\n")[0] == "ValueError: Expected 16 elements"
+
+
+def test_create_instance_handle_from_list_with_less_elements():
+    ih = fastdds.InstanceHandle_t()
+    with pytest.raises(SystemError) as exception:
+        ih.value = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14]
+    repr = exception.getrepr()
+    assert str(repr).split("\n")[0] == "ValueError: Expected 16 elements"
+
+
+def test_create_instance_handle_from_list_with_more_elements():
+    ih = fastdds.InstanceHandle_t()
+    with pytest.raises(SystemError) as exception:
+        ih.value = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18]
+    repr = exception.getrepr()
+    assert str(repr).split("\n")[0] == "ValueError: Expected 16 elements"
+
+
+def test_create_instance_handle_from_bytearray_with_with_negative_number():
+    ih = fastdds.InstanceHandle_t()
+    with pytest.raises(ValueError) as exception:
+        ih.value = bytearray([1, 2, 3, 4, 5, 6, 7, 8, 9, -10, 11, 12, 13, 14, 15, 16])
+    assert str(exception.value) == "byte must be in range(0, 256)"
+
+
+def test_create_instance_handle_from_bytearray_with_large_number():
+    ih = fastdds.InstanceHandle_t()
+    with pytest.raises(ValueError) as exception:
+        ih.value = bytearray([1, 2, 3, 4, 5, 6, 7, 8, 9, 1000, 11, 12, 13, 14, 15, 16])
+    assert str(exception.value) == "byte must be in range(0, 256)"
+
+
+def test_create_instance_handle_from_tuple_with_negative_number():
+    ih = fastdds.InstanceHandle_t()
+    with pytest.raises(SystemError) as exception:
+        ih.value = (1, 2, 3, 4, 5, 6, 7, 8, 9, -10, 11, 12, 13, 14, -15, 16)
+    repr = exception.getrepr()
+    assert str(repr).split("\n")[0] == "ValueError: Each value must be in 0..255"
+
+
+def test_create_instance_handle_from_tuple_with_large_number():
+    ih = fastdds.InstanceHandle_t()
+    with pytest.raises(SystemError) as exception:
+        ih.value = (1, 2, 3, 4, 5, 6, 7, 8, 9, 1000, 11, 12, 13, 14, 15, 16)
+    repr = exception.getrepr()
+    assert str(repr).split("\n")[0] == "ValueError: Each value must be in 0..255"
+
+
+def test_create_instance_handle_from_list_with_negative_number():
+    ih = fastdds.InstanceHandle_t()
+    with pytest.raises(SystemError) as exception:
+        ih.value = [1, 2, 3, 4, 5, 6, 7, 8, 9, -10, 11, 12, 13, 14, 15, 16]
+    repr = exception.getrepr()
+    assert str(repr).split("\n")[0] == "ValueError: Each value must be in 0..255"
+
+
+def test_create_instance_handle_from_list_with_large_number():
+    ih = fastdds.InstanceHandle_t()
+    with pytest.raises(SystemError) as exception:
+        ih.value = [1, 2, 3, 4, 5, 6, 7, 8, 9, 1000, 11, 12, 13, 14, 15, 16]
+    repr = exception.getrepr()
+    assert str(repr).split("\n")[0] == "ValueError: Each value must be in 0..255"

Original file line number	Diff line number	Diff line change
`@@ -27,6 +27,7 @@ set(${PROJECT_NAME}_MODULE`
`27`	`27`	`set(${PROJECT_NAME}_FILES`
`28`	`28`	`${${PROJECT_NAME}_MODULE}.i`
`29`	`29`	`)`
	`30`	`+set_property(SOURCE ${${PROJECT_NAME}_FILE} PROPERTY USE_SWIG_DEPENDENCIES TRUE)`
`30`	`31`
`31`	`32`	`SET_SOURCE_FILES_PROPERTIES(`
`32`	`33`	`${${PROJECT_NAME}_FILES}`