diff --git a/libuavcan/include/uavcan/error.hpp b/libuavcan/include/uavcan/error.hpp index db9b3992e8..4f67e9a6ec 100644 --- a/libuavcan/include/uavcan/error.hpp +++ b/libuavcan/include/uavcan/error.hpp @@ -35,7 +35,6 @@ const int16_t ErrLogic = 10; const int16_t ErrPassiveMode = 11; ///< Operation not permitted in passive mode const int16_t ErrTransferTooLong = 12; ///< Transfer of this length cannot be sent with given transfer type const int16_t ErrInvalidConfiguration = 13; -const int16_t ErrBufferTooSmall = 14; ///< Statically allocated buffer is not large enough /** * @} */ diff --git a/libuavcan/include/uavcan/node/generic_subscriber.hpp b/libuavcan/include/uavcan/node/generic_subscriber.hpp index 5b958f0a97..b749f8614e 100644 --- a/libuavcan/include/uavcan/node/generic_subscriber.hpp +++ b/libuavcan/include/uavcan/node/generic_subscriber.hpp @@ -33,7 +33,7 @@ namespace uavcan template class UAVCAN_EXPORT ReceivedDataStructure : public DataType_ { - const IncomingTransfer* _transfer_; ///< Such weird name is necessary to avoid clashing with DataType fields + const IncomingTransfer* const _transfer_; ///< Such weird name is necessary to avoid clashing with DataType fields template Ret safeget() const @@ -46,12 +46,14 @@ class UAVCAN_EXPORT ReceivedDataStructure : public DataType_ } protected: - ReceivedDataStructure() : _transfer_(NULL) { } + ReceivedDataStructure() + : _transfer_(NULL) + { } - void setTransfer(const IncomingTransfer* arg_transfer) + ReceivedDataStructure(const IncomingTransfer* arg_transfer) + : _transfer_(arg_transfer) { UAVCAN_ASSERT(arg_transfer != NULL); - _transfer_ = arg_transfer; } public: @@ -169,63 +171,16 @@ class UAVCAN_EXPORT GenericSubscriber : public GenericSubscriberBase int genericStart(bool (Dispatcher::*registration_method)(TransferListenerBase*)); - /* - * Received object and its allocators - */ +protected: struct ReceivedDataStructureSpec : public ReceivedDataStructure { - using ReceivedDataStructure::setTransfer; + ReceivedDataStructureSpec() { } + + ReceivedDataStructureSpec(const IncomingTransfer* arg_transfer) + : ReceivedDataStructure(arg_transfer) + { } }; - class StructBufferStatic - { - ReceivedDataStructureSpec object_; - - public: - explicit StructBufferStatic(GenericSubscriber&) { } - - ReceivedDataStructureSpec* getObjectPtr() { return &object_; } - }; - - class StructBufferShared - { - ReceivedDataStructureSpec* object_; - - public: - explicit StructBufferShared(GenericSubscriber& owner) - : object_(NULL) - { - IMarshalBuffer* const buf = - owner.getNode().getMarshalBufferProvider().getBuffer(sizeof(ReceivedDataStructureSpec)); - if (buf != NULL && buf->getSize() >= sizeof(ReceivedDataStructureSpec)) - { - object_ = new (buf->getDataPtr()) ReceivedDataStructureSpec; - } - } - - /* - * Destructor MUST NOT be invoked - * TODO explain why - */ - - ReceivedDataStructureSpec* getObjectPtr() { return object_; } - }; - -protected: - /** - * This value defines the threshold for stack allocation. See the typedef below. - */ - enum { MaxObjectSizeForStackAllocation = 80 }; - - /** - * This type resolves to either type of buffer, depending on the object size: - * - if the object is small, it will be allocated on the stack (StructBufferStatic); - * - if the object is large, it will be allocated in the shared buffer (StructBufferShared). - */ - typedef typename Select<(sizeof(DataStruct) > MaxObjectSizeForStackAllocation), - StructBufferShared, - StructBufferStatic>::Result ReceivedDataStructureBuffer; - explicit GenericSubscriber(INode& node) : GenericSubscriberBase(node) { } @@ -289,18 +244,6 @@ int GenericSubscriber::checkInit() return 0; } - /* - * Making sure that the buffer is large enough - * This check MUST be performed BEFORE initialization, otherwise we may encounter some terrible runtime failures. - */ - if (ReceivedDataStructureBuffer(*this).getObjectPtr() == NULL) - { - return -ErrBufferTooSmall; - } - - /* - * Initializing the transfer forwarder - */ GlobalDataTypeRegistry::instance().freeze(); const DataTypeDescriptor* const descr = GlobalDataTypeRegistry::instance().find(DataTypeKind(DataSpec::DataTypeKind), DataSpec::getDataTypeFullName()); @@ -319,20 +262,7 @@ int GenericSubscriber::checkInit() template void GenericSubscriber::handleIncomingTransfer(IncomingTransfer& transfer) { - /* - * Initializing the temporary RX structure - */ - ReceivedDataStructureBuffer rx_struct_buffer(*this); - - if (rx_struct_buffer.getObjectPtr() == NULL) - { - UAVCAN_ASSERT(0); // The init method should have caught this error. - failure_count_++; - node_.getDispatcher().getTransferPerfCounter().addError(); - return; - } - - rx_struct_buffer.getObjectPtr()->setTransfer(&transfer); + ReceivedDataStructureSpec rx_struct(&transfer); /* * Decoding into the temporary storage @@ -340,7 +270,7 @@ void GenericSubscriber::handleIncomi BitStream bitstream(transfer); ScalarCodec codec(bitstream); - const int decode_res = DataStruct::decode(*rx_struct_buffer.getObjectPtr(), codec); + const int decode_res = DataStruct::decode(rx_struct, codec); // We don't need the data anymore, the memory can be reused from the callback: transfer.release(); @@ -357,7 +287,7 @@ void GenericSubscriber::handleIncomi /* * Invoking the callback */ - handleReceivedDataStruct(*rx_struct_buffer.getObjectPtr()); + handleReceivedDataStruct(rx_struct); } template diff --git a/libuavcan/include/uavcan/node/marshal_buffer.hpp b/libuavcan/include/uavcan/node/marshal_buffer.hpp index e90a9b396f..353233bc33 100644 --- a/libuavcan/include/uavcan/node/marshal_buffer.hpp +++ b/libuavcan/include/uavcan/node/marshal_buffer.hpp @@ -17,11 +17,8 @@ namespace uavcan class UAVCAN_EXPORT IMarshalBuffer : public ITransferBuffer { public: - virtual uint8_t* getDataPtr() = 0; + virtual const uint8_t* getDataPtr() const = 0; virtual unsigned getMaxWritePos() const = 0; - virtual unsigned getSize() const = 0; - - const uint8_t* getDataPtr() const { return const_cast(this)->getDataPtr(); } }; /** @@ -64,12 +61,10 @@ class UAVCAN_EXPORT MarshalBufferProvider : public IMarshalBufferProvider return buf_.write(offset, data, len); } - virtual uint8_t* getDataPtr() { return buf_.getRawPtr(); } + virtual const uint8_t* getDataPtr() const { return buf_.getRawPtr(); } virtual unsigned getMaxWritePos() const { return buf_.getMaxWritePos(); } - virtual unsigned getSize() const { return MaxSize_; } - public: void reset() { buf_.reset(); } }; diff --git a/libuavcan/include/uavcan/node/node.hpp b/libuavcan/include/uavcan/node/node.hpp index d12cec6558..9cc2773c5c 100644 --- a/libuavcan/include/uavcan/node/node.hpp +++ b/libuavcan/include/uavcan/node/node.hpp @@ -47,24 +47,21 @@ namespace uavcan * be allocated in the memory pool if needed. * Default value is acceptable for any use case. * - * @tparam MarshalBufferSize Size of the marshal buffer, that is used to provide short-term temporary storage for: - * 1. Serialized data for TX transfers; - * 2. De-serialized data for RX transfers. - * The buffer must be large enough to accommodate largest serialized TX transfer and - * largest de-serialized RX data structure. The former value is constant for UAVCAN, the - * latter is platform-dependent (depends on the field padding, memory alignment, pointer - * size, etc.). - * The default value should be enough for all use cases on virtually all platforms. - * If this value is not large enough, transport objects (such as Subscriber<>, - * Publisher<>, Service*<>) will be failing at run time during initialization. + * @tparam MarshalBufferSize Size of the marshal buffer that is used to provide short-term temporary storage for + * serialized data for TX transfers. The buffer must be large enough to accommodate + * largest serialized TX transfer. The default value is guaranteed to be large enough, + * but it can be reduced if long TX transfers are not used to optimize memory use. + * If UAVCAN_TINY mode is enabled, this value defaults to the maximum length of a + * response transfer of uavcan.protocol.GetNodeInfo. */ template ::Result #else unsigned OutgoingTransferRegistryStaticEntries = 10, + unsigned MarshalBufferSize = MaxPossibleTransferPayloadLen #endif - unsigned MarshalBufferSize = 512 > class UAVCAN_EXPORT Node : public INode { diff --git a/libuavcan/include/uavcan/node/service_client.hpp b/libuavcan/include/uavcan/node/service_client.hpp index e166c07420..35e38cd535 100644 --- a/libuavcan/include/uavcan/node/service_client.hpp +++ b/libuavcan/include/uavcan/node/service_client.hpp @@ -59,6 +59,7 @@ struct ServiceCallID /** * Object of this type will be returned to the application as a result of service call. * Note that application ALWAYS gets this result, even when it times out or fails because of some other reason. + * The class is made noncopyable because it keeps a reference to a stack-allocated object. */ template class UAVCAN_EXPORT ServiceCallResult : Noncopyable @@ -258,14 +259,10 @@ private: int(state.getCallID().server_node_id.get()), int(state.getCallID().transfer_id.get()), DataType::getDataTypeFullName()); - typename SubscriberType::ReceivedDataStructureBuffer rx_struct_buffer(owner); - if (rx_struct_buffer.getObjectPtr() == NULL) - { - handleFatalError("RX obj buf"); - } + typename SubscriberType::ReceivedDataStructureSpec rx_struct; // Default-initialized ServiceCallResultType result(ServiceCallResultType::ErrorTimeout, state.getCallID(), - *rx_struct_buffer.getObjectPtr()); // Mutable! + rx_struct); // Mutable! owner.invokeCallback(result); }