Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow zero-valued InstanceHandle_t [12942] #2316

Merged
merged 9 commits into from
Nov 30, 2021
229 changes: 134 additions & 95 deletions include/fastdds/rtps/common/InstanceHandle.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
#ifndef _FASTDDS_RTPS_INSTANCEHANDLE_H_
#define _FASTDDS_RTPS_INSTANCEHANDLE_H_

#include <array>

#include <fastrtps/fastrtps_dll.h>
#include <fastdds/rtps/common/Types.h>
#include <fastdds/rtps/common/Guid.h>
Expand All @@ -27,101 +29,164 @@ namespace eprosima {
namespace fastrtps {
namespace rtps {

using KeyHash_t = std::array<octet, 16>;

struct RTPS_DllAPI InstanceHandleValue_t
{
/**
* Write access indexing operator.
*
* Provides a reference to the byte value at position @c i.
*
* @param [in] i index of the byte to return.
*
* @post Method has_been_set() returns @c true.
*
* @remark Do not use this method to check if this value has been set.
* Use method has_been_set() instead.
*/
octet& operator [] (
richiware marked this conversation as resolved.
Show resolved Hide resolved
size_t i) noexcept
{
has_been_set_ = true;
return value_[i];
}

/**
* Read access indexing operator.
*
* Provides the byte value at position @c i.
*
* @param [in] i index of the byte to return.
*
* @remark Do not use this method to check if this value has been set.
* Use method has_been_set() instead.
*/
octet operator [] (
size_t i) const noexcept
{
return value_[i];
}

/**
* Write access pointer cast operator.
*
* Provides a pointer to the start of the raw data.
*
* @post Method has_been_set() returns @c true.
*
* @remark Do not use this method to check if this value has been set.
* Use method has_been_set() instead.
*/
operator octet* () noexcept
{
has_been_set_ = true;
richiware marked this conversation as resolved.
Show resolved Hide resolved
return value_.data();
}

/**
* Read access pointer cast operator.
*
* Provides a pointer to the start of the raw data.
*
* @remark Do not use this method to check if this value has been set.
* Use method has_been_set() instead.
*/
operator const octet* () const noexcept
{
return value_.data();
}

/**
* Return whether any of the write access operators of this value has been used.
*/
bool has_been_set() const noexcept
{
return has_been_set_;
}

/**
* Equality comparison operator.
*/
bool operator == (
const InstanceHandleValue_t& other) const noexcept
{
return (has_been_set_ == other.has_been_set_) && (value_ == other.value_);
}

/**
* Less than comparisor operator.
*/
bool operator < (
const InstanceHandleValue_t& other) const noexcept
{
if (has_been_set_)
{
return other.has_been_set_ && value_ < other.value_;
}

return other.has_been_set_;
}

private:

//! Hash value
KeyHash_t value_ {};
//! Flag indicating if value_ has been modified since the creation of this object
bool has_been_set_ = false;
};

/**
* Struct InstanceHandle_t, used to contain the key for WITH_KEY topics.
* @ingroup COMMON_MODULE
*/
struct RTPS_DllAPI InstanceHandle_t
{
//!Value
octet value[16];
InstanceHandle_t()
{
for (uint8_t i = 0; i < 16; i++)
{
value[i] = 0;
}
}
InstanceHandleValue_t value;

InstanceHandle_t() noexcept = default;

InstanceHandle_t(
const InstanceHandle_t& ihandle)
{
for (uint8_t i = 0; i < 16; i++)
{
value[i] = ihandle.value[i];
}
}
const InstanceHandle_t& ihandle) noexcept = default;

InstanceHandle_t(
const GUID_t& guid)
const GUID_t& guid) noexcept
{
for (uint8_t i = 0; i < 16; ++i)
{
if (i < 12)
{
value[i] = guid.guidPrefix.value[i];
}
else
{
value[i] = guid.entityId.value[i - 12];
}
}
*this = guid;
}

/**
* Assignment operator
* @param ihandle Instance handle to copy the data from
*/
InstanceHandle_t& operator =(
const InstanceHandle_t& ihandle)
{

for (uint8_t i = 0; i < 16; i++)
{
value[i] = ihandle.value[i];
}
return *this;
}
const InstanceHandle_t& ihandle) noexcept = default;

/**
* Assignment operator
* @param guid GUID to copy the data from
*/
InstanceHandle_t& operator =(
const GUID_t& guid)
const GUID_t& guid) noexcept
{
for (uint8_t i = 0; i < 16; i++)
{
if (i < 12)
{
value[i] = guid.guidPrefix.value[i];
}
else
{
value[i] = guid.entityId.value[i - 12];
}
}
octet* dst = value;
memcpy(dst, guid.guidPrefix.value, 12);
memcpy(&dst[12], guid.entityId.value, 4);
return *this;
}

/**
* Know if the instance handle is defined
* @return True if the values are not zero.
*/
bool isDefined() const
bool isDefined() const noexcept
{
for (uint8_t i = 0; i < 16; ++i)
{
if (value[i] != 0)
{
return true;
}
}
return false;
return value.has_been_set();
}

