From eafcfa173329edf1eb3251117c68c489524aacca Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Wed, 29 Apr 2015 03:08:07 +0300 Subject: [PATCH] Support for different DTID limits depending on data type kind; tests are failing now! --- libuavcan/include/uavcan/data_type.hpp | 35 +++++++++++++++++-- .../uavcan/node/global_data_type_registry.hpp | 3 +- libuavcan/include/uavcan/transport/frame.hpp | 2 +- .../src/node/uc_global_data_type_registry.cpp | 4 +-- .../protocol/uc_data_type_info_provider.cpp | 10 ++++-- libuavcan/src/transport/uc_frame.cpp | 2 +- libuavcan/src/uc_data_type.cpp | 28 ++++++++++++++- libuavcan/test/data_type.cpp | 13 +++++-- libuavcan/test/protocol/logger.cpp | 2 ++ libuavcan/test/transport/frame.cpp | 2 +- .../test/transport/transfer_receiver.cpp | 2 +- 11 files changed, 86 insertions(+), 17 deletions(-) diff --git a/libuavcan/include/uavcan/data_type.hpp b/libuavcan/include/uavcan/data_type.hpp index 323b6f8a18..f8f752ac8d 100644 --- a/libuavcan/include/uavcan/data_type.hpp +++ b/libuavcan/include/uavcan/data_type.hpp @@ -24,22 +24,49 @@ enum DataTypeKind }; +static inline DataTypeKind getDataTypeKindForTransferType(const TransferType tt) +{ + if (tt == TransferTypeServiceResponse || + tt == TransferTypeServiceRequest) + { + return DataTypeKindService; + } + else if (tt == TransferTypeMessageBroadcast || + tt == TransferTypeMessageUnicast) + { + return DataTypeKindMessage; + } + else + { + UAVCAN_ASSERT(0); + return DataTypeKind(0); + } +} + + class UAVCAN_EXPORT DataTypeID { uint16_t value_; public: - static const uint16_t Max = 1023; + static const uint16_t MaxServiceDataTypeIDValue = 511; + static const uint16_t MaxMessageDataTypeIDValue = 2047; + static const uint16_t MaxPossibleDataTypeIDValue = MaxMessageDataTypeIDValue; DataTypeID() : value_(0xFFFF) { } DataTypeID(uint16_t id) // Implicit : value_(id) { - UAVCAN_ASSERT(isValid()); + UAVCAN_ASSERT(id < 0xFFFF); } - bool isValid() const { return value_ <= Max; } + static DataTypeID getMaxValueForDataTypeKind(const DataTypeKind dtkind); + + bool isValidForDataTypeKind(DataTypeKind dtkind) const + { + return value_ <= getMaxValueForDataTypeKind(dtkind).get(); + } uint16_t get() const { return value_; } @@ -128,6 +155,8 @@ public: UAVCAN_ASSERT(std::strlen(name) <= MaxFullNameLen); } + bool isValid() const; + DataTypeKind getKind() const { return kind_; } DataTypeID getID() const { return id_; } const DataTypeSignature& getSignature() const { return signature_; } diff --git a/libuavcan/include/uavcan/node/global_data_type_registry.hpp b/libuavcan/include/uavcan/node/global_data_type_registry.hpp index d7f837c199..0f4550a19f 100644 --- a/libuavcan/include/uavcan/node/global_data_type_registry.hpp +++ b/libuavcan/include/uavcan/node/global_data_type_registry.hpp @@ -21,7 +21,7 @@ namespace uavcan /** * Bit mask where bit at index X is set if there's a Data Type with ID X. */ -typedef BitSet DataTypeIDMask; +typedef BitSet DataTypeIDMask; /** * This singleton is shared among all existing node instances. It is instantiated automatically @@ -153,6 +153,7 @@ public: /** * Computes Aggregate Signature for all known data types selected by the mask. + * Extra bits will be zeroed. * Please read the DSDL specification. * @param[in] kind Data Type Kind - messages or services. * @param[inout] inout_id_mask Data types to compute aggregate signature for; bits at diff --git a/libuavcan/include/uavcan/transport/frame.hpp b/libuavcan/include/uavcan/transport/frame.hpp index 5d149ba837..d93bf997a8 100644 --- a/libuavcan/include/uavcan/transport/frame.hpp +++ b/libuavcan/include/uavcan/transport/frame.hpp @@ -49,7 +49,7 @@ public: , last_frame_(last_frame) { UAVCAN_ASSERT((transfer_type == TransferTypeMessageBroadcast) == dst_node_id.isBroadcast()); - UAVCAN_ASSERT(data_type_id.isValid()); + UAVCAN_ASSERT(data_type_id.isValidForDataTypeKind(getDataTypeKindForTransferType(transfer_type))); UAVCAN_ASSERT(src_node_id.isUnicast() ? (src_node_id != dst_node_id) : true); UAVCAN_ASSERT(frame_index <= MaxIndex); } diff --git a/libuavcan/src/node/uc_global_data_type_registry.cpp b/libuavcan/src/node/uc_global_data_type_registry.cpp index aa96308d0d..f882348a81 100644 --- a/libuavcan/src/node/uc_global_data_type_registry.cpp +++ b/libuavcan/src/node/uc_global_data_type_registry.cpp @@ -61,7 +61,7 @@ GlobalDataTypeRegistry::RegistrationResult GlobalDataTypeRegistry::remove(Entry* GlobalDataTypeRegistry::RegistrationResult GlobalDataTypeRegistry::registImpl(Entry* dtd) { - if (!dtd || (dtd->descriptor.getID() > DataTypeID::Max)) + if (!dtd || !dtd->descriptor.isValid()) { UAVCAN_ASSERT(0); return RegistrationResultInvalidParams; @@ -241,7 +241,7 @@ DataTypeSignature GlobalDataTypeRegistry::computeAggregateSignature(DataTypeKind p = p->getNextListNode(); } prev_dtid++; - while (prev_dtid <= DataTypeID::Max) + while (unsigned(prev_dtid) < inout_id_mask.size()) { inout_id_mask[unsigned(prev_dtid++)] = false; } diff --git a/libuavcan/src/protocol/uc_data_type_info_provider.cpp b/libuavcan/src/protocol/uc_data_type_info_provider.cpp index c736d06842..22dfd2b7a4 100644 --- a/libuavcan/src/protocol/uc_data_type_info_provider.cpp +++ b/libuavcan/src/protocol/uc_data_type_info_provider.cpp @@ -17,17 +17,21 @@ void DataTypeInfoProvider::handleComputeAggregateTypeSignatureRequest( const protocol::ComputeAggregateTypeSignature::Request& request, protocol::ComputeAggregateTypeSignature::Response& response) { - const DataTypeKind kind = DataTypeKind(request.kind.value); + const DataTypeKind kind = DataTypeKind(request.kind.value); // No mapping needed if (!isValidDataTypeKind(kind)) { UAVCAN_TRACE("DataTypeInfoProvider", - "ComputeAggregateTypeSignature request with invalid DataTypeKind %i", kind); + "ComputeAggregateTypeSignature request with invalid DataTypeKind %d", kind); return; } - UAVCAN_TRACE("DataTypeInfoProvider", "ComputeAggregateTypeSignature request for dtk=%i", int(request.kind.value)); + UAVCAN_TRACE("DataTypeInfoProvider", "ComputeAggregateTypeSignature request for dtk=%d, len(known_ids)=%d", + int(request.kind.value), int(request.known_ids.size())); + // Correcting the mask length according to the data type kind response.mutually_known_ids = request.known_ids; + response.mutually_known_ids.resize(static_cast(DataTypeID::getMaxValueForDataTypeKind(kind).get() + 1U)); + response.aggregate_signature = GlobalDataTypeRegistry::instance().computeAggregateSignature(kind, response.mutually_known_ids).get(); } diff --git a/libuavcan/src/transport/uc_frame.cpp b/libuavcan/src/transport/uc_frame.cpp index 3a6f6ac814..412e31df71 100644 --- a/libuavcan/src/transport/uc_frame.cpp +++ b/libuavcan/src/transport/uc_frame.cpp @@ -179,7 +179,7 @@ bool Frame::isValid() const ((transfer_type_ == TransferTypeMessageBroadcast) != dst_node_id_.isBroadcast()) || (transfer_type_ >= NumTransferTypes) || (static_cast(payload_len_) > getMaxPayloadLen()) || - (!data_type_id_.isValid()); + (!data_type_id_.isValidForDataTypeKind(getDataTypeKindForTransferType(transfer_type_))); return !invalid; } diff --git a/libuavcan/src/uc_data_type.cpp b/libuavcan/src/uc_data_type.cpp index 184d6dc610..ec9e648183 100644 --- a/libuavcan/src/uc_data_type.cpp +++ b/libuavcan/src/uc_data_type.cpp @@ -12,7 +12,26 @@ namespace uavcan /* * DataTypeID */ -const uint16_t DataTypeID::Max; +const uint16_t DataTypeID::MaxServiceDataTypeIDValue; +const uint16_t DataTypeID::MaxMessageDataTypeIDValue; +const uint16_t DataTypeID::MaxPossibleDataTypeIDValue; + +DataTypeID DataTypeID::getMaxValueForDataTypeKind(const DataTypeKind dtkind) +{ + if (dtkind == DataTypeKindService) + { + return MaxServiceDataTypeIDValue; + } + else if (dtkind == DataTypeKindMessage) + { + return MaxMessageDataTypeIDValue; + } + else + { + UAVCAN_ASSERT(0); + return DataTypeID(0); + } +} /* * DataTypeSignatureCRC @@ -78,6 +97,13 @@ TransferCRC DataTypeSignature::toTransferCRC() const */ const unsigned DataTypeDescriptor::MaxFullNameLen; +bool DataTypeDescriptor::isValid() const +{ + return id_.isValidForDataTypeKind(kind_) && + (full_name_ != NULL) && + (*full_name_ != '\0'); +} + bool DataTypeDescriptor::match(DataTypeKind kind, const char* name) const { return (kind_ == kind) && !std::strncmp(full_name_, name, MaxFullNameLen); diff --git a/libuavcan/test/data_type.cpp b/libuavcan/test/data_type.cpp index 4d849076eb..f10c7c77c4 100644 --- a/libuavcan/test/data_type.cpp +++ b/libuavcan/test/data_type.cpp @@ -129,7 +129,8 @@ TEST(DataTypeID, Basic) uavcan::DataTypeID id; ASSERT_EQ(0xFFFF, id.get()); - ASSERT_FALSE(id.isValid()); + ASSERT_FALSE(id.isValidForDataTypeKind(uavcan::DataTypeKindMessage)); + ASSERT_FALSE(id.isValidForDataTypeKind(uavcan::DataTypeKindService)); id = 123; uavcan::DataTypeID id2 = 456; @@ -137,8 +138,10 @@ TEST(DataTypeID, Basic) ASSERT_EQ(123, id.get()); ASSERT_EQ(456, id2.get()); - ASSERT_TRUE(id.isValid()); - ASSERT_TRUE(id2.isValid()); + ASSERT_TRUE(id.isValidForDataTypeKind(uavcan::DataTypeKindMessage)); + ASSERT_TRUE(id.isValidForDataTypeKind(uavcan::DataTypeKindService)); + ASSERT_TRUE(id2.isValidForDataTypeKind(uavcan::DataTypeKindMessage)); + ASSERT_TRUE(id2.isValidForDataTypeKind(uavcan::DataTypeKindService)); ASSERT_TRUE(id < id2); ASSERT_TRUE(id <= id2); @@ -152,4 +155,8 @@ TEST(DataTypeID, Basic) ASSERT_FALSE(id2 > id); ASSERT_TRUE(id2 >= id); ASSERT_TRUE(id == id2); + + id = 1024; + ASSERT_TRUE(id.isValidForDataTypeKind(uavcan::DataTypeKindMessage)); + ASSERT_FALSE(id.isValidForDataTypeKind(uavcan::DataTypeKindService)); } diff --git a/libuavcan/test/protocol/logger.cpp b/libuavcan/test/protocol/logger.cpp index 41a665a5f8..1d38cec64b 100644 --- a/libuavcan/test/protocol/logger.cpp +++ b/libuavcan/test/protocol/logger.cpp @@ -87,6 +87,7 @@ TEST(Logger, Basic) ASSERT_LE(0, logger.logError("foo", "Error")); nodes.spinBoth(uavcan::MonotonicDuration::fromMSec(10)); + ASSERT_TRUE(log_sub.collector.msg.get()); ASSERT_EQ(log_sub.collector.msg->level.value, uavcan::protocol::debug::LogLevel::ERROR); ASSERT_EQ(log_sub.collector.msg->source, "foo"); ASSERT_EQ(log_sub.collector.msg->text, "Error"); @@ -134,6 +135,7 @@ TEST(Logger, Cpp11Formatting) ASSERT_LE(0, logger.logWarning("foo", "char='%*', %* is %*", '$', "double", 12.34)); nodes.spinBoth(uavcan::MonotonicDuration::fromMSec(10)); + ASSERT_TRUE(log_sub.collector.msg.get()); ASSERT_EQ(log_sub.collector.msg->level.value, uavcan::protocol::debug::LogLevel::WARNING); ASSERT_EQ(log_sub.collector.msg->source, "foo"); ASSERT_EQ(log_sub.collector.msg->text, "char='$', double is 12.34"); diff --git a/libuavcan/test/transport/frame.cpp b/libuavcan/test/transport/frame.cpp index 719dcf325b..bb54554fc9 100644 --- a/libuavcan/test/transport/frame.cpp +++ b/libuavcan/test/transport/frame.cpp @@ -230,7 +230,7 @@ TEST(Frame, FrameToString) rx_frame.toString()); // RX frame max len - rx_frame = RxFrame(Frame(uavcan::DataTypeID::Max, uavcan::TransferTypeMessageUnicast, + rx_frame = RxFrame(Frame(uavcan::DataTypeID::MaxPossibleDataTypeIDValue, uavcan::TransferTypeMessageUnicast, uavcan::NodeID::Max, uavcan::NodeID::Max - 1, Frame::MaxIndex, uavcan::TransferID::Max, true), uavcan::MonotonicTime::getMax(), uavcan::UtcTime::getMax(), 3); diff --git a/libuavcan/test/transport/transfer_receiver.cpp b/libuavcan/test/transport/transfer_receiver.cpp index e6c379d39e..fb888ec0d1 100644 --- a/libuavcan/test/transport/transfer_receiver.cpp +++ b/libuavcan/test/transport/transfer_receiver.cpp @@ -428,7 +428,7 @@ TEST(TransferReceiver, UtcTransferTimestamping) TEST(TransferReceiver, HeaderParsing) { Context<32> context; - RxFrameGenerator gen(789); + RxFrameGenerator gen(123); uavcan::TransferReceiver& rcv = context.receiver; uavcan::ITransferBufferManager& bufmgr = context.bufmgr;