Skip to content

Commit

Permalink
Refs #20595: apply review suggestions
Browse files Browse the repository at this point in the history 
Signed-off-by: JLBuenoLopez-eProsima <[email protected]>
  • Loading branch information
@JLBuenoLopez
JLBuenoLopez committed Mar 25, 2024
1 parent 675f796 commit 4af3c43
Show file tree
Hide file tree
Showing 23 changed files with 115 additions and 111 deletions.
2 changes: 2 additions & 0 deletions include/fastdds/dds/xtypes/dynamic_types/DynamicData.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ class DynamicData : public std::enable_shared_from_this<DynamicData>
* Compares two @ref DynamicData, equality requires:
* - Their respective type definitions are equal
* - All contained values are equal and occur in the same order
* - If the samples' type is an aggregated type, previous rule shall be amended as follows:
* -# Members shall be compared without regard to their order.
* @param [in] other @ref DynamicData reference to compare to
* @return `true` on equality
*/
Expand Down
2 changes: 1 addition & 1 deletion include/fastdds/dds/xtypes/dynamic_types/DynamicDataFactory.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class DynamicDataFactory : public std::enable_shared_from_this<DynamicDataFactor
* Resets the singleton reference.
* @return @ref ReturnCode_t
* @retval RETCODE_OK is always returned.
* @todo Improve this documentation.
* @retval RETCODE_BAD_PARAMETER if singleton reference is currently nil.
*/
FASTDDS_EXPORTED_API static ReturnCode_t delete_instance();

Expand Down
1 change: 1 addition & 0 deletions include/fastdds/dds/xtypes/dynamic_types/DynamicTypeBuilder.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ class DynamicTypeBuilder : public std::enable_shared_from_this<DynamicTypeBuilde
* @return ReturnCode_t
* @retval RETCODE_OK when the member was created successfully.
* @retval RETCODE_BAD_PARAMETER when there is an inconsistency.
* @retval RETCODE_PRECONDITION_NOT_MET when the type does not have members.
*/
FASTDDS_EXPORTED_API virtual ReturnCode_t add_member(
traits<MemberDescriptor>::ref_type descriptor) = 0;
Expand Down
6 changes: 6 additions & 0 deletions include/fastdds/dds/xtypes/dynamic_types/MemberDescriptor.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,12 @@ class FASTDDS_EXPORTED_API MemberDescriptor
virtual void default_value(
std::string&& default_value) = 0;

/*!
* Returns the order of definition of the member.
* @return Order of definition.
*/
virtual uint32_t& index() = 0;

