Stack-allocating RX objects

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
 /**
  * @}
  */

libuavcan/include/uavcan/node/generic_subscriber.hpp

@@ -33,7 +33,7 @@ namespace uavcan
 template <typename DataType_>
 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 <typename Ret, Ret(IncomingTransfer::*Fun) () const>
     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*));
 
-    /*
+protected:
-     * Received object and its allocators
-     */
     struct ReceivedDataStructureSpec : public ReceivedDataStructure<DataStruct>
     {
-        using ReceivedDataStructure<DataStruct>::setTransfer;
+        ReceivedDataStructureSpec() { }
+
+        ReceivedDataStructureSpec(const IncomingTransfer* arg_transfer)
+            : ReceivedDataStructure<DataStruct>(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<DataSpec, DataStruct, TransferListenerType>::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<DataSpec, DataStruct, TransferListenerType>::checkInit()
 template <typename DataSpec, typename DataStruct, typename TransferListenerType>
 void GenericSubscriber<DataSpec, DataStruct, TransferListenerType>::handleIncomingTransfer(IncomingTransfer& transfer)
 {
-    /*
+    ReceivedDataStructureSpec rx_struct(&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);
 
     /*
      * Decoding into the temporary storage
@@ -340,7 +270,7 @@ void GenericSubscriber<DataSpec, DataStruct, TransferListenerType>::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<DataSpec, DataStruct, TransferListenerType>::handleIncomi
     /*
      * Invoking the callback
      */
-    handleReceivedDataStruct(*rx_struct_buffer.getObjectPtr());
+    handleReceivedDataStruct(rx_struct);
 }
 
 template <typename DataSpec, typename DataStruct, typename TransferListenerType>

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<IMarshalBuffer*>(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(); }
     };

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:
+ * @tparam MarshalBufferSize    Size of the marshal buffer that is used to provide short-term temporary storage for
- *                                1. Serialized data for TX transfers;
+ *                              serialized data for TX transfers. The buffer must be large enough to accommodate
- *                                2. De-serialized data for RX transfers.
+ *                              largest serialized TX transfer. The default value is guaranteed to be large enough,
- *                              The buffer must be large enough to accommodate largest serialized TX transfer and
+ *                              but it can be reduced if long TX transfers are not used to optimize memory use.
- *                              largest de-serialized RX data structure. The former value is constant for UAVCAN, the
+ *                              If UAVCAN_TINY mode is enabled, this value defaults to the maximum length of a
- *                              latter is platform-dependent (depends on the field padding, memory alignment, pointer
+ *                              response transfer of uavcan.protocol.GetNodeInfo.
- *                              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.
  */
 template <std::size_t MemPoolSize_,
 #if UAVCAN_TINY
           unsigned OutgoingTransferRegistryStaticEntries = 0,
+          unsigned MarshalBufferSize = BitLenToByteLen<protocol::GetNodeInfo::Response::MaxBitLen>::Result
 #else
           unsigned OutgoingTransferRegistryStaticEntries = 10,
+          unsigned MarshalBufferSize = MaxPossibleTransferPayloadLen
 #endif
-          unsigned MarshalBufferSize = 512
           >
 class UAVCAN_EXPORT Node : public INode
 {

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 <typename DataType>
 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);
+                typename SubscriberType::ReceivedDataStructureSpec rx_struct; // Default-initialized
-                if (rx_struct_buffer.getObjectPtr() == NULL)
-                {
-                    handleFatalError("RX obj buf");
-                }
 
                 ServiceCallResultType result(ServiceCallResultType::ErrorTimeout, state.getCallID(),
-                                             *rx_struct_buffer.getObjectPtr());    // Mutable!
+                                             rx_struct);    // Mutable!
 
                 owner.invokeCallback(result);
             }