// TODO Review this conversion once InstanceHandle_t is implemented as DDS standard defines
explicit operator const GUID_t&() const
explicit operator const GUID_t&() const noexcept
{
return *reinterpret_cast<const GUID_t*>(this);
}
Expand All @@ -140,21 +205,14 @@ const InstanceHandle_t c_InstanceHandle_Unknown;
*/
inline bool operator ==(
const InstanceHandle_t& ihandle1,
const InstanceHandle_t& ihandle2)
const InstanceHandle_t& ihandle2) noexcept
{
for (uint8_t i = 0; i < 16; ++i)
{
if (ihandle1.value[i] != ihandle2.value[i])
{
return false;
}
}
return true;
return ihandle1.value == ihandle2.value;
}

inline bool operator !=(
const InstanceHandle_t& ihandle1,
const InstanceHandle_t& ihandle2)
const InstanceHandle_t& ihandle2) noexcept
{
return !(ihandle1 == ihandle2);
}
Expand All @@ -168,20 +226,11 @@ inline bool operator !=(
*/
inline void iHandle2GUID(
GUID_t& guid,
const InstanceHandle_t& ihandle)
const InstanceHandle_t& ihandle) noexcept
{
for (uint8_t i = 0; i < 16; ++i)
{
if (i < 12)
{
guid.guidPrefix.value[i] = ihandle.value[i];
}
else
{
guid.entityId.value[i - 12] = ihandle.value[i];
}
}
return;
const octet* value = ihandle.value;
memcpy(guid.guidPrefix.value, value, 12);
memcpy(guid.entityId.value, &value[12], 4);
}

/**
Expand All @@ -190,28 +239,18 @@ inline void iHandle2GUID(
* @return GUID_t
*/
inline GUID_t iHandle2GUID(
const InstanceHandle_t& ihandle)
const InstanceHandle_t& ihandle) noexcept
{
GUID_t guid;
for (uint8_t i = 0; i < 16; ++i)
{
if (i < 12)
{
guid.guidPrefix.value[i] = ihandle.value[i];
}
else
{
guid.entityId.value[i - 12] = ihandle.value[i];
}
}
iHandle2GUID(guid, ihandle);
return guid;
}

inline bool operator <(
const InstanceHandle_t& h1,
const InstanceHandle_t& h2)
const InstanceHandle_t& h2) noexcept
{
return memcmp(h1.value, h2.value, 16) < 0;
return h1.value < h2.value;
}

#ifndef DOXYGEN_SHOULD_SKIP_THIS_PUBLIC
Expand Down
16 changes: 8 additions & 8 deletions test/unittest/dds/subscriber/DataReaderTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ class DataReaderTests : public ::testing::Test
{
FooType data;

data.index(1);
data.index(0);
type_.get_key(&data, &handle_ok_);

data.index(2);
Expand Down Expand Up @@ -512,7 +512,7 @@ class DataReaderTests : public ::testing::Test

// Send data
DataType data;
data.index(1);
data.index(0);
EXPECT_EQ(ReturnCode_t::RETCODE_OK, data_writer_->write(&data, HANDLE_NIL));

// Wait for data to arrive and check OK should be returned
Expand Down Expand Up @@ -883,7 +883,7 @@ TEST_F(DataReaderTests, return_loan)
EXPECT_EQ(ok_code, reader2->enable());

FooType data;
data.index(1);
data.index(0);

// Send a bunch of samples
for (int32_t i = 0; i < num_samples; ++i)
Expand Down Expand Up @@ -1030,7 +1030,7 @@ TEST_F(DataReaderTests, resource_limits)
create_entities(nullptr, reader_qos, SUBSCRIBER_QOS_DEFAULT, writer_qos);

FooType data;
data.index(1);
data.index(0);

// Send a bunch of samples
for (int32_t i = 0; i < num_samples; ++i)
Expand Down Expand Up @@ -1232,7 +1232,7 @@ TEST_F(DataReaderTests, read_unread)
create_entities(nullptr, reader_qos, SUBSCRIBER_QOS_DEFAULT, writer_qos);

FooType data;
data.index(1);
data.index(0);
data.message()[1] = '\0';

// Send a bunch of samples
Expand Down Expand Up @@ -1527,7 +1527,7 @@ TEST_F(DataReaderTests, Deserialization_errors)
create_entities(nullptr, reader_qos, SUBSCRIBER_QOS_DEFAULT, writer_qos);

FooType data;
data.index(1);
data.index(0);
data.message()[1] = '\0';

// Check deserialization errors without loans
Expand Down Expand Up @@ -1780,7 +1780,7 @@ TEST_F(DataReaderTests, check_key_history_wholesomeness_on_unmatch)
FooType sample;
std::array<char, 256> msg = {"checking robustness"};

sample.index(1);
sample.index(0);
sample.message(msg);

ASSERT_TRUE(data_writer_->write(&sample));
Expand Down Expand Up @@ -2053,7 +2053,7 @@ TEST_F(DataReaderTests, read_samples_with_future_changes)
std::this_thread::sleep_for(std::chrono::milliseconds(100)); // Wait discovery

FooType data;
data.index(1);
data.index(0);
data.message()[0] = '\0';
data.message()[1] = '\0';

Expand Down