Cleanup

.gitignore

@@ -1,2 +1,4 @@
 .idea
-.vscode
+.vscode
+
+CMakeLists.txt

library.json

@@ -11,7 +11,7 @@
     "url": "https://github.com/eigen-value",
     "maintainer": true
   },
-  "version": "0.1.2",
+  "version": "0.1.3",
   "license": "MPL2.0",
   "frameworks": "arduino",
   "platforms": "*",

library.properties

@@ -1,5 +1,5 @@
 name=Arduino_RPClite
-version=0.1.2
+version=0.1.3
 author=Arduino, Lucio Rossi (eigen-value)
 maintainer=Arduino, Lucio Rossi (eigen-value)
 sentence=A MessagePack RPC library for Arduino

src/Arduino_RPClite.h

@@ -14,6 +14,9 @@
 
 #include "Arduino.h"
 
+#define DECODER_BUFFER_SIZE     1024
+#define RPCLITE_MAX_TRANSPORTS  3
+
 //#define HANDLE_RPC_ERRORS
 #include "transport.h"
 #include "client.h"

src/SerialTransport.h

@@ -13,17 +13,15 @@
 #define SERIALTRANSPORT_H
 #include "transport.h"
 
-class SerialTransport: public ITransport {
+class SerialTransport final : public ITransport {
 
     Stream* _stream;
 
     public:
 
-        SerialTransport(Stream* stream): _stream(stream){}
+        explicit SerialTransport(Stream* stream): _stream(stream){}
 
-        SerialTransport(Stream& stream): _stream(&stream){}
-
-        void begin(){}
+        explicit SerialTransport(Stream& stream): _stream(&stream){}
 
         bool available() override {
             return _stream->available();

src/client.h

@@ -13,31 +13,24 @@
 #define RPCLITE_CLIENT_H
 #include "error.h"
 #include "decoder_manager.h"
-#include "SerialTransport.h"
 
 
 class RPCClient {
-    RpcDecoder<>* decoder = nullptr;
+    RpcDecoder<>* decoder;
 
 public:
     RpcError lastError;
 
-    RPCClient(ITransport& t) : decoder(&RpcDecoderManager<>::getDecoder(t)) {}
-
-    // This constructor was removed because it leads to decoder duplication
-    // RPCClient(Stream& stream) {
-    //     ITransport* transport = (ITransport*) new SerialTransport(stream);
-    //     decoder = &RpcDecoderManager<>::getDecoder(*transport);
-    // }
+    explicit RPCClient(ITransport& t) : decoder(&RpcDecoderManager<>::getDecoder(t)) {}
 
     template<typename... Args>
-    void notify(const MsgPack::str_t method, Args&&... args)  {
+    void notify(const MsgPack::str_t& method, Args&&... args)  {
         uint32_t _id;
         decoder->send_call(NOTIFY_MSG, method, _id, std::forward<Args>(args)...);
     }
 
     template<typename RType, typename... Args>
-    bool call(const MsgPack::str_t method, RType& result, Args&&... args) {
+    bool call(const MsgPack::str_t& method, RType& result, Args&&... args) {
 
         uint32_t msg_id_wait;
 
@@ -57,7 +50,7 @@ class RPCClient {
     }
 
     template<typename... Args>
-    bool send_rpc(const MsgPack::str_t method, uint32_t& wait_id, Args&&... args) {
+    bool send_rpc(const MsgPack::str_t& method, uint32_t& wait_id, Args&&... args) {
         uint32_t msg_id;
         if (decoder->send_call(CALL_MSG, method, msg_id, std::forward<Args>(args)...)) {
             wait_id = msg_id;

src/decoder.h

@@ -19,18 +19,16 @@
 using namespace RpcUtils::detail;
 
 #define MIN_RPC_BYTES   4
-
-#define MAX_BUFFER_SIZE 1024
 #define CHUNK_SIZE      32
 
-template<size_t BufferSize = MAX_BUFFER_SIZE>
+template<size_t BufferSize = DECODER_BUFFER_SIZE>
 class RpcDecoder {
 
 public:
-    RpcDecoder(ITransport& transport) : _transport(transport) {}
+    explicit RpcDecoder(ITransport& transport) : _transport(&transport) {}
 
     template<typename... Args>
-    bool send_call(const int call_type, const MsgPack::str_t method, uint32_t& msg_id, Args&&... args) {
+    bool send_call(const int call_type, const MsgPack::str_t& method, uint32_t& msg_id, Args&&... args) {
 
         if (call_type!=CALL_MSG && call_type!=NOTIFY_MSG) return false;
 
@@ -89,7 +87,7 @@ class RpcDecoder {
 
     }
 
-    bool send_response(const MsgPack::Packer& packer) {
+    bool send_response(const MsgPack::Packer& packer) const {
         return send(reinterpret_cast<const uint8_t*>(packer.data()), packer.size()) == packer.size();
     }
 
@@ -111,7 +109,6 @@ class RpcDecoder {
         };
 
         int msg_type;
-        uint32_t msg_id;
         MsgPack::str_t method;
         MsgPack::arr_size_t req_size;
 
@@ -122,6 +119,7 @@ class RpcDecoder {
         }
 
         if (msg_type == CALL_MSG && req_size.size() == REQUEST_SIZE) {
+            uint32_t msg_id;
             if (!unpacker.deserialize(msg_id, method)) {
                 consume(_packet_size);
                 reset_packet();
@@ -160,8 +158,8 @@ class RpcDecoder {
     // Fill the raw buffer to its capacity
     void advance() {
 
-        if (_transport.available() && !buffer_full()) {
-            size_t bytes_read = _transport.read(_raw_buffer + _bytes_stored, BufferSize - _bytes_stored);
+        if (_transport->available() && !buffer_full()) {
+            size_t bytes_read = _transport->read(_raw_buffer + _bytes_stored, BufferSize - _bytes_stored);
             _bytes_stored += bytes_read;
         }
 
@@ -204,35 +202,35 @@ class RpcDecoder {
 
     }
 
-    inline bool packet_incoming() const { return _packet_size >= MIN_RPC_BYTES; }
+    bool packet_incoming() const { return _packet_size >= MIN_RPC_BYTES; }
 
-    inline int packet_type() const { return _packet_type; }
+    int packet_type() const { return _packet_type; }
 
     size_t get_packet_size() const { return _packet_size;}
 
-    inline size_t size() const {return _bytes_stored;}
+    size_t size() const {return _bytes_stored;}
 
     friend class DecoderTester;
 
 private:
-    ITransport& _transport;
-    uint8_t _raw_buffer[BufferSize];
+    ITransport* _transport;
+    uint8_t _raw_buffer[BufferSize]{};
     size_t _bytes_stored = 0;
     int _packet_type = NO_MSG;
     size_t _packet_size = 0;
     uint32_t _msg_id = 0;
 
-    inline bool buffer_full() const { return _bytes_stored == BufferSize; }
+    bool buffer_full() const { return _bytes_stored == BufferSize; }
 
-    inline bool buffer_empty() const { return _bytes_stored == 0;}
+    bool buffer_empty() const { return _bytes_stored == 0;}
 
-    // This is a blocking send, under the assumption _transport.write will always succeed eventually
-    inline size_t send(const uint8_t* data, const size_t size) {
+    // This is a blocking send, under the assumption _transport->write will always succeed eventually
+    size_t send(const uint8_t* data, const size_t size) const {
 
         size_t offset = 0;
 
         while (offset < size) {
-            size_t bytes_written = _transport.write(data + offset, size - offset);
+            size_t bytes_written = _transport->write(data + offset, size - offset);
             offset += bytes_written;
         }
 

src/decoder_manager.h

@@ -14,16 +14,13 @@
 #ifndef RPCLITE_DECODER_MANAGER_H
 #define RPCLITE_DECODER_MANAGER_H
 
-#define RPCLITE_MAX_TRANSPORTS  3
-
 #include <array>
 #include "transport.h"
 #include "decoder.h"
 
 template<size_t MaxTransports = RPCLITE_MAX_TRANSPORTS>
 class RpcDecoderManager {
 public:
-    // todo parametrize so the RpcDecoder returned has a user defined buffer size ?
     static RpcDecoder<>& getDecoder(ITransport& transport) {
         for (auto& entry : decoders_) {
             if (entry.transport == &transport) {
@@ -51,7 +48,7 @@ class RpcDecoderManager {
     struct DecoderStorage {
         union {
             RpcDecoder<> instance;
-            uint8_t raw[sizeof(RpcDecoder<>)];
+            uint8_t raw[sizeof(RpcDecoder<>)]{};
         };
 
         DecoderStorage() {}

src/dispatcher.h

@@ -24,6 +24,9 @@ struct DispatchEntry {
 template<size_t N>
 class RpcFunctionDispatcher {
 public:
+
+    RpcFunctionDispatcher() = default;
+
     template<typename F>
     bool bind(MsgPack::str_t name, F&& f, MsgPack::str_t tag="") {
         if (_count >= N) return false;
@@ -34,7 +37,7 @@ class RpcFunctionDispatcher {
         return true;
     }
 
-    bool isBound(MsgPack::str_t name) const {
+    bool isBound(const MsgPack::str_t& name) const {
         for (size_t i = 0; i < _count; ++i) {
             if (_entries[i].name == name) {
                 return true;
@@ -43,7 +46,7 @@ class RpcFunctionDispatcher {
         return false;
     }
 
-    bool hasTag(MsgPack::str_t name, MsgPack::str_t tag) const {
+    bool hasTag(const MsgPack::str_t& name, MsgPack::str_t& tag) const {
         for (size_t i = 0; i < _count; ++i) {
             if (_entries[i].name == name && _entries[i].tag == tag) {
                 return true;
@@ -52,7 +55,7 @@ class RpcFunctionDispatcher {
         return false;
     }
 
-    bool call(MsgPack::str_t name, MsgPack::Unpacker& unpacker, MsgPack::Packer& packer) {
+    bool call(const MsgPack::str_t& name, MsgPack::Unpacker& unpacker, MsgPack::Packer& packer) {
         for (size_t i = 0; i < _count; ++i) {
             if (_entries[i].name == name) {
                 return (*_entries[i].fn)(unpacker, packer);

src/error.h

@@ -12,6 +12,8 @@
 #ifndef RPCLITE_ERROR_H
 #define RPCLITE_ERROR_H
 
+#include <utility>
+
 #include "MsgPack.h"
 
 #define NO_ERR                   0x00
@@ -29,8 +31,8 @@ struct RpcError {
         traceback = "";
     }
 
-    RpcError(int c, const MsgPack::str_t& tb)
-        : code(c), traceback(tb) {}
+    RpcError(const int c, MsgPack::str_t tb)
+        : code(c), traceback(std::move(tb)) {}
 
     MSGPACK_DEFINE(code, traceback); // -> [code, traceback]
 };

src/request.h

@@ -12,7 +12,7 @@
 #ifndef RPCLITE_REQUEST_H
 #define RPCLITE_REQUEST_H
 
-#define DEFAULT_RPC_BUFFER_SIZE  256
+#define DEFAULT_RPC_BUFFER_SIZE  (DECODER_BUFFER_SIZE / 4)
 
 
 #include "rpclite_utils.h"
@@ -21,24 +21,15 @@ template<size_t BufferSize = DEFAULT_RPC_BUFFER_SIZE>
 class RPCRequest {
 
 public:
-    uint8_t buffer[BufferSize];
+    uint8_t buffer[BufferSize]{};
     size_t size = 0;
     int type = NO_MSG;
     uint32_t msg_id = 0;
     MsgPack::str_t method;
     MsgPack::Packer packer;
     MsgPack::Unpacker unpacker;
 
-    // void print(){
-
-    //     Serial.print("internal buffer ");
-    //     for (size_t i=0; i<size; i++){
-    //         Serial.print(buffer[i], HEX);
-    //     }
-    //     Serial.println("");
-    // }
-
-    size_t get_buffer_size() const {
+    static size_t get_buffer_size() {
         return BufferSize;
     }
 
@@ -94,4 +85,4 @@ class RPCRequest {
 
 };
 
-#endif RPCLITE_REQUEST_H
+#endif //RPCLITE_REQUEST_H

src/rpclite_utils.h

@@ -19,8 +19,9 @@
 namespace RpcUtils {
 namespace detail {
 
-#define WRONG_MSG       -2
-#define NO_MSG          -1
+#define TYPE_ERROR      (-1)
+#define WRONG_MSG       (-2)
+#define NO_MSG          (-1)
 #define CALL_MSG        0
 #define RESP_MSG        1
 #define NOTIFY_MSG      2
@@ -48,12 +49,12 @@ inline bool unpackTypedArray(MsgPack::Unpacker& unpacker, size_t& size, int& typ
 
     size = 0;
     for (size_t i=0; i<sz.size(); i++){
-        if ((i==0)) {
+        if (i==0) {
             if (unpacker.isInt() || unpacker.isUInt()) {
                 unpacker.deserialize(rpc_type);
                 type = rpc_type;
                 size++;
-                continue; // First element must be the type
+                continue; // the First element must be the type
             } else {
                 type = WRONG_MSG; // Not a valid type
             }
@@ -165,10 +166,10 @@ inline bool unpackObject(MsgPack::Unpacker& unpacker){
 }
 
 template<typename T>
-inline bool deserialize_single(MsgPack::Unpacker& unpacker, T& value) {
-    if (!unpacker.unpackable(value)) return false;
+int deserialize_single(MsgPack::Unpacker& unpacker, T& value) {
+    if (!unpacker.unpackable(value)) return TYPE_ERROR;
     unpacker.deserialize(value);
-    return true;
+    return 0;
 }
 
 
@@ -177,21 +178,22 @@ inline bool deserialize_single(MsgPack::Unpacker& unpacker, T& value) {
 /////////////////////////////
 
 template<std::size_t I = 0, typename... Ts>
-inline typename std::enable_if<I == sizeof...(Ts), bool>::type
+typename std::enable_if<I == sizeof...(Ts), int>::type
 deserialize_tuple(MsgPack::Unpacker&, std::tuple<Ts...>&) {
-    return true;
+    return 0;
 }
 
 template<std::size_t I = 0, typename... Ts>
-inline typename std::enable_if<I < sizeof...(Ts), bool>::type
+typename std::enable_if<I < sizeof...(Ts), int>::type
 deserialize_tuple(MsgPack::Unpacker& unpacker, std::tuple<Ts...>& out) {
-    if (!deserialize_single(unpacker, std::get<I>(out))) return false;
+    const int res = deserialize_single(unpacker, std::get<I>(out));
+    if (res==TYPE_ERROR) return TYPE_ERROR-I;
     return deserialize_tuple<I + 1>(unpacker, out);
 }
 
 // Helper to invoke a function with a tuple of arguments
 template<typename F, typename Tuple, std::size_t... I>
-inline auto invoke_with_tuple(F&& f, Tuple&& t, arx::stdx::index_sequence<I...>)
+auto invoke_with_tuple(F&& f, Tuple&& t, arx::stdx::index_sequence<I...>)
     -> decltype(f(std::get<I>(std::forward<Tuple>(t))...)) {
     return f(std::get<I>(std::forward<Tuple>(t))...);
 }