diff --git a/.gitignore b/.gitignore index d48c759..8bc50b1 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,4 @@ .idea -.vscode \ No newline at end of file +.vscode + +CMakeLists.txt \ No newline at end of file diff --git a/library.json b/library.json index 168f631..1fa6098 100644 --- a/library.json +++ b/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": "*", diff --git a/library.properties b/library.properties index df4bc5b..8ebdfb6 100644 --- a/library.properties +++ b/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 diff --git a/src/Arduino_RPClite.h b/src/Arduino_RPClite.h index eb0cbf0..57c4899 100644 --- a/src/Arduino_RPClite.h +++ b/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" diff --git a/src/SerialTransport.h b/src/SerialTransport.h index 861e0f1..5009254 100644 --- a/src/SerialTransport.h +++ b/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(); diff --git a/src/client.h b/src/client.h index 655f1d4..8d95748 100644 --- a/src/client.h +++ b/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 - 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)...); } template - 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 - 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)...)) { wait_id = msg_id; diff --git a/src/decoder.h b/src/decoder.h index 234adae..f3ab3d3 100644 --- a/src/decoder.h +++ b/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 +template class RpcDecoder { public: - RpcDecoder(ITransport& transport) : _transport(transport) {} + explicit RpcDecoder(ITransport& transport) : _transport(&transport) {} template - 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(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; } diff --git a/src/decoder_manager.h b/src/decoder_manager.h index 04d9ce2..31f0615 100644 --- a/src/decoder_manager.h +++ b/src/decoder_manager.h @@ -14,8 +14,6 @@ #ifndef RPCLITE_DECODER_MANAGER_H #define RPCLITE_DECODER_MANAGER_H -#define RPCLITE_MAX_TRANSPORTS 3 - #include #include "transport.h" #include "decoder.h" @@ -23,7 +21,6 @@ template 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() {} diff --git a/src/dispatcher.h b/src/dispatcher.h index 9490cb1..72ff389 100644 --- a/src/dispatcher.h +++ b/src/dispatcher.h @@ -24,6 +24,9 @@ struct DispatchEntry { template class RpcFunctionDispatcher { public: + + RpcFunctionDispatcher() = default; + template 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); diff --git a/src/error.h b/src/error.h index d3713c5..925b30a 100644 --- a/src/error.h +++ b/src/error.h @@ -12,6 +12,8 @@ #ifndef RPCLITE_ERROR_H #define RPCLITE_ERROR_H +#include + #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] }; diff --git a/src/request.h b/src/request.h index 5460408..941dd86 100644 --- a/src/request.h +++ b/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,7 +21,7 @@ template class RPCRequest { public: - uint8_t buffer[BufferSize]; + uint8_t buffer[BufferSize]{}; size_t size = 0; int type = NO_MSG; uint32_t msg_id = 0; @@ -29,16 +29,7 @@ class RPCRequest { MsgPack::Packer packer; MsgPack::Unpacker unpacker; - // void print(){ - - // Serial.print("internal buffer "); - // for (size_t i=0; i -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 -inline typename std::enable_if::type +typename std::enable_if::type deserialize_tuple(MsgPack::Unpacker&, std::tuple&) { - return true; + return 0; } template -inline typename std::enable_if::type +typename std::enable_if::type deserialize_tuple(MsgPack::Unpacker& unpacker, std::tuple& out) { - if (!deserialize_single(unpacker, std::get(out))) return false; + const int res = deserialize_single(unpacker, std::get(out)); + if (res==TYPE_ERROR) return TYPE_ERROR-I; return deserialize_tuple(unpacker, out); } // Helper to invoke a function with a tuple of arguments template -inline auto invoke_with_tuple(F&& f, Tuple&& t, arx::stdx::index_sequence) +auto invoke_with_tuple(F&& f, Tuple&& t, arx::stdx::index_sequence) -> decltype(f(std::get(std::forward(t))...)) { return f(std::get(std::forward(t))...); } diff --git a/src/server.h b/src/server.h index 8de3c96..3200b40 100644 --- a/src/server.h +++ b/src/server.h @@ -12,60 +12,56 @@ #ifndef RPCLITE_SERVER_H #define RPCLITE_SERVER_H +#include + #include "request.h" #include "error.h" -#include "wrapper.h" #include "dispatcher.h" #include "decoder.h" #include "decoder_manager.h" -#include "SerialTransport.h" #define MAX_CALLBACKS 100 class RPCServer { public: - RPCServer(ITransport& t) : decoder(&RpcDecoderManager<>::getDecoder(t)) {} - - // This constructor was removed because it leads to decoder duplication - // RPCServer(Stream& stream) { - // ITransport* transport = (ITransport*) new SerialTransport(stream); - // decoder = &RpcDecoderManager<>::getDecoder(*transport); - // } + explicit RPCServer(ITransport& t) : decoder(&RpcDecoderManager<>::getDecoder(t)) {} template bool bind(const MsgPack::str_t& name, F&& func, MsgPack::str_t tag=""){ return dispatcher.bind(name, func, tag); } - bool hasTag(MsgPack::str_t name, MsgPack::str_t tag){ + bool hasTag(const MsgPack::str_t& name, MsgPack::str_t tag) const { return dispatcher.hasTag(name, tag); } - void run() { + bool run() { RPCRequest<> req; - if (!get_rpc(req)) return; // Populate local request + if (!get_rpc(req)) return false; // Populate local request - process_request(req); // Process local data + process_request(req); // Process local data - send_response(req); // Send from local data + return send_response(req); // Send local data } - bool get_rpc(RPCRequest<>& req, MsgPack::str_t tag="") { + template + bool get_rpc(RPCRequest& req, MsgPack::str_t tag="") { decoder->decode(); - MsgPack::str_t method = decoder->fetch_rpc_method(); + const MsgPack::str_t method = decoder->fetch_rpc_method(); - if (method == "" || !hasTag(method, tag)) return false; + if (method == "" || !dispatcher.hasTag(method, tag)) return false; - req.size = decoder->get_request(req.buffer, req.get_buffer_size()); // todo overload get_request(RPCRequest& req) so all the request info is in req + req.size = decoder->get_request(req.buffer, RpcSize); return req.size > 0; } - void process_request(RPCRequest<>& req) { + template + void process_request(RPCRequest& req) { if (!req.unpack_request_headers()) { req.reset(); @@ -78,7 +74,8 @@ class RPCServer { } - bool send_response(RPCRequest<>& req) { + template + bool send_response(const RPCRequest& req) const { if (req.type == NO_MSG || req.packer.size() == 0) { return true; // No response to send @@ -92,7 +89,7 @@ class RPCServer { private: RpcDecoder<>* decoder = nullptr; - RpcFunctionDispatcher dispatcher; + RpcFunctionDispatcher dispatcher{}; }; diff --git a/src/transport.h b/src/transport.h index 37e3f10..615848c 100644 --- a/src/transport.h +++ b/src/transport.h @@ -16,7 +16,9 @@ class ITransport { // Transport abstraction interface public: - virtual size_t write(const uint8_t* data, const size_t size) = 0; +virtual ~ITransport() = default; + +virtual size_t write(const uint8_t* data, size_t size) = 0; virtual size_t read(uint8_t* buffer, size_t size) = 0; virtual size_t read_byte(uint8_t& r) = 0; virtual bool available() = 0; diff --git a/src/wrapper.h b/src/wrapper.h index fb9af84..feee71e 100644 --- a/src/wrapper.h +++ b/src/wrapper.h @@ -12,6 +12,8 @@ #ifndef RPCLITE_WRAPPER_H #define RPCLITE_WRAPPER_H +#include + #include "error.h" #include "rpclite_utils.h" @@ -23,7 +25,7 @@ using namespace RpcUtils::detail; class IFunctionWrapper { public: - virtual ~IFunctionWrapper() {} + virtual ~IFunctionWrapper() = default; virtual bool operator()(MsgPack::Unpacker& unpacker, MsgPack::Packer& packer) = 0; }; @@ -33,7 +35,7 @@ class RpcFunctionWrapper; template class RpcFunctionWrapper>: public IFunctionWrapper { public: - RpcFunctionWrapper(std::function func) : _func(func) {} + explicit RpcFunctionWrapper(std::function func) : _func(func) {} R operator()(Args... args) { return _func(args...); @@ -69,7 +71,20 @@ class RpcFunctionWrapper>: public IFunctionWrapper { return false; } - return handle_call(unpacker, packer); + const int res = handle_call(unpacker, packer); + if (res<0) { + MsgPack::str_t err_msg = "Wrong type parameter in position: "; +#ifdef ARDUINO + err_msg += String(TYPE_ERROR-res); +#else + err_msg += std::to_string(TYPE_ERROR-res); +#endif + RpcError error(MALFORMED_CALL_ERR, err_msg); + packer.serialize(error, nil); + return false; + } + + return true; #ifdef HANDLE_RPC_ERRORS } catch (const std::exception& e) { @@ -86,27 +101,29 @@ class RpcFunctionWrapper>: public IFunctionWrapper { std::function _func; template - typename std::enable_if::value, bool>::type + typename std::enable_if::value, int>::type handle_call(MsgPack::Unpacker& unpacker, MsgPack::Packer& packer) { //unpacker not ready if deserialization fails at this point std::tuple args; - if (!deserialize_tuple(unpacker, args)) return false; + const int res = deserialize_tuple(unpacker, args); + if (res<0) return res; MsgPack::object::nil_t nil; invoke_with_tuple(_func, args, arx::stdx::make_index_sequence{}); packer.serialize(nil, nil); - return true; + return 0; } template - typename std::enable_if::value, bool>::type + typename std::enable_if::value, int>::type handle_call(MsgPack::Unpacker& unpacker, MsgPack::Packer& packer) { //unpacker not ready if deserialization fails at this point std::tuple args; - if (!deserialize_tuple(unpacker, args)) return false; + const int res = deserialize_tuple(unpacker, args); + if (res<0) return res; MsgPack::object::nil_t nil; R out = invoke_with_tuple(_func, args, arx::stdx::make_index_sequence{}); packer.serialize(nil, out); - return true; + return 0; } };