PSMX support for C++ #453

eboasson · 2023-09-06T09:49:01Z

Companion to eclipse-cyclonedds/cyclonedds#1676

The addition of the PSMX model of data exchange in CycloneDDS requires the addition of PSMX specific serdata functions: from_loaned_sample and from_psmx, these have been implemented It also requires the removal of the hardcoded iceoryx code, as this is now handled through PSMX functionality and any loaded module's implementation Signed-off-by: Martijn Reicher <[email protected]>

Signed-off-by: Martijn Reicher <[email protected]>

As this type of exchange is now abstracted away behind PSMX calls, it is no longer necessary Signed-off-by: Martijn Reicher <[email protected]>

As per clang's recommendation. Signed-off-by: Erik Boasson <[email protected]>

Signed-off-by: Erik Boasson <[email protected]>

dpotman

The datatopic changes and the PSMX qos implementation look fine to me.

A few details on other things:

wrong indenting for an if statement, see below
getSampleSize defined in src/ddscxx/include/org/eclipse/cyclonedds/topic/TopicTraits.hpp:113 seems to be unused.

And I think we need a replacement for the shared memory tests that are removed with this PR.

dpotman · 2023-09-07T07:36:19Z

src/ddscxx/src/org/eclipse/cyclonedds/core/policy/PolicyDelegate.cpp

