Remove compilation warning with InstanceHandle_t (#262)

richiware · richiware · commit 45155d831771 · 2025-10-03T11:54:23.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/dds/core/Time_t.i b/fastdds_python/src/swig/fastdds/dds/core/Time_t.i
@@ -0,0 +1,97 @@
+// Copyright 2022 Proyectos y Sistemas de Mantenimiento SL (eProsima).
+//
+// 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.
+
+%{
+#include <fastdds/dds/core/Time_t.hpp>
+%}
+
+// Also ignore the insertion/exraction operator of the remaining Time_t,
+// as it makes no sense on the target language
+%ignore eprosima::fastdds::dds::operator<<(std::ostream&, const Time_t&);
+%ignore eprosima::fastdds::dds::operator>>(std::istream&, Time_t&);
+
+// Ignore the global comparison and arithmetic operators
+// and make them class-internal
+%ignore eprosima::fastdds::dds::operator==(const Time_t&, const Time_t&);
+%ignore eprosima::fastdds::dds::operator!=(const Time_t&, const Time_t&);
+%ignore eprosima::fastdds::dds::operator<(const Time_t&, const Time_t&);
+%ignore eprosima::fastdds::dds::operator<=(const Time_t&, const Time_t&);
+%ignore eprosima::fastdds::dds::operator>(const Time_t&, const Time_t&);
+%ignore eprosima::fastdds::dds::operator>=(const Time_t&, const Time_t&);
+%ignore eprosima::fastdds::dds::operator+(const Time_t&, const Time_t&);
+%ignore eprosima::fastdds::dds::operator-(const Time_t&, const Time_t&);
+
+%ignore eprosima::fastdds::dds::Time_t::is_infinite(const Time_t&);
+
+// Declare the comparison operators as internal to the class
+%extend eprosima::fastdds::dds::Time_t {
+    bool operator==(const eprosima::fastdds::dds::Time_t& other) const
+    {
+        return *$self == other;
+    }
+
+    bool operator!=(const eprosima::fastdds::dds::Time_t& other) const
+    {
+        return *$self != other;
+    }
+
+    bool operator<(const eprosima::fastdds::dds::Time_t& other) const
+    {
+        return *$self < other;
+    }
+
+    bool operator>(const eprosima::fastdds::dds::Time_t& other) const
+    {
+        return *$self > other;
+    }
+
+    bool operator<=(const eprosima::fastdds::dds::Time_t& other) const
+    {
+        return *$self <= other;
+    }
+
+    bool operator>=(const eprosima::fastdds::dds::Time_t& other) const
+    {
+        return *$self >= other;
+    }
+
+    eprosima::fastdds::dds::Time_t operator+ (const eprosima::fastdds::dds::Time_t& other) const
+    {
+        return *$self + other;
+    }
+
+    eprosima::fastdds::dds::Time_t operator- (const eprosima::fastdds::dds::Time_t& other) const
+    {
+        return *$self - other;
+    }
+}
+
+%include <fastdds/dds/core/Time_t.hpp>
+
+namespace eprosima {
+namespace fastdds {
+namespace dds {
+
+struct Duration_t : public Time_t
+{
+    Duration_t();
+
+    Duration_t(
+            int32_t sec,
+            uint32_t nsec);
+};
+
+} // namespace dds
+} // namespace fastdds
+} // namespace eprosima
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

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}`