Skip to content

Add XTypes Runtime XML/JSON Support for Interop Tests#5217

Open
simpsont-oci wants to merge 14 commits into
masterfrom
xtypes_interop_testing
Open

Add XTypes Runtime XML/JSON Support for Interop Tests#5217
simpsont-oci wants to merge 14 commits into
masterfrom
xtypes_interop_testing

Conversation

@simpsont-oci
Copy link
Copy Markdown
Member

@simpsont-oci simpsont-oci commented May 1, 2026

This adds the OpenDDS-side runtime XTypes pieces needed to drive the dds-xtypes interop harness with OpenDDS participants.

  • Adds DDS-XML type loading under OpenDDS::XTypes, gated on Xerces support.
  • Adds DynamicData JSON load/write helpers, including union discriminator format handling for DDS-JSON and dds-xtypes test data.
  • Implements and tests DynamicData::equals() for runtime data comparison.
  • Adds focused unit coverage for XML type loading, JSON DynamicData population/round-trip, and DynamicData equality.
  • Updates build/CI plumbing so Xerces-dependent XML type loading is only built and tested where Xerces is available.

Comment thread dds/DCPS/XTypes/DynamicDataJson.h Outdated
Comment thread dds/DCPS/XTypes/Utils.cpp Outdated
@simpsont-oci simpsont-oci force-pushed the xtypes_interop_testing branch from e1a6326 to 5882a72 Compare May 20, 2026 13:45
@simpsont-oci simpsont-oci changed the title XTypes Interop Testing Support & Fixes Add XTypes Runtime XML/JSON Support for Interop Tests May 20, 2026
@sonndinh sonndinh marked this pull request as ready for review May 20, 2026 19:10
@simpsont-oci simpsont-oci force-pushed the xtypes_interop_testing branch from 1fe5eae to 9d338f9 Compare May 26, 2026 15:58
Comment thread dds/DCPS/XTypes/DynamicDataJson.h Outdated

OpenDDS_Dcps_Export DDS::ReturnCode_t dynamic_data_from_json_file(
DDS::DynamicData_ptr data,
const ACE_TString& json_file,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why ACE_TString?

Comment thread dds/DCPS/XTypes/Utils.cpp Outdated
Comment thread dds/DCPS/XTypes/XmlTypeProvider.cpp Outdated
return DDS::RETCODE_BAD_PARAMETER;
}
DDS::DynamicType_var discriminator_type = get_base_type(td->discriminator_type());
DDS::Int32 discriminator_value = 0;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to handle 64-bit discriminators?

Comment on lines +927 to +929
case TK_ARRAY:
case TK_SEQUENCE:
return populate_collection(data, base, value, options, path);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this (across the entire PR, not just here) support TK_MAP as well?

@simpsont-oci simpsont-oci force-pushed the xtypes_interop_testing branch from ecd8be8 to 765beb3 Compare May 27, 2026 14:38
Comment thread dds/DCPS/XTypes/DynamicDataJson.cpp Outdated
Comment thread dds/DCPS/XTypes/DynamicDataJson.h Outdated
Comment thread dds/DCPS/XTypes/DynamicDataJson.cpp Outdated
Comment thread dds/DCPS/XTypes/Utils.cpp Outdated
Comment on lines +540 to +543
DDS::TypeDescriptor_var descriptor;
if (!get_type_descriptor(descriptor, type)) {
return DDS::DynamicType::_nil();
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

descriptor is obtained here but not used after that -- looks like it is used to check if a valid TypeDescriptor is present?

get_base_type call at line 544 has similar checks to the new get_type_descriptor function, so it looks like there are some duplicates here. Could we use the existing get_base_type function in places where safe_base_type is being used?

Comment thread dds/DCPS/XTypes/Utils.cpp
Comment on lines +695 to +697
if (lhs_rc != DDS::RETCODE_OK) {
return true;
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this return false because this looks like an error case when it fails to get the nested dynamic data objects to continue comparing?

Comment thread dds/DCPS/XTypes/Utils.cpp Outdated
Comment on lines +712 to +715
const bool member_equal = data_equal_i(lhs, rhs, md->id(), md->id(), md->type());
if (!member_equal) {
return false;
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const bool member_equal = data_equal_i(lhs, rhs, md->id(), md->id(), md->type());
if (!member_equal) {
return false;
}
if (!data_equal_i(lhs, rhs, md->id(), md->id(), md->type())) {
return false;
}

Comment thread dds/DCPS/XTypes/Utils.cpp Outdated
Comment on lines +758 to +759
DDS::UInt32 lhs_count = lhs->get_item_count();
DDS::UInt32 rhs_count = rhs->get_item_count();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
DDS::UInt32 lhs_count = lhs->get_item_count();
DDS::UInt32 rhs_count = rhs->get_item_count();
const DDS::UInt32 lhs_count = lhs->get_item_count();
const DDS::UInt32 rhs_count = rhs->get_item_count();

Comment thread dds/DCPS/XTypes/Utils.cpp Outdated
Comment on lines +760 to +761
if (kind == TK_ARRAY) {
lhs_count = rhs_count = bound_total(td);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think lhs_count and rhs_count have already been set by the above get_item_count calls, so this may be redundant.

Comment thread dds/DCPS/XTypes/Utils.cpp Outdated
CORBA::Boolean left = false, right = false;
left_rc = lhs->get_boolean_value(left, lhs_id);
right_rc = rhs->get_boolean_value(right, rhs_id);
return left_rc == right_rc && (left_rc != DDS::RETCODE_OK || left == right);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This returns true when left_rc == right_rc and both are not RETCODE_OK. Shouldn't it return false instead in that case? The other switch cases have the same logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants