From 62ea9e27467b4e5789bbede3e53be0d1fe613eef Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Tue, 11 Mar 2014 21:29:35 +0400 Subject: [PATCH] Type safe DataTypeID class --- libuavcan/include/uavcan/data_type.hpp | 41 +++++++++++++++---- .../uavcan/global_data_type_registry.hpp | 8 ++-- .../uavcan/internal/transport/dispatcher.hpp | 4 +- .../uavcan/internal/transport/frame.hpp | 9 ++-- .../transport/outgoing_transfer_registry.hpp | 7 ++-- libuavcan/src/data_type.cpp | 5 ++- libuavcan/src/global_data_type_registry.cpp | 19 +++++---- libuavcan/src/internal/transport/frame.cpp | 8 ++-- libuavcan/test/data_type.cpp | 2 +- libuavcan/test/internal/transport/frame.cpp | 14 +++---- .../transport/transfer_test_helpers.hpp | 2 +- 11 files changed, 73 insertions(+), 46 deletions(-) diff --git a/libuavcan/include/uavcan/data_type.hpp b/libuavcan/include/uavcan/data_type.hpp index 7c54db98df..fe276aa9f7 100644 --- a/libuavcan/include/uavcan/data_type.hpp +++ b/libuavcan/include/uavcan/data_type.hpp @@ -23,6 +23,36 @@ enum DataTypeKind NumDataTypeKinds }; + +class DataTypeID +{ + uint16_t value_; + +public: + enum { Max = 1023 }; + + DataTypeID() : value_(0xFFFF) { } + + DataTypeID(uint16_t id) + : value_(id) + { + assert(isValid()); + } + + bool isValid() const { return value_ <= Max; } + + uint16_t get() const { return value_; } + + bool operator==(DataTypeID rhs) const { return value_ == rhs.value_; } + bool operator!=(DataTypeID rhs) const { return value_ != rhs.value_; } + + bool operator<(DataTypeID rhs) const { return value_ < rhs.value_; } + bool operator>(DataTypeID rhs) const { return value_ > rhs.value_; } + bool operator<=(DataTypeID rhs) const { return value_ <= rhs.value_; } + bool operator>=(DataTypeID rhs) const { return value_ >= rhs.value_; } +}; + + /** * CRC-64-WE * Description: http://reveng.sourceforge.net/crc-catalogue/17plus.htm#crc.cat-bits.64 @@ -89,39 +119,36 @@ public: class DataTypeDescriptor { DataTypeKind kind_; - uint16_t id_; + DataTypeID id_; DataTypeSignature signature_; const char* full_name_; public: - enum { MaxDataTypeID = 1023 }; enum { MaxFullNameLen = 80 }; DataTypeDescriptor() : kind_(DataTypeKind(0)) - , id_(0) , full_name_("") { } - DataTypeDescriptor(DataTypeKind kind, uint16_t id, const DataTypeSignature& signature, const char* name) + DataTypeDescriptor(DataTypeKind kind, DataTypeID id, const DataTypeSignature& signature, const char* name) : kind_(kind) , id_(id) , signature_(signature) , full_name_(name) { - assert(id <= MaxDataTypeID); assert(kind < NumDataTypeKinds); assert(name); assert(std::strlen(name) <= MaxFullNameLen); } DataTypeKind getKind() const { return kind_; } - uint16_t getID() const { return id_; } + DataTypeID getID() const { return id_; } const DataTypeSignature& getSignature() const { return signature_; } const char* getFullName() const { return full_name_; } bool match(DataTypeKind kind, const char* name) const; - bool match(DataTypeKind kind, uint16_t id) const; + bool match(DataTypeKind kind, DataTypeID id) const; std::string toString() const; diff --git a/libuavcan/include/uavcan/global_data_type_registry.hpp b/libuavcan/include/uavcan/global_data_type_registry.hpp index 5ef9672ad7..1bd376427c 100644 --- a/libuavcan/include/uavcan/global_data_type_registry.hpp +++ b/libuavcan/include/uavcan/global_data_type_registry.hpp @@ -19,7 +19,7 @@ namespace uavcan { -typedef std::bitset DataTypeIDMask; +typedef std::bitset DataTypeIDMask; class GlobalDataTypeRegistry : Noncopyable { @@ -29,14 +29,14 @@ class GlobalDataTypeRegistry : Noncopyable Entry() { } - Entry(DataTypeKind kind, uint16_t id, const DataTypeSignature& signature, const char* name) + Entry(DataTypeKind kind, DataTypeID id, const DataTypeSignature& signature, const char* name) : descriptor(kind, id, signature, name) { } }; struct EntryInsertionComparator { - const uint16_t id; + const DataTypeID id; EntryInsertionComparator(Entry* dtd) : id(dtd->descriptor.getID()) { } bool operator()(const Entry* entry) const { @@ -73,7 +73,7 @@ public: /// Last call wins template - RegistResult regist(uint16_t id) + RegistResult regist(DataTypeID id) { if (isFrozen()) return RegistResultFrozen; diff --git a/libuavcan/include/uavcan/internal/transport/dispatcher.hpp b/libuavcan/include/uavcan/internal/transport/dispatcher.hpp index b8694d53d0..bde6fa905d 100644 --- a/libuavcan/include/uavcan/internal/transport/dispatcher.hpp +++ b/libuavcan/include/uavcan/internal/transport/dispatcher.hpp @@ -26,9 +26,9 @@ class Dispatcher : Noncopyable class DataTypeIDInsertionComparator { - const uint16_t id_; + const DataTypeID id_; public: - DataTypeIDInsertionComparator(uint16_t id) : id_(id) { } + DataTypeIDInsertionComparator(DataTypeID id) : id_(id) { } bool operator()(const TransferListenerBase* listener) const { assert(listener); diff --git a/libuavcan/include/uavcan/internal/transport/frame.hpp b/libuavcan/include/uavcan/internal/transport/frame.hpp index 235a215607..6098cad3aa 100644 --- a/libuavcan/include/uavcan/internal/transport/frame.hpp +++ b/libuavcan/include/uavcan/internal/transport/frame.hpp @@ -20,7 +20,7 @@ class Frame { uint8_t payload_[sizeof(CanFrame::data)]; TransferType transfer_type_; - uint_fast16_t data_type_id_; + DataTypeID data_type_id_; uint_fast8_t payload_len_; NodeID src_node_id_; NodeID dst_node_id_; @@ -33,14 +33,13 @@ public: Frame() : transfer_type_(TransferType(NumTransferTypes)) // That is invalid value - , data_type_id_(0) , payload_len_(0) , frame_index_(0) , transfer_id_(0) , last_frame_(false) { } - Frame(uint_fast16_t data_type_id, TransferType transfer_type, NodeID src_node_id, NodeID dst_node_id, + Frame(DataTypeID data_type_id, TransferType transfer_type, NodeID src_node_id, NodeID dst_node_id, uint_fast8_t frame_index, TransferID transfer_id, bool last_frame = false) : transfer_type_(transfer_type) , data_type_id_(data_type_id) @@ -52,7 +51,7 @@ public: , last_frame_(last_frame) { assert((transfer_type == TransferTypeMessageBroadcast) == dst_node_id.isBroadcast()); - assert(data_type_id <= DataTypeDescriptor::MaxDataTypeID); + assert(data_type_id.isValid()); assert(src_node_id != dst_node_id); assert(frame_index <= MaxIndex); } @@ -64,7 +63,7 @@ public: const uint8_t* getPayloadPtr() const { return payload_; } TransferType getTransferType() const { return transfer_type_; } - uint_fast16_t getDataTypeID() const { return data_type_id_; } + DataTypeID getDataTypeID() const { return data_type_id_; } NodeID getSrcNodeID() const { return src_node_id_; } NodeID getDstNodeID() const { return dst_node_id_; } TransferID getTransferID() const { return transfer_id_; } diff --git a/libuavcan/include/uavcan/internal/transport/outgoing_transfer_registry.hpp b/libuavcan/include/uavcan/internal/transport/outgoing_transfer_registry.hpp index 1778654cdd..2ef04964f9 100644 --- a/libuavcan/include/uavcan/internal/transport/outgoing_transfer_registry.hpp +++ b/libuavcan/include/uavcan/internal/transport/outgoing_transfer_registry.hpp @@ -17,17 +17,16 @@ namespace uavcan UAVCAN_PACKED_BEGIN class OutgoingTransferRegistryKey { - uint16_t data_type_id_; + DataTypeID data_type_id_; uint8_t transfer_type_; NodeID destination_node_id_; ///< Not applicable for message broadcasting public: OutgoingTransferRegistryKey() - : data_type_id_(0xFFFF) - , transfer_type_(0xFF) + : transfer_type_(0xFF) { } - OutgoingTransferRegistryKey(uint16_t data_type_id, TransferType transfer_type, NodeID destination_node_id) + OutgoingTransferRegistryKey(DataTypeID data_type_id, TransferType transfer_type, NodeID destination_node_id) : data_type_id_(data_type_id) , transfer_type_(transfer_type) , destination_node_id_(destination_node_id) diff --git a/libuavcan/src/data_type.cpp b/libuavcan/src/data_type.cpp index 14cd939424..a76330bea9 100644 --- a/libuavcan/src/data_type.cpp +++ b/libuavcan/src/data_type.cpp @@ -45,7 +45,7 @@ bool DataTypeDescriptor::match(DataTypeKind kind, const char* name) const return (kind_ == kind) && !std::strcmp(full_name_, name); } -bool DataTypeDescriptor::match(DataTypeKind kind, uint16_t id) const +bool DataTypeDescriptor::match(DataTypeKind kind, DataTypeID id) const { return (kind_ == kind) && (id_ == id); } @@ -61,7 +61,8 @@ std::string DataTypeDescriptor::toString() const } std::ostringstream os; - os << full_name_ << ":" << id_ << kindch << ":" << std::hex << std::setfill('0') << std::setw(16) << signature_.get(); + os << full_name_ << ":" << id_.get() << kindch << ":" << std::hex + << std::setfill('0') << std::setw(16) << signature_.get(); return os.str(); } diff --git a/libuavcan/src/global_data_type_registry.cpp b/libuavcan/src/global_data_type_registry.cpp index cdec7e59ab..10e5831257 100644 --- a/libuavcan/src/global_data_type_registry.cpp +++ b/libuavcan/src/global_data_type_registry.cpp @@ -62,7 +62,7 @@ GlobalDataTypeRegistry::RegistResult GlobalDataTypeRegistry::remove(Entry* dtd) GlobalDataTypeRegistry::RegistResult GlobalDataTypeRegistry::registImpl(Entry* dtd) { - if (!dtd || (dtd->descriptor.getID() > DataTypeDescriptor::MaxDataTypeID)) + if (!dtd || (dtd->descriptor.getID() > DataTypeID::Max)) { assert(0); return RegistResultInvalidParams; @@ -93,12 +93,12 @@ GlobalDataTypeRegistry::RegistResult GlobalDataTypeRegistry::registImpl(Entry* d int id = -1; while (p) { - if (id >= p->descriptor.getID()) + if (id >= p->descriptor.getID().get()) { assert(0); std::abort(); } - id = p->descriptor.getID(); + id = p->descriptor.getID().get(); p = p->getNextListNode(); } } @@ -158,8 +158,9 @@ DataTypeSignature GlobalDataTypeRegistry::computeAggregateSignature(DataTypeKind while (p) { const DataTypeDescriptor& desc = p->descriptor; + const int dtid = desc.getID().get(); - if (inout_id_mask[desc.getID()]) + if (inout_id_mask[dtid]) { if (signature_initialized) signature.extend(desc.getSignature()); @@ -168,16 +169,16 @@ DataTypeSignature GlobalDataTypeRegistry::computeAggregateSignature(DataTypeKind signature_initialized = true; } - assert(prev_dtid < desc.getID()); // Making sure that list is ordered properly + assert(prev_dtid < dtid); // Making sure that list is ordered properly prev_dtid++; - while (prev_dtid < desc.getID()) + while (prev_dtid < dtid) inout_id_mask[prev_dtid++] = false; // Erasing bits for missing types - assert(prev_dtid == desc.getID()); + assert(prev_dtid == dtid); p = p->getNextListNode(); } prev_dtid++; - while (prev_dtid <= DataTypeDescriptor::MaxDataTypeID) + while (prev_dtid <= DataTypeID::Max) inout_id_mask[prev_dtid++] = false; return signature; @@ -196,7 +197,7 @@ void GlobalDataTypeRegistry::getDataTypeIDMask(DataTypeKind kind, DataTypeIDMask while (p) { assert(p->descriptor.getKind() == kind); - mask[p->descriptor.getID()] = true; + mask[p->descriptor.getID().get()] = true; p = p->getNextListNode(); } } diff --git a/libuavcan/src/internal/transport/frame.cpp b/libuavcan/src/internal/transport/frame.cpp index dc3bd8068e..89e82483d9 100644 --- a/libuavcan/src/internal/transport/frame.cpp +++ b/libuavcan/src/internal/transport/frame.cpp @@ -122,7 +122,7 @@ bool Frame::compile(CanFrame& out_can_frame) const bitpack<4, 6>(frame_index_) | bitpack<10, 7>(src_node_id_.get()) | bitpack<17, 2>(transfer_type_) | - bitpack<19, 10>(data_type_id_); + bitpack<19, 10>(data_type_id_.get()); switch (transfer_type_) { @@ -159,7 +159,7 @@ bool Frame::isValid() const ((transfer_type_ == TransferTypeMessageBroadcast) != dst_node_id_.isBroadcast()) || (transfer_type_ >= NumTransferTypes) || (payload_len_ > getMaxPayloadLen()) || - (data_type_id_ > DataTypeDescriptor::MaxDataTypeID); + (!data_type_id_.isValid()); return !invalid; } @@ -192,8 +192,8 @@ std::string Frame::toString() const static const int BUFLEN = 100; char buf[BUFLEN]; int ofs = std::snprintf(buf, BUFLEN, "dtid=%i tt=%i snid=%i dnid=%i idx=%i last=%i tid=%i payload=[", - int(data_type_id_), int(transfer_type_), int(src_node_id_.get()), int(dst_node_id_.get()), - int(frame_index_), int(last_frame_), int(transfer_id_.get())); + int(data_type_id_.get()), int(transfer_type_), int(src_node_id_.get()), + int(dst_node_id_.get()), int(frame_index_), int(last_frame_), int(transfer_id_.get())); for (int i = 0; i < payload_len_; i++) { diff --git a/libuavcan/test/data_type.cpp b/libuavcan/test/data_type.cpp index 15fd79d203..5c907ef1e8 100644 --- a/libuavcan/test/data_type.cpp +++ b/libuavcan/test/data_type.cpp @@ -96,7 +96,7 @@ TEST(DataTypeSignature, Correctness) TEST(DataTypeDescriptor, ToString) { uavcan::DataTypeDescriptor desc; - ASSERT_EQ(":0s:0000000000000000", desc.toString()); + ASSERT_EQ(":65535s:0000000000000000", desc.toString()); desc = uavcan::DataTypeDescriptor(uavcan::DataTypeKindMessage, 123, uavcan::DataTypeSignature(0xdeadbeef1234), "Bar"); diff --git a/libuavcan/test/internal/transport/frame.cpp b/libuavcan/test/internal/transport/frame.cpp index c5d66cc351..ed32ce994a 100644 --- a/libuavcan/test/internal/transport/frame.cpp +++ b/libuavcan/test/internal/transport/frame.cpp @@ -43,7 +43,7 @@ TEST(Frame, FrameParseCompile) EXPECT_EQ(29, frame.getIndex()); EXPECT_EQ(uavcan::NodeID(42), frame.getSrcNodeID()); EXPECT_EQ(uavcan::TransferTypeMessageBroadcast, frame.getTransferType()); - EXPECT_EQ(456, frame.getDataTypeID()); + EXPECT_EQ(456, frame.getDataTypeID().get()); EXPECT_EQ(payload_string.length(), frame.getPayloadLen()); EXPECT_TRUE(std::equal(frame.getPayloadPtr(), frame.getPayloadPtr() + frame.getPayloadLen(), @@ -100,7 +100,7 @@ TEST(Frame, FrameParsing) ASSERT_EQ(0, frame.getIndex()); ASSERT_EQ(NodeID(42), frame.getSrcNodeID()); ASSERT_EQ(NodeID::Broadcast, frame.getDstNodeID()); - ASSERT_EQ(456, frame.getDataTypeID()); + ASSERT_EQ(456, frame.getDataTypeID().get()); ASSERT_EQ(TransferID(2), frame.getTransferID()); ASSERT_EQ(uavcan::TransferTypeMessageBroadcast, frame.getTransferType()); ASSERT_EQ(sizeof(CanFrame::data), frame.getMaxPayloadLen()); @@ -125,7 +125,7 @@ TEST(Frame, FrameParsing) ASSERT_EQ(0, frame.getIndex()); ASSERT_EQ(NodeID(42), frame.getSrcNodeID()); ASSERT_EQ(NodeID(127), frame.getDstNodeID()); - ASSERT_EQ(456, frame.getDataTypeID()); + ASSERT_EQ(456, frame.getDataTypeID().get()); ASSERT_EQ(TransferID(2), frame.getTransferID()); ASSERT_EQ(uavcan::TransferTypeMessageUnicast, frame.getTransferType()); ASSERT_EQ(sizeof(CanFrame::data) - 1, frame.getMaxPayloadLen()); @@ -199,7 +199,7 @@ TEST(Frame, RxFrameParse) ASSERT_TRUE(rx_frame.parse(can_rx_frame)); ASSERT_EQ(123, rx_frame.getMonotonicTimestamp().toUSec()); ASSERT_EQ(2, rx_frame.getIfaceIndex()); - ASSERT_EQ(456, rx_frame.getDataTypeID()); + ASSERT_EQ(456, rx_frame.getDataTypeID().get()); ASSERT_EQ(uavcan::TransferTypeMessageBroadcast, rx_frame.getTransferType()); } @@ -211,10 +211,10 @@ TEST(Frame, FrameToString) // RX frame default RxFrame rx_frame; - EXPECT_EQ("dtid=0 tt=4 snid=255 dnid=255 idx=0 last=0 tid=0 payload=[] ts_m=0.000000 ts_utc=0.000000 iface=0", rx_frame.toString()); + EXPECT_EQ("dtid=65535 tt=4 snid=255 dnid=255 idx=0 last=0 tid=0 payload=[] ts_m=0.000000 ts_utc=0.000000 iface=0", rx_frame.toString()); // RX frame max len - rx_frame = RxFrame(Frame(uavcan::DataTypeDescriptor::MaxDataTypeID, uavcan::TransferTypeMessageUnicast, + rx_frame = RxFrame(Frame(uavcan::DataTypeID::Max, uavcan::TransferTypeMessageUnicast, uavcan::NodeID::Max, uavcan::NodeID::Max - 1, Frame::MaxIndex, uavcan::TransferID::Max, true), uavcan::MonotonicTime::getMax(), uavcan::UtcTime::getMax(), 3); @@ -230,7 +230,7 @@ TEST(Frame, FrameToString) // Plain frame default Frame frame; - EXPECT_EQ("dtid=0 tt=4 snid=255 dnid=255 idx=0 last=0 tid=0 payload=[]", frame.toString()); + EXPECT_EQ("dtid=65535 tt=4 snid=255 dnid=255 idx=0 last=0 tid=0 payload=[]", frame.toString()); // Plain frame max len frame = rx_frame; diff --git a/libuavcan/test/internal/transport/transfer_test_helpers.hpp b/libuavcan/test/internal/transport/transfer_test_helpers.hpp index 930b9061ce..d2853fca01 100644 --- a/libuavcan/test/internal/transport/transfer_test_helpers.hpp +++ b/libuavcan/test/internal/transport/transfer_test_helpers.hpp @@ -98,7 +98,7 @@ struct Transfer << " tid=" << int(transfer_id.get()) << " snid=" << int(src_node_id.get()) << " dnid=" << int(dst_node_id.get()) - << " dtid=" << int(data_type.getID()) + << " dtid=" << int(data_type.getID().get()) << "\n\t'" << payload << "'"; return os.str(); }