/*!
* Returns the order of definition of the member.
* @return Order of definition.
Expand Down
117 changes: 62 additions & 55 deletions src/cpp/fastdds/xtypes/dynamic_types/DynamicDataImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -775,7 +775,7 @@ ReturnCode_t DynamicDataImpl::get_complex_value(
}
else
{
EPROSIMA_LOG_ERROR(DYN_TYPES, "Error setting complex value. MemberId not found.");
EPROSIMA_LOG_ERROR(DYN_TYPES, "Error getting complex value. MemberId not found.");
}
}
}
Expand Down Expand Up @@ -1244,7 +1244,7 @@ ReturnCode_t DynamicDataImpl::set_complex_value(
}
else
{
EPROSIMA_LOG_ERROR(DYN_TYPES, "Error setting due to the types are different.");
EPROSIMA_LOG_ERROR(DYN_TYPES, "Error setting due to the fact that types are different.");
}
}
else
Expand All @@ -1264,63 +1264,59 @@ ReturnCode_t DynamicDataImpl::set_complex_value(
TypeKind element_kind =
get_enclosing_typekind(traits<DynamicType>::narrow<DynamicTypeImpl>(
enclosing_type_->get_descriptor().element_type()));
if (enclosing_type_->get_descriptor().element_type()->equals(value->type()))
if (enclosing_type_->get_descriptor().element_type()->equals(value->type()) &&
is_complex_kind(element_kind))
{
if (is_complex_kind(element_kind))
auto it = value_.cbegin();
assert(value_.cend() != it && MEMBER_ID_INVALID == it->first);
auto sequence =
std::static_pointer_cast<std::vector<traits<DynamicData>::ref_type>>(it->second);
assert(sequence);
if ((TK_ARRAY == type_kind && sequence->size() > id) ||
(TK_SEQUENCE == type_kind &&
(static_cast<uint32_t>(LENGTH_UNLIMITED) ==
enclosing_type_->get_descriptor().bound().at(0) ||
enclosing_type_->get_descriptor().bound().at(0) > id)))
{
auto it = value_.cbegin();
assert(value_.cend() != it && MEMBER_ID_INVALID == it->first);
auto sequence =
std::static_pointer_cast<std::vector<traits<DynamicData>::ref_type>>(it->second);
assert(sequence);
if ((TK_ARRAY == type_kind && sequence->size() > id) ||
(TK_SEQUENCE == type_kind &&
(static_cast<uint32_t>(LENGTH_UNLIMITED) ==
enclosing_type_->get_descriptor().bound().at(0) ||
enclosing_type_->get_descriptor().bound().at(0) > id)))
if (sequence->size() < id + 1)
{
if (sequence->size() < id + 1)
{
sequence->resize(id + 1);
}

auto pos = sequence->begin() + id;
pos = sequence->erase(pos);
sequence->emplace(pos, value->clone());
ret_value = RETCODE_OK;
sequence->resize(id + 1);
}

auto pos = sequence->begin() + id;
pos = sequence->erase(pos);
sequence->emplace(pos, value->clone());
ret_value = RETCODE_OK;
}
}
else
{
EPROSIMA_LOG_ERROR(DYN_TYPES, "Error setting due to the types are different.");
EPROSIMA_LOG_ERROR(DYN_TYPES, "Error setting due to the fact that types are different.");
}
}
else if (TK_MAP == type_kind)
{
TypeKind element_kind =
get_enclosing_typekind(traits<DynamicType>::narrow<DynamicTypeImpl>(
enclosing_type_->get_descriptor().element_type()));
if (enclosing_type_->get_descriptor().element_type()->equals(value->type()))
if (enclosing_type_->get_descriptor().element_type()->equals(value->type()) &&
is_complex_kind(element_kind))
{
if (is_complex_kind(element_kind))
auto it = value_.find(id);
if (it != value_.end())
{
auto it = value_.find(id);
if (it != value_.end())
{
value_.erase(it);
value_.emplace(id, value->clone());
ret_value = RETCODE_OK;
}
else
{
EPROSIMA_LOG_ERROR(DYN_TYPES, "Error setting complex value. MemberId not found.");
}
value_.erase(it);
value_.emplace(id, value->clone());
ret_value = RETCODE_OK;
}
else
{
EPROSIMA_LOG_ERROR(DYN_TYPES, "Error setting complex value. MemberId not found.");
}
}
else
{
EPROSIMA_LOG_ERROR(DYN_TYPES, "Error setting due to the types are different.");
EPROSIMA_LOG_ERROR(DYN_TYPES, "Error setting due to the fact that types are different.");
}
}
}
Expand Down Expand Up @@ -1592,7 +1588,7 @@ size_t DynamicDataImpl::calculate_key_serialized_size(
else
{
EPROSIMA_LOG_ERROR(DYN_TYPES,
"Error serializing structure member because not found on DynamicData");
"Error calculating structure member size because it is not found on DynamicData");
}
}
}
Expand Down Expand Up @@ -1957,7 +1953,7 @@ void DynamicDataImpl::serialize_key(
else
{
EPROSIMA_LOG_ERROR(DYN_TYPES,
"Error serializing structure member because not found on DynamicData");
"Error serializing structure member because it is not found on DynamicData");
}
}
}
Expand All @@ -1982,7 +1978,7 @@ void DynamicDataImpl::serialize_key(
else
{
EPROSIMA_LOG_ERROR(DYN_TYPES,
"Error serializing structure member because not found on DynamicData");
"Error serializing structure member because it is not found on DynamicData");
}
}
}
Expand Down Expand Up @@ -3746,6 +3742,10 @@ ReturnCode_t DynamicDataImpl::get_value(
value,
MEMBER_ID_INVALID);
}
else
{
EPROSIMA_LOG_ERROR(DYN_TYPES, "Error getting value. MemberId not found.");
}
}
else
{
Expand Down Expand Up @@ -3805,6 +3805,10 @@ ReturnCode_t DynamicDataImpl::get_value(
ret_value = get_primitive_value<TK>(element_kind, it, value, MEMBER_ID_INVALID);
}
}
else
{
EPROSIMA_LOG_ERROR(DYN_TYPES, "Error getting value. MemberId not found.");
}
}
else
{
Expand Down Expand Up @@ -3919,7 +3923,7 @@ void DynamicDataImpl::set_default_value(
if (TK_UNION == enclosing_type_->get_kind() && 0 == member->get_descriptor().index())
{
// Set default discriminator value.
set_discriminator_value(enclosing_type_->default_discriminator_value(), member_type, data);
set_discriminator_value(enclosing_type_->default_value(), member_type, data);
selected_union_member_ = enclosing_type_->default_union_member();
}
// Other case, set member with default value.
Expand Down Expand Up @@ -3950,7 +3954,7 @@ void DynamicDataImpl::set_discriminator_value(

if (m_impl->get_descriptor().is_default_label())
{
label = enclosing_type_->default_discriminator_value();
label = enclosing_type_->default_value();
}
else
{
Expand Down Expand Up @@ -4751,22 +4755,16 @@ void DynamicDataImpl::set_value(
{
auto& members = enclosed_type->get_all_members_by_index();
assert(0 < members.size());
TypeForKind<TK_UINT64> value = TypeValueConverter::sto(members.at(0)->get_descriptor().default_value());
bool found = false;
TypeForKind<TK_UINT32> value = TypeValueConverter::sto(members.at(0)->get_descriptor().default_value());
for (auto member_it {members.begin()}; member_it != members.end(); ++member_it)
{
if (0 == sValue.to_string().compare((*member_it)->get_name().to_string()))
{
found = true;
value = TypeValueConverter::sto((*member_it)->get_descriptor().default_value());
break;
}
}

if (found)
{
value = TypeValueConverter::sto(members.at(0)->get_descriptor().default_value());
}

switch (enclosing_type_->get_kind())
{
case TK_INT8:
Expand Down Expand Up @@ -4832,6 +4830,10 @@ ReturnCode_t DynamicDataImpl::set_value(
ret_value = std::static_pointer_cast<DynamicDataImpl>(it->second)->set_value<TK>(
MEMBER_ID_INVALID, new_value);
}
else
{
EPROSIMA_LOG_ERROR(DYN_TYPES, "Error setting value. MemberId not found.");
}

if (RETCODE_OK == ret_value && TK_UNION == type_kind && 0 != id) // Set new discriminator.
{
Expand Down Expand Up @@ -4888,6 +4890,10 @@ ReturnCode_t DynamicDataImpl::set_value(
ret_value = set_primitive_value<TK>(element_type, it, value);
}
}
else
{
EPROSIMA_LOG_ERROR(DYN_TYPES, "Error setting value. MemberId not found.");
}
}
else
{
Expand Down Expand Up @@ -5182,7 +5188,8 @@ size_t DynamicDataImpl::calculate_serialized_size(
current_alignment += 4 + eprosima::fastcdr::Cdr::alignment(current_alignment, 4);
}

// Serializing map's length. Standard rule 15 doesn't specify this but we think it is a typo.
// Serializing map's length. XTypes v1.3 section 7.4.3.5.3 rule 15 doesn't specify this but it is probably a
// typo (other collection types behave this way).
current_alignment += 4 + eprosima::fastcdr::Cdr::alignment(current_alignment, 4);

calculated_size = current_alignment - initial_alignment;
Expand Down Expand Up @@ -5901,7 +5908,7 @@ bool DynamicDataImpl::deserialize(
else
{
EPROSIMA_LOG_ERROR(DYN_TYPES,
"Error deserializing bitset bitfield because not found on DynamicData");
"Error deserializing bitset bitfield because it is not found on DynamicData");
}

++index;
Expand Down Expand Up @@ -6676,7 +6683,7 @@ void DynamicDataImpl::serialize(
else
{
EPROSIMA_LOG_ERROR(DYN_TYPES,
"Error serializing bitset bitfield because not found on DynamicData");
"Error serializing bitset bitfield because it is not found on DynamicData");
}

++index;
Expand Down Expand Up @@ -7018,7 +7025,7 @@ void DynamicDataImpl::serialize(
else
{
EPROSIMA_LOG_ERROR(DYN_TYPES,
"Error serializing structure member because not found on DynamicData");
"Error serializing structure member because it is not found on DynamicData");
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/cpp/fastdds/xtypes/dynamic_types/DynamicDataImpl.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,7 @@ class DynamicDataImpl : public traits<DynamicData>::base_type
{
ret_value = true;

if (enclosing_type_->default_discriminator_value() != static_cast<int32_t>(value))
if (enclosing_type_->default_value() != static_cast<int32_t>(value))
{
for (auto member : enclosing_type_->get_all_members_by_index())
{
Expand Down
6 changes: 3 additions & 3 deletions src/cpp/fastdds/xtypes/dynamic_types/DynamicTypeBuilderImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ ReturnCode_t DynamicTypeBuilderImpl::add_member(

if (mid == new_member_id)
{
EPROSIMA_LOG_ERROR(DYN_TYPES, "Inconsistency in the new MemberId because is equal than MemberId(" <<
EPROSIMA_LOG_ERROR(DYN_TYPES, "Inconsistency in the new MemberId because it is equal to MemberId(" <<
mid << ")");
return RETCODE_BAD_PARAMETER;
}
Expand Down Expand Up @@ -626,7 +626,7 @@ traits<DynamicType>::ref_type DynamicTypeBuilderImpl::build() noexcept
EPROSIMA_LOG_ERROR(DYN_TYPES, "Expected more members in BITSET according to the size of bounds.");
}

// In case of ENUM, it must have at least one literal
// In case of ENUM and BITSET, it must have at least one member
preconditions &= (TK_ENUM != type_descriptor_.kind() && TK_BITSET != type_descriptor_.kind())
|| 0 < members_.size();
if (!preconditions)
Expand All @@ -645,7 +645,7 @@ traits<DynamicType>::ref_type DynamicTypeBuilderImpl::build() noexcept
ret_val->member_ = member_;
ret_val->member_by_name_ = member_by_name_;
ret_val->members_ = members_;
ret_val->default_discriminator_value_ = default_value_;
ret_val->default_value_ = default_value_;
ret_val->default_union_member_ = default_union_member_;
}
}
Expand Down
6 changes: 2 additions & 4 deletions src/cpp/fastdds/xtypes/dynamic_types/DynamicTypeImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,8 @@ bool DynamicTypeImpl::equals(
ret_value &= annotation_.size() == impl->annotation_.size();
if (ret_value)
{
for (size_t count {0}; ret_value && count < annotation_.size(); ++count)
for (auto& annotation : annotation_)
{
auto& annotation = annotation_.at(count);
ret_value &= impl->annotation_.end() != std::find_if(impl->annotation_.begin(), impl->annotation_.end(),
[&annotation](AnnotationDescriptorImpl& a)
{
Expand Down Expand Up @@ -203,9 +202,8 @@ bool DynamicTypeImpl::equals(
ret_value &= verbatim_.size() == impl->verbatim_.size();
if (ret_value)
{
for (size_t count {0}; ret_value && count < verbatim_.size(); ++count)
for (auto& verbatim : verbatim_)
{
auto& verbatim = verbatim_.at(count);
ret_value &= impl->verbatim_.end() != std::find_if(impl->verbatim_.begin(), impl->verbatim_.end(),
[&verbatim](VerbatimTextDescriptorImpl& v)
{
Expand Down
Loading

0 comments on commit 4af3c43

Please sign in to comment.