From f8595800e8147f6c12d52aef99c4e453ec4ad227 Mon Sep 17 00:00:00 2001 From: lonkaars Date: Sun, 19 May 2024 14:28:07 +0200 Subject: finish puzzle bus writer/reader functions --- proto/puzbusv1.c | 43 +++++++++++++++++++++++++++++++------------ proto/puzbusv1.h | 37 +++++++++++++++++++++++++++++++++++-- 2 files changed, 66 insertions(+), 14 deletions(-) (limited to 'proto') diff --git a/proto/puzbusv1.c b/proto/puzbusv1.c index 3ff7c63..d6cd597 100644 --- a/proto/puzbusv1.c +++ b/proto/puzbusv1.c @@ -1,25 +1,44 @@ #include #include +// MIN() macro +#include +// TODO: check if this works on pico as well + #include "puzbusv1.h" -int pb_read(struct pb_msg* target, char* buf, size_t buf_sz) { - mpack_reader_t reader; - printf("read %lu bytes...\n", buf_sz); +bool pb_read(struct pb_msg* target, char* buf, size_t buf_sz) { + // remaining bytes to be read to target->data; this is the only variable that + // needs to persist between buffer blocks, and is therefore static + static size_t rdata = 0; + // a new reader is used per buffer block passed to this function + mpack_reader_t reader; mpack_reader_init_data(&reader, buf, buf_sz); - uint16_t address = mpack_expect_u16(&reader); - char data_buf[80]; - size_t data_size = mpack_expect_bin_buf(&reader, data_buf, sizeof(data_buf)); - - printf("0x%02x\n", address); - printf("\"%.*s\"\n", data_size, data_buf); - - return 0; + // at start of message + if (rdata == 0) { + // NOTE: This approach will crash and burn when target->addr can be read + // and target->length is past the end of the current buffer block. This is + // a highly unlikely scenario, as pb_read is called for each chunk of a TCP + // frame, and frames (should) include only one puzzle bus message. + target->addr = mpack_expect_u16(&reader); + target->length = rdata = mpack_expect_bin(&reader); + target->data = (char*) malloc(target->length); + } + + // continue reading chunks of target->data until the amount of bytes + // specified in target->length + size_t to_read = MIN(mpack_reader_remaining(&reader, NULL), rdata); + char* data = target->data + target->length - rdata; // 'ol pointer arithmetic + mpack_read_bytes(&reader, data, to_read); + rdata -= to_read; + + // if rdata = 0, the message was completely read + return rdata == 0; } -int pb_write(struct pb_msg* target, char** buf, size_t* buf_sz) { +bool pb_write(struct pb_msg* target, char** buf, size_t* buf_sz) { mpack_writer_t writer; mpack_writer_init_growable(&writer, buf, buf_sz); diff --git a/proto/puzbusv1.h b/proto/puzbusv1.h index 116dbf9..814bc93 100644 --- a/proto/puzbusv1.h +++ b/proto/puzbusv1.h @@ -13,8 +13,41 @@ struct pb_msg { size_t length; }; -int pb_read(struct pb_msg* target, char* buf, size_t buf_sz); -int pb_write(struct pb_msg* target, char** buf, size_t* buf_sz); +/** + * \brief Read chunk of input stream, and store resulting message in \p target + * + * This function is called for each chunk of data from an input stream, and + * will parse the next puzzle bus message into \p target. The input stream is + * assumed to only contain messages encoded by \p pb_write() + * + * \param target pointer to struct that will contain the finished message data + * \param buf pointer to input stream data chunk + * \param buf_sz size of \p buf + * + * \returns boolean true if a message was completely read, false if more data + * is required + * + * \note target->data will automatically be allocated by this function, and + * must be `free()`d by the caller when finished + */ +bool pb_read(struct pb_msg* target, char* buf, size_t buf_sz); +/** + * \brief Allocate and write a msgpack-formatted message to \p buf + * + * This function allocates a buffer large enough to fit the message specified + * in \p target, and encodes the data in \p target in a format that can be + * decoded later using \p pb_read() + * + * \param target pointer to struct that contains the message data + * \param buf pointer to \c char* that will contain the formatted message + * \param buf_sz pointer to \c size_t that will represent the final size of \p buf + * + * \returns boolean true if a the message could be encoded successfully, false + * if there was some kind of error + * + * \note the pointer stored in \p buf must be `free()`d by the caller afterwards + */ +bool pb_write(struct pb_msg* target, char** buf, size_t* buf_sz); #ifdef __cplusplus } -- cgit v1.2.3 From 9ff66bfe22c5f378584db2a57160d00b585a77ce Mon Sep 17 00:00:00 2001 From: lonkaars Date: Mon, 20 May 2024 10:25:21 +0200 Subject: make puzbus slightly more robust --- client/main.cpp | 27 ++++++++++++++++++++------- proto/puzbusv1.c | 33 ++++++++++++++++++++++++--------- proto/puzbusv1.h | 23 ++++++++++++++++++----- 3 files changed, 62 insertions(+), 21 deletions(-) (limited to 'proto') diff --git a/client/main.cpp b/client/main.cpp index 30d7045..dcc965b 100644 --- a/client/main.cpp +++ b/client/main.cpp @@ -17,31 +17,44 @@ int send_message() { size_t size; if (!pb_write(&output, &packed, &size)) { printf("error writing!\n"); - return 1; + return EXIT_FAILURE; } fwrite(packed, sizeof(packed[0]), size, stdout); fflush(stdout); - return 0; + return EXIT_SUCCESS; } int read_message() { freopen(NULL, "rb", stdin); // allow binary on stdin struct pb_msg input; - char buf[8]; // extremely small buffer to test chunked message parsing + char buf[4]; // extremely small buffer to test chunked message parsing size_t bytes = 0; + while ((bytes = fread(buf, sizeof(buf[0]), sizeof(buf), stdin)) > 0) { - if (!pb_read(&input, buf, bytes)) continue; + int ret = pb_read(&input, buf, bytes); + + // header read error + if (ret < 0) { + printf("error reading!\n"); + return EXIT_FAILURE; + } + + // continue reading if more bytes needed... + if (ret > 0) continue; + // message read completely! printf("address: 0x%02x\n", input.addr); printf("data: \"%.*s\"\n", input.length, input.data); free(input.data); - return 0; + return EXIT_SUCCESS; } - return 1; + // if we reach this point, data was read but it did not contain a complete + // message, and is thus considered a failure + return EXIT_FAILURE; } int main() { @@ -49,6 +62,6 @@ int main() { if (!isatty(fileno(stdin))) return read_message(); printf("please pipe some data in or out to use this program\n"); - return 0; + return EXIT_SUCCESS; } diff --git a/proto/puzbusv1.c b/proto/puzbusv1.c index d6cd597..3be4939 100644 --- a/proto/puzbusv1.c +++ b/proto/puzbusv1.c @@ -7,21 +7,32 @@ #include "puzbusv1.h" -bool pb_read(struct pb_msg* target, char* buf, size_t buf_sz) { - // remaining bytes to be read to target->data; this is the only variable that - // needs to persist between buffer blocks, and is therefore static - static size_t rdata = 0; +/** + * \brief Remaining bytes to be read to target->data + * + * This is the only variable that needs to persist between buffer blocks. It is + * declared in the global scope to allow resetting using the \c pb_read_reset() + * function. + * + * \note \p rdata may be reset by calling \c pb_read_reset() + */ +static size_t rdata = 0; +int pb_read(struct pb_msg* target, char* buf, size_t buf_sz) { // a new reader is used per buffer block passed to this function mpack_reader_t reader; mpack_reader_init_data(&reader, buf, buf_sz); // at start of message if (rdata == 0) { - // NOTE: This approach will crash and burn when target->addr can be read - // and target->length is past the end of the current buffer block. This is - // a highly unlikely scenario, as pb_read is called for each chunk of a TCP - // frame, and frames (should) include only one puzzle bus message. + // NOTE: The entire start of a message needs to be readable from the buffer + // at this point. When target->addr can be read and target->length is past + // the end of the current buffer block, this function will crash and burn. + // This is a highly unlikely scenario, as pb_read is called for each chunk + // of a TCP frame, and frames (should) include only one puzzle bus message. + // The check here is kind of optional. + if (buf_sz < 4) return -1; + target->addr = mpack_expect_u16(&reader); target->length = rdata = mpack_expect_bin(&reader); target->data = (char*) malloc(target->length); @@ -35,7 +46,11 @@ bool pb_read(struct pb_msg* target, char* buf, size_t buf_sz) { rdata -= to_read; // if rdata = 0, the message was completely read - return rdata == 0; + return rdata; +} + +void pb_read_reset() { + rdata = 0; } bool pb_write(struct pb_msg* target, char** buf, size_t* buf_sz) { diff --git a/proto/puzbusv1.h b/proto/puzbusv1.h index 814bc93..95130c9 100644 --- a/proto/puzbusv1.h +++ b/proto/puzbusv1.h @@ -24,13 +24,26 @@ struct pb_msg { * \param buf pointer to input stream data chunk * \param buf_sz size of \p buf * - * \returns boolean true if a message was completely read, false if more data - * is required + * \returns Integer representing amount of bytes required to finish message, or + * -1 if the message header could not be read. If this function returns 0, the + * message in \p target is complete. * - * \note target->data will automatically be allocated by this function, and - * must be `free()`d by the caller when finished + * \note target->data will automatically be allocated by this function, even if + * the message is not fully parsed. This variable must be `free()`d by the + * caller after each complete message to prevent memory leaks. */ -bool pb_read(struct pb_msg* target, char* buf, size_t buf_sz); +int pb_read(struct pb_msg* target, char* buf, size_t buf_sz); + +/** + * \brief reset the remaining message data counter + * + * Calling this function has the effect of forcing \c pb_read() to parse the + * next buffer chunk as the start of a new message. This function may be called + * before reading a TCP frame's data to mitigate any synchronization issues + * arising from earlier corrupt or otherwise malformed messages. + */ +void pb_read_reset(); + /** * \brief Allocate and write a msgpack-formatted message to \p buf * -- cgit v1.2.3 From 68a5c65f9b0e1df30e9cef490d9b218b2f21f90d Mon Sep 17 00:00:00 2001 From: lonkaars Date: Tue, 21 May 2024 10:30:06 +0200 Subject: clean up puzbusv1 API --- client/sock.cpp | 12 ++++++------ client/sock.h | 13 ++++++------- main/sock.c | 10 ++++++---- main/sock.h | 4 ++-- proto/puzbusv1.c | 33 +++++++++++---------------------- proto/puzbusv1.h | 14 ++++++++------ 6 files changed, 39 insertions(+), 47 deletions(-) (limited to 'proto') diff --git a/client/sock.cpp b/client/sock.cpp index cc18a69..f967f64 100644 --- a/client/sock.cpp +++ b/client/sock.cpp @@ -18,7 +18,7 @@ using std::logic_error; using std::thread; PBSocket::PBSocket() { } -PBSocket::PBSocket(char* addr, uint16_t port) : PBSocket() { +PBSocket::PBSocket(const char * addr, uint16_t port) : PBSocket() { set_server(addr, port); } @@ -32,7 +32,7 @@ PBSocket::~PBSocket() { sock_close(); } -void PBSocket::set_server(char* addr, uint16_t port) { +void PBSocket::set_server(const char * addr, uint16_t port) { _addr = addr; _port = port; } @@ -67,7 +67,7 @@ void PBSocket::sock_close() { _fd = -1; } -void PBSocket::send(char* buf, size_t buf_sz) { +void PBSocket::send(const char * buf, size_t buf_sz) { write(_fd, buf, buf_sz); } @@ -105,10 +105,10 @@ void PBSocket::sock_task() { sock_close(); } -void i2c_send(uint16_t addr, char* data, size_t data_size) { +void i2c_send(uint16_t addr, const char * data, size_t data_size) { struct pb_msg msg = { .addr = addr, - .data = data, + .data = (char *) data, .length = data_size, }; @@ -119,7 +119,7 @@ void i2c_send(uint16_t addr, char* data, size_t data_size) { sock->send(packed, size); } -void i2c_recv(uint16_t addr, char* data, size_t data_size) { +void i2c_recv(uint16_t addr, const char * data, size_t data_size) { rl_printf("[0x%02x]: %.*s\n", addr, data_size, data); } diff --git a/client/sock.h b/client/sock.h index 818ea72..42eba3b 100644 --- a/client/sock.h +++ b/client/sock.h @@ -6,14 +6,14 @@ class PBSocket { public: PBSocket(); - PBSocket(char* addr, uint16_t port); + PBSocket(const char * addr, uint16_t port); virtual ~PBSocket(); - void set_server(char* addr, uint16_t port); + void set_server(const char * addr, uint16_t port); void sock_connect(); - void send(char* buf, size_t buf_sz); + void send(const char * buf, size_t buf_sz); private: void sock_task(); @@ -21,15 +21,14 @@ private: std::thread* _thread = nullptr; - char* _addr = NULL; + const char * _addr = NULL; uint16_t _port = 0; int _fd = -1; - }; extern PBSocket* sock; -void i2c_send(uint16_t addr, char* data, size_t data_size); -void i2c_recv(uint16_t addr, char* data, size_t data_size); +void i2c_send(uint16_t addr, const char * data, size_t data_size); +void i2c_recv(uint16_t addr, const char * data, size_t data_size); diff --git a/main/sock.c b/main/sock.c index 705e2eb..4f50981 100644 --- a/main/sock.c +++ b/main/sock.c @@ -13,16 +13,16 @@ struct netconn* current_connection = NULL; struct pb_msg recv_msg; -void i2c_send(uint16_t addr, char* data, size_t data_size) { +void i2c_send(uint16_t addr, const char * data, size_t data_size) { if (current_connection == NULL) return; struct pb_msg send_msg = { .addr = addr, - .data = data, + .data = (char *) data, .length = data_size, }; - char* buf; + char * buf; size_t buf_sz; if (!pb_write(&send_msg, &buf, &buf_sz)) return; @@ -34,7 +34,7 @@ void i2c_send(uint16_t addr, char* data, size_t data_size) { free(buf); } -void i2c_recv(uint16_t addr, char* data, size_t data_size) { +void i2c_recv(uint16_t addr, const char * data, size_t data_size) { printf("address: 0x%02x\n", addr); printf("data: \"%.*s\"\n", data_size, data); @@ -47,6 +47,8 @@ void i2c_recv(uint16_t addr, char* data, size_t data_size) { } void recv_handler(struct netconn* conn, struct netbuf* buf) { + pb_read_reset(&recv_msg); + do { char* data; uint16_t len; diff --git a/main/sock.h b/main/sock.h index 2a73418..f2db35d 100644 --- a/main/sock.h +++ b/main/sock.h @@ -6,6 +6,6 @@ /** \brief start listening for TCP socket requests */ void serve_task(); -void i2c_send(uint16_t addr, char* data, size_t data_size); -void i2c_recv(uint16_t addr, char* data, size_t data_size); +void i2c_send(uint16_t addr, const char * data, size_t data_size); +void i2c_recv(uint16_t addr, const char * data, size_t data_size); diff --git a/proto/puzbusv1.c b/proto/puzbusv1.c index 3be4939..73deda5 100644 --- a/proto/puzbusv1.c +++ b/proto/puzbusv1.c @@ -7,24 +7,13 @@ #include "puzbusv1.h" -/** - * \brief Remaining bytes to be read to target->data - * - * This is the only variable that needs to persist between buffer blocks. It is - * declared in the global scope to allow resetting using the \c pb_read_reset() - * function. - * - * \note \p rdata may be reset by calling \c pb_read_reset() - */ -static size_t rdata = 0; - -int pb_read(struct pb_msg* target, char* buf, size_t buf_sz) { +int pb_read(struct pb_msg * target, const char * buf, size_t buf_sz) { // a new reader is used per buffer block passed to this function mpack_reader_t reader; mpack_reader_init_data(&reader, buf, buf_sz); // at start of message - if (rdata == 0) { + if (target->_rdata == 0) { // NOTE: The entire start of a message needs to be readable from the buffer // at this point. When target->addr can be read and target->length is past // the end of the current buffer block, this function will crash and burn. @@ -34,26 +23,26 @@ int pb_read(struct pb_msg* target, char* buf, size_t buf_sz) { if (buf_sz < 4) return -1; target->addr = mpack_expect_u16(&reader); - target->length = rdata = mpack_expect_bin(&reader); - target->data = (char*) malloc(target->length); + target->length = target->_rdata = mpack_expect_bin(&reader); + target->data = (char *) malloc(target->length); } // continue reading chunks of target->data until the amount of bytes // specified in target->length - size_t to_read = MIN(mpack_reader_remaining(&reader, NULL), rdata); - char* data = target->data + target->length - rdata; // 'ol pointer arithmetic + size_t to_read = MIN(mpack_reader_remaining(&reader, NULL), target->_rdata); + char * data = target->data + target->length - target->_rdata; mpack_read_bytes(&reader, data, to_read); - rdata -= to_read; + target->_rdata -= to_read; // if rdata = 0, the message was completely read - return rdata; + return target->_rdata; } -void pb_read_reset() { - rdata = 0; +void pb_read_reset(struct pb_msg * target) { + target->_rdata = 0; } -bool pb_write(struct pb_msg* target, char** buf, size_t* buf_sz) { +bool pb_write(const struct pb_msg * target, char ** buf, size_t * buf_sz) { mpack_writer_t writer; mpack_writer_init_growable(&writer, buf, buf_sz); diff --git a/proto/puzbusv1.h b/proto/puzbusv1.h index 95130c9..0985b2b 100644 --- a/proto/puzbusv1.h +++ b/proto/puzbusv1.h @@ -7,10 +7,12 @@ extern "C" { #endif +/** \brief Puzzle bus message (v1) */ struct pb_msg { - uint16_t addr; - char* data; - size_t length; + uint16_t addr; //!< I^2^C address + char * data; //!< message content + size_t length; //!< message size + size_t _rdata; //!< \private remaining bytes to read until message is complete }; /** @@ -32,7 +34,7 @@ struct pb_msg { * the message is not fully parsed. This variable must be `free()`d by the * caller after each complete message to prevent memory leaks. */ -int pb_read(struct pb_msg* target, char* buf, size_t buf_sz); +int pb_read(struct pb_msg * target, const char * buf, size_t buf_sz); /** * \brief reset the remaining message data counter @@ -42,7 +44,7 @@ int pb_read(struct pb_msg* target, char* buf, size_t buf_sz); * before reading a TCP frame's data to mitigate any synchronization issues * arising from earlier corrupt or otherwise malformed messages. */ -void pb_read_reset(); +void pb_read_reset(struct pb_msg * target); /** * \brief Allocate and write a msgpack-formatted message to \p buf @@ -60,7 +62,7 @@ void pb_read_reset(); * * \note the pointer stored in \p buf must be `free()`d by the caller afterwards */ -bool pb_write(struct pb_msg* target, char** buf, size_t* buf_sz); +bool pb_write(const struct pb_msg * target, char ** buf, size_t * buf_sz); #ifdef __cplusplus } -- cgit v1.2.3