+void PSMXInstancesDelegate::check() const
+{
+    if (instances_.size() > DDS_MAX_PSMX_INSTANCES)
+    ISOCPP_THROW_EXCEPTION(ISOCPP_INVALID_ARGUMENT_ERROR, "Invalid size of PSMXInstances::instances value (%ld > %d).", instances_.size(), DDS_MAX_PSMX_INSTANCES);


Wrong indenting

You are correct.

dpotman · 2023-09-07T10:09:30Z

src/ddscxx/include/org/eclipse/cyclonedds/topic/datatopic.hpp

+
+  if (!m_t.compare_exchange_strong(t, static_cast<T*>(newloan->sample_ptr), std::memory_order_seq_cst))
+  {
+    //something went wrong


Is error handling required here?

Let me go through this a bit.

dpotman · 2023-09-07T10:20:34Z

src/ddscxx/include/org/eclipse/cyclonedds/topic/datatopic.hpp

+  if (!serialize_data)
+    metadata->sample_state = (key ? DDS_LOANED_SAMPLE_STATE_RAW_KEY : DDS_LOANED_SAMPLE_STATE_RAW_DATA);
+
+  metadata->cdr_identifier = hdr[0];  //remove endianness?


I think there's no need to 'remove' any endianness, as the endianness in the header is native

You are correct.
I will fix this.

This reverts commit 99b708e.

Signed-off-by: Erik Boasson <[email protected]>

Signed-off-by: Dennis Potman <[email protected]>

…in from_psmx Signed-off-by: Dennis Potman <[email protected]>

Signed-off-by: Dennis Potman <[email protected]>

…me as sertype_default) Signed-off-by: Dennis Potman <[email protected]>

Signed-off-by: Dennis Potman <[email protected]>

Now that Iceoryx is loaded as a PSMX plugin there is no longer any dependency on Iceoryx in the C++ API. The Iceoryx-based test uses the C++ API of Iceoryx, and so the Iceoryx C binding is no longer relevant. Signed-off-by: Erik Boasson <[email protected]>

* ENABLE_SHM => ENABLE_ICEORYX, which now only has to do with testing * SharedMemoryTest => IceoryxTest Signed-off-by: Erik Boasson <[email protected]>

Signed-off-by: Erik Boasson <[email protected]>

reicheratwork · 2023-09-18T08:48:01Z

src/ddscxx/include/dds/core/policy/TCorePolicy.hpp

@@ -1887,6 +1887,19 @@ class TTypeConsistencyEnforcement : public dds::core::Value<D>
 //==============================================================================


+template <typename D>


I think we still need to add documentation to this.

reicheratwork · 2023-09-18T08:52:36Z

src/ddscxx/include/dds/core/policy/CorePolicy.hpp

@@ -195,6 +198,7 @@ OMG_DDS_POLICY_TRAITS(DataRepresentation,   23)
 OMG_DDS_POLICY_TRAITS(TypeConsistencyEnforcement, 24)
 #endif  // OMG_DDS_EXTENSIBLE_AND_DYNAMIC_TOPIC_TYPE_SUPPORT
 OMG_DDS_POLICY_TRAITS(WriterBatching,       25)
+OMG_DDS_POLICY_TRAITS(PSMXInstances,        34)


Maybe this comment should be on the CycloneDDS-C repository's PR, but are we sure that this new Policy-ID will not conflict with other vendors' stuff?

reicheratwork · 2023-09-18T08:54:25Z

src/ddscxx/include/org/eclipse/cyclonedds/topic/datatopic.hpp

+  if (!serialize_data)
+    metadata->sample_state = (key ? DDS_LOANED_SAMPLE_STATE_RAW_KEY : DDS_LOANED_SAMPLE_STATE_RAW_DATA);
+
+  metadata->cdr_identifier = hdr[0];  //remove endianness?


You are correct.
I will fix this.

reicheratwork · 2023-09-18T08:54:56Z

src/ddscxx/include/org/eclipse/cyclonedds/topic/datatopic.hpp

+
+  if (!m_t.compare_exchange_strong(t, static_cast<T*>(newloan->sample_ptr), std::memory_order_seq_cst))
+  {
+    //something went wrong


Let me go through this a bit.

reicheratwork · 2023-09-18T08:55:38Z

src/ddscxx/src/org/eclipse/cyclonedds/core/policy/PolicyDelegate.cpp

+void PSMXInstancesDelegate::check() const
+{
+    if (instances_.size() > DDS_MAX_PSMX_INSTANCES)
+    ISOCPP_THROW_EXCEPTION(ISOCPP_INVALID_ARGUMENT_ERROR, "Invalid size of PSMXInstances::instances value (%ld > %d).", instances_.size(), DDS_MAX_PSMX_INSTANCES);


You are correct.

- Added documentation to new PSMXInstances QoSPolicy - Added exception thrown when loan setting fails. - Small layout fixes Signed-off-by: Martijn Reicher <[email protected]>

eboasson · 2023-09-19T14:36:48Z

CI run against eclipse-cyclonedds/cyclonedds#1676: https://dev.azure.com/erikboasson/Cyclone%20DDS%20CXX/_build/results?buildId=993&view=results

reicheratwork and others added 6 commits September 5, 2023 12:35

Added C++ PSMXInstances QoSPolicy

dbcfc36

Signed-off-by: Martijn Reicher <[email protected]>

Removed the ENABLE_SHM CMake option

99b708e

As this type of exchange is now abstracted away behind PSMX calls, it is no longer necessary Signed-off-by: Martijn Reicher <[email protected]>

Mark static function in header file as "inline"

1416bc6

As per clang's recommendation. Signed-off-by: Erik Boasson <[email protected]>

Update for DDS_LOANED_SAMPLE_STATE_RAW_KEY

9f42b1f

Signed-off-by: Erik Boasson <[email protected]>

No warnings for intentional use of deprecated APIs

d76550d

Signed-off-by: Erik Boasson <[email protected]>

dpotman reviewed Sep 7, 2023

View reviewed changes

eboasson and others added 14 commits September 9, 2023 15:12

Revert "Removed the ENABLE_SHM CMake option"

e292878

This reverts commit 99b708e.

Fix alignment issue in type object output by IDLC

04eac26

Signed-off-by: Erik Boasson <[email protected]>

WiP: Iceoryx + C++

c2ca988

Signed-off-by: Erik Boasson <[email protected]>

Some fixes in SharedMemory tests

1609961

Signed-off-by: Dennis Potman <[email protected]>

Fix header offset in from_loaned_sample and assume native endianness …

c76a306

…in from_psmx Signed-off-by: Dennis Potman <[email protected]>

Replace RTPS encoding header defines

e5a8476

Signed-off-by: Dennis Potman <[email protected]>

Memset cdr header to 0 in serdata tests

835e94f

Signed-off-by: Dennis Potman <[email protected]>

Fix build-shared flag for iceoryx in build template.

ecaf346

Signed-off-by: Dennis Potman <[email protected]>

Fix sertype get_serialized_size: shouldn't include the CDR header (sa…

2136806

…me as sertype_default) Signed-off-by: Dennis Potman <[email protected]>

Fixing populate hash in from_loaned_sample

6f76d01

Signed-off-by: Dennis Potman <[email protected]>

Things related to Iceoryx should be named that way

4464393

* ENABLE_SHM => ENABLE_ICEORYX, which now only has to do with testing * SharedMemoryTest => IceoryxTest Signed-off-by: Erik Boasson <[email protected]>

Use ENABLE_ICEORYX in Cyclone core build config

0fc073c

Signed-off-by: Erik Boasson <[email protected]>

Add a basic test for write_cdr

73e3055

Signed-off-by: Erik Boasson <[email protected]>

reicheratwork approved these changes Sep 18, 2023

View reviewed changes

Fixes originating from review process

96b09a3

- Added documentation to new PSMXInstances QoSPolicy - Added exception thrown when loan setting fails. - Small layout fixes Signed-off-by: Martijn Reicher <[email protected]>

eboasson merged commit ef77160 into eclipse-cyclonedds:master Sep 20, 2023
1 of 19 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PSMX support for C++ #453

PSMX support for C++ #453

eboasson commented Sep 6, 2023

dpotman left a comment

dpotman Sep 7, 2023

reicheratwork Sep 18, 2023

dpotman Sep 7, 2023

reicheratwork Sep 18, 2023

dpotman Sep 7, 2023

reicheratwork Sep 18, 2023

reicheratwork Sep 18, 2023

reicheratwork Sep 18, 2023

reicheratwork Sep 18, 2023

reicheratwork Sep 18, 2023

reicheratwork Sep 18, 2023

eboasson commented Sep 19, 2023

		@@ -1887,6 +1887,19 @@ class TTypeConsistencyEnforcement : public dds::core::Value<D>
		//==============================================================================


		template <typename D>

PSMX support for C++ #453

PSMX support for C++ #453

Conversation

eboasson commented Sep 6, 2023

dpotman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eboasson commented Sep 19, 2023