Skip to content

Commit

Permalink
Simplified entity_properties usage
Browse files Browse the repository at this point in the history
Enum write functions no longer take entity_properties, the bit_bound
is now taken from a constexpr trait for the enum
Entity properties trees are now generated the same for primitives/
enums/constructed types
Fixed bug in xcdr1 serialization of sequences of primitives, which
did had an emheader which might not be able to contain its length

Signed-off-by: Martijn Reicher <[email protected]>
  • Loading branch information
reicheratwork authored and eboasson committed Mar 20, 2022
1 parent e319d18 commit 589bce0
Show file tree
Hide file tree
Showing 7 changed files with 135 additions and 192 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ class OMG_DDS_API basic_cdr_stream : public cdr_stream {
* @param[in] N The number of entities to read.
*/
template<typename T, std::enable_if_t<std::is_enum<T>::value && !std::is_arithmetic<T>::value, bool> = true >
bool read(basic_cdr_stream& str, T& toread, const entity_properties_t &, size_t N = 1) {
bool read(basic_cdr_stream& str, T& toread, size_t N = 1) {
return read_enum_impl<basic_cdr_stream,T,uint32_t>(str, toread, N);
}

Expand All @@ -108,7 +108,7 @@ bool read(basic_cdr_stream& str, T& toread, const entity_properties_t &, size_t
* @param[in] N The number of entities to write.
*/
template<typename T, std::enable_if_t<std::is_enum<T>::value && !std::is_arithmetic<T>::value, bool> = true >
bool write(basic_cdr_stream& str, const T& towrite, const entity_properties_t &, size_t N = 1) {
bool write(basic_cdr_stream& str, const T& towrite, size_t N = 1) {
return write_enum_impl<basic_cdr_stream,T,uint32_t>(str, towrite, N);
}

Expand All @@ -120,7 +120,7 @@ bool write(basic_cdr_stream& str, const T& towrite, const entity_properties_t &,
* @param[in] N The number of entities to move.
*/
template<typename T, std::enable_if_t<std::is_enum<T>::value && !std::is_arithmetic<T>::value, bool> = true >
bool move(basic_cdr_stream& str, const T&, const entity_properties_t &, size_t N = 1) {
bool move(basic_cdr_stream& str, const T&, size_t N = 1) {
return move(str, uint32_t(0), N);
}

Expand All @@ -132,7 +132,7 @@ bool move(basic_cdr_stream& str, const T&, const entity_properties_t &, size_t N
* @param[in] N The number of entities at most to move.
*/
template<typename T, std::enable_if_t<std::is_enum<T>::value && !std::is_arithmetic<T>::value, bool> = true >
bool max(basic_cdr_stream& str, const T&, const entity_properties_t &, size_t N = 1) {
bool max(basic_cdr_stream& str, const T&, size_t N = 1) {
return max(str, uint32_t(0), N);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <list>
#include <map>
#include <mutex>
#include <type_traits>
#include "cdr_enums.hpp"

namespace org {
Expand All @@ -25,6 +26,8 @@ namespace cyclonedds {
namespace core {
namespace cdr {

#define decl_ref_type(x) std::remove_cv_t<std::remove_reference_t<decltype(x)>>

/**
* @brief
* Bit bound descriptors.
Expand Down Expand Up @@ -244,45 +247,39 @@ struct OMG_DDS_API final_entry: public entity_properties_t {
* @brief
* Type properties getter function for basic types.
*
* @return entity_properties_t "Tree" representing the type.
*/
template<typename T, std::enable_if_t<std::is_arithmetic<T>::value, bool> = true >
entity_properties_t get_type_props() {
entity_properties_t props;
static_assert(sizeof(T) == 1 || sizeof(T) == 2 || sizeof(T) == 4 || sizeof(T) == 8);
props.e_bb = bit_bound(sizeof(T));
return props;
}

/**
* @brief
* Forward declaration for type properties getter function.
*
* This template function is replaced/implemented by the types implemented through IDL generation.
* It generates a static container which is initialized the first time the function is called,
* this is then returned.
*
* @return entity_properties_t "Tree" representing the type.
*/
template<typename T>
entity_properties_t get_type_props() {
static std::mutex mtx;
static entity_properties_t props;
static bool initialized = false;
std::lock_guard<std::mutex> lock(mtx);

if (initialized)
return props;
template<typename T, std::enable_if_t<!std::is_arithmetic<T>::value, bool> = true >
entity_properties_t get_type_props();

props.clear();
key_endpoint keylist;
switch (sizeof(T)) {
case 1:
props.e_bb = bb_8_bits;
break;
case 2:
props.e_bb = bb_16_bits;
break;
case 4:
props.e_bb = bb_32_bits;
break;
case 8:
props.e_bb = bb_64_bits;
break;
}
props.m_members_by_seq.push_back(final_entry());
props.m_members_by_id.push_back(final_entry());
props.m_keys.push_back(final_entry());
props.finish(keylist);
initialized = true;
return props;
}
/**
* @brief
* Forward declaration for bit bound property of enum classes.
*
* This function is implementated by each enum class that is encountered.
*
* @return bit_bound The bit bound for the indicated enum.
*/
template<typename T, std::enable_if_t<std::is_enum<T>::value, bool> = true >
constexpr bit_bound get_enum_bit_bound();

}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,15 +224,14 @@ class OMG_DDS_API xcdr_v1_stream : public cdr_stream {
*
* @param[in, out] str The stream which is read from.
* @param[out] toread The variable to read into.
* @param[in] props The properties of the variable.
* @param[in] N The number of entities to read.
*
* @return Whether the operation was completed succesfully.
*/
template<typename T, std::enable_if_t<std::is_enum<T>::value && !std::is_arithmetic<T>::value, bool> = true >
bool read(xcdr_v1_stream& str, T& toread, const entity_properties_t &props, size_t N = 1)
bool read(xcdr_v1_stream& str, T& toread, size_t N = 1)
{
switch (str.is_key() ? bb_32_bits : props.e_bb)
switch (str.is_key() ? bb_32_bits : get_enum_bit_bound<T>())
{
case bb_8_bits:
return read_enum_impl<xcdr_v1_stream,T,uint8_t>(str, toread, N);
Expand All @@ -255,15 +254,14 @@ bool read(xcdr_v1_stream& str, T& toread, const entity_properties_t &props, size
*
* @param[in, out] str The stream which is written to.
* @param[in] towrite The variable to write.
* @param[in] props The properties of the variable.
* @param[in] N The number of entities to write.
*
* @return Whether the operation was completed succesfully.
*/
template<typename T, std::enable_if_t<std::is_enum<T>::value && !std::is_arithmetic<T>::value, bool> = true >
bool write(xcdr_v1_stream& str, const T& towrite, const entity_properties_t &props, size_t N = 1)
bool write(xcdr_v1_stream& str, const T& towrite, size_t N = 1)
{
switch (str.is_key() ? bb_32_bits : props.e_bb)
switch (str.is_key() ? bb_32_bits : get_enum_bit_bound<T>())
{
case bb_8_bits:
return write_enum_impl<xcdr_v1_stream,T,uint8_t>(str, towrite, N);
Expand All @@ -285,15 +283,14 @@ bool write(xcdr_v1_stream& str, const T& towrite, const entity_properties_t &pro
* Moves the cursor of the stream by the size the enum would take up.
*
* @param[in, out] str The stream whose cursor is moved.
* @param[in] props The properties of the variable.
* @param[in] N The number of entities to move.
*
* @return Whether the operation was completed succesfully.
*/
template<typename T, std::enable_if_t<std::is_enum<T>::value && !std::is_arithmetic<T>::value, bool> = true >
bool move(xcdr_v1_stream& str, const T&, const entity_properties_t &props, size_t N = 1)
bool move(xcdr_v1_stream& str, const T&, size_t N = 1)
{
switch (str.is_key() ? bb_32_bits : props.e_bb)
switch (str.is_key() ? bb_32_bits : get_enum_bit_bound<T>())
{
case bb_8_bits:
return move(str, int8_t(0), N);
Expand All @@ -316,15 +313,14 @@ bool move(xcdr_v1_stream& str, const T&, const entity_properties_t &props, size_
*
* @param[in, out] str The stream whose cursor is moved.
* @param[in] max_sz The variable to move the cursor by, no contents of this variable are used, it is just used to determine the template.
* @param[in] props The properties of the variable.
* @param[in] N The number of entities at most to move.
*
* @return Whether the operation was completed succesfully.
*/
template<typename T, std::enable_if_t<std::is_enum<T>::value && !std::is_arithmetic<T>::value, bool> = true >
bool max(xcdr_v1_stream& str, const T& max_sz, const entity_properties_t &props, size_t N = 1)
bool max(xcdr_v1_stream& str, const T& max_sz, size_t N = 1)
{
return move(str, max_sz, props, N);
return move(str, max_sz, N);
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -295,14 +295,13 @@ class OMG_DDS_API xcdr_v2_stream : public cdr_stream {
*
* @param[in, out] str The stream which is read from.
* @param[out] toread The variable to read into.
* @param[in] props The properties of the variable.
* @param[in] N The number of entities to read.
*
* @return Whether the operation was completed succesfully.
*/
template<typename T, std::enable_if_t<std::is_enum<T>::value && !std::is_arithmetic<T>::value, bool> = true >
bool read(xcdr_v2_stream& str, T& toread, const entity_properties_t &props, size_t N = 1) {
switch (str.is_key() ? bb_32_bits : props.e_bb)
bool read(xcdr_v2_stream& str, T& toread, size_t N = 1) {
switch (str.is_key() ? bb_32_bits : get_enum_bit_bound<T>())
{
case bb_8_bits:
return read_enum_impl<xcdr_v2_stream,T,uint8_t>(str, toread, N);
Expand All @@ -325,14 +324,13 @@ bool read(xcdr_v2_stream& str, T& toread, const entity_properties_t &props, size
*
* @param [in, out] str The stream which is written to.
* @param [in] towrite The variable to write.
* @param[in] props The properties of the variable.
* @param[in] N The number of entities to write.
*
* @return Whether the operation was completed succesfully.
*/
template<typename T, std::enable_if_t<std::is_enum<T>::value && !std::is_arithmetic<T>::value, bool> = true >
bool write(xcdr_v2_stream& str, const T& towrite, const entity_properties_t &props, size_t N = 1) {
switch (str.is_key() ? bb_32_bits : props.e_bb)
bool write(xcdr_v2_stream& str, const T& towrite, size_t N = 1) {
switch (str.is_key() ? bb_32_bits : get_enum_bit_bound<T>())
{
case bb_8_bits:
return write_enum_impl<xcdr_v2_stream,T,uint8_t>(str, towrite, N);
Expand All @@ -354,14 +352,13 @@ bool write(xcdr_v2_stream& str, const T& towrite, const entity_properties_t &pro
* Moves the cursor of the stream by the size the enum would take up.
*
* @param[in, out] str The stream whose cursor is moved.
* @param[in] props The properties of the variable.
* @param[in] N The number of entities to move.
*
* @return Whether the operation was completed succesfully.
*/
template<typename T, std::enable_if_t<std::is_enum<T>::value && !std::is_arithmetic<T>::value, bool> = true >
bool move(xcdr_v2_stream& str, const T&, const entity_properties_t &props, size_t N = 1) {
switch (str.is_key() ? bb_32_bits : props.e_bb)
bool move(xcdr_v2_stream& str, const T&, size_t N = 1) {
switch (str.is_key() ? bb_32_bits : get_enum_bit_bound<T>())
{
case bb_8_bits:
return move(str, int8_t(0), N);
Expand All @@ -384,14 +381,13 @@ bool move(xcdr_v2_stream& str, const T&, const entity_properties_t &props, size_
*
* @param[in, out] str The stream whose cursor is moved.
* @param[in] max_sz The variable to move the cursor by, no contents of this variable are used, it is just used to determine the template.
* @param[in] props The properties of the variable.
* @param[in] N The number of entities at most to move.
*
* @return Whether the operation was completed succesfully.
*/
template<typename T, std::enable_if_t<std::is_enum<T>::value && !std::is_arithmetic<T>::value, bool> = true >
bool max(xcdr_v2_stream& str, const T& max_sz, const entity_properties_t &props, size_t N = 1) {
return move(str, max_sz, props, N);
bool max(xcdr_v2_stream& str, const T& max_sz, size_t N = 1) {
return move(str, max_sz, N);
}

}
Expand Down
6 changes: 4 additions & 2 deletions src/ddscxx/tests/CDRStreamer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -353,10 +353,12 @@ TEST_F(CDRStreamer, cdr_sequence)
};

bytes SSM_xcdr_v1_normal {
0x40, 0x00, 0x00, 0x07 /*sequence_struct.c.mheader*/,
0x7F, 0x01, 0x00, 0x08 /*sequence_struct.c.mheader (pid_list_extended + length = 8)*/,
0x40, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x07 /*sequence_struct.c.mheader (extended)*/,
0x00, 0x00, 0x00, 0x03/*sequence_struct.c.length*/, 'z', 'y', 'x'/*sequence_struct.c.data*/,
0x00 /*padding bytes (1)*/,
0x00, 0x01, 0x00, 0x14 /*sequence_struct.c.mheader (extended)*/,
0x7F, 0x01, 0x00, 0x08 /*sequence_struct.l.mheader (pid_list_extended + length = 8)*/,
0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x14 /*sequence_struct.l.mheader (extended)*/,
0x00, 0x00, 0x00, 0x04/*sequence_struct.l.length*/, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x01/*sequence_struct.l.data*/,
0x7F, 0x02, 0x00, 0x00 /*inner list termination header*/
};
Expand Down
26 changes: 16 additions & 10 deletions src/ddscxx/tests/data/RegressionModels.idl
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,27 @@ union duplicate_types_union switch(long) {
td_d d_2; //should be the same as d_1;
};

@bit_bound(8) @appendable enum e1 {
e_0, e_1, e_2, e_3
};

typedef e1 e1_arr[3];

struct s_e1 {
e1_arr c;
};

union W switch(char) {
case 'a':
char c[2];
case 'b':
char d[2][3];
case 'x':
e1 x[3];
case 'y':
e1_arr y;
case 'z':
e1 z;
};

typedef long long_array_5[5];
Expand Down Expand Up @@ -119,14 +135,4 @@ struct s_bm1 {
bm1 c[3];
};

@bit_bound(8) @appendable enum e1 {
e_0, e_1, e_2, e_3
};

typedef e1 e1_arr[3];

struct s_e1 {
e1_arr c;
};

};
Loading

0 comments on commit 589bce0

Please sign in to comment.