From d1aec88f23aa5a40cd70e51aab3ebed81779c9ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Hru=C5=A1ka?= Date: Sun, 15 Oct 2017 18:18:20 +0200 Subject: [PATCH] did some cleaning --- Makefile | 2 +- TinyFrame.c | 145 +++++++++++++++++++++++++++++++++------------------- TinyFrame.h | 15 +++--- test.c | 9 ++-- 4 files changed, 107 insertions(+), 64 deletions(-) diff --git a/Makefile b/Makefile index 0124af5..259b362 100644 --- a/Makefile +++ b/Makefile @@ -7,4 +7,4 @@ debug: tf.bin gdb -q -ex run ./tf.bin tf.bin: test.c TinyFrame.c TinyFrame.h - gcc -Os --std=gnu89 -Wall -Wno-main -Wno-unused -Wextra test.c TinyFrame.c -I. -o tf.bin + gcc -O0 -ggdb --std=gnu89 -Wall -Wno-main -Wextra test.c TinyFrame.c -I. -o tf.bin diff --git a/TinyFrame.c b/TinyFrame.c index 55fc727..ad76c00 100644 --- a/TinyFrame.c +++ b/TinyFrame.c @@ -23,8 +23,8 @@ enum TFState { typedef struct _IdListener_struct_ { TF_ID id; TF_LISTENER fn; - unsigned int timeout; // nr of ticks remaining to disable this listener - unsigned int timeout_max; // the original timeout is stored here + TF_COUNT timeout; // nr of ticks remaining to disable this listener + TF_COUNT timeout_max; // the original timeout is stored here void *userdata; } IdListener; @@ -51,7 +51,7 @@ static struct TinyFrameStruct { TF_ID id; //!< Incoming packet ID TF_LEN len; //!< Payload length uint8_t data[TF_MAX_PAYLOAD_RX]; //!< Data byte buffer - size_t rxi; //!< Byte counter + TF_LEN rxi; //!< Field size byte counter TF_CKSUM cksum; //!< Checksum calculated of the data stream TF_CKSUM ref_cksum; //!< Reference checksum read from the message TF_TYPE type; //!< Collected message type number @@ -67,9 +67,9 @@ static struct TinyFrameStruct { // Those counters are used to optimize look-up times. // They point to the highest used slot number, // or close to it, depending on the removal order. - size_t count_id_lst; - size_t count_type_lst; - size_t count_generic_lst; + TF_COUNT count_id_lst; + TF_COUNT count_type_lst; + TF_COUNT count_generic_lst; // Buffer for building frames uint8_t sendbuf[TF_MAX_PAYLOAD_TX + TF_OVERHEAD_BYTES]; // TODO generate and send frames without a buffer @@ -210,19 +210,66 @@ void _TF_FN TF_Init(TF_PEER peer_bit) //region Listeners +/** + * Notify callback about ID listener demise & clean it + * + * @param lst - listener to clean + */ +static void _TF_FN cleanup_id_listener(TF_COUNT i, IdListener *lst) +{ + TF_MSG msg; + if (lst->fn == NULL) return; + + msg.userdata = lst->userdata; + msg.data = NULL; // this is a signal that the listener should clean up + lst->fn(&msg); + lst->fn = NULL; // Discard listener + + if (i == tf.count_id_lst - 1) { + tf.count_id_lst--; + } +} + +/** + * Clean up Type listener + * + * @param lst - listener to clean + */ +static inline void _TF_FN cleanup_type_listener(TF_COUNT i, TypeListener *lst) +{ + lst->fn = NULL; // Discard listener + if (i == tf.count_type_lst - 1) { + tf.count_type_lst--; + } +} + +/** + * Clean up Generic listener + * + * @param lst - listener to clean + */ +static inline void _TF_FN cleanup_generic_listener(TF_COUNT i, GenericListener *lst) +{ + lst->fn = NULL; // Discard listener + if (i == tf.count_generic_lst - 1) { + tf.count_generic_lst--; + } +} + bool _TF_FN TF_AddIdListener(TF_MSG *msg, TF_LISTENER cb, TF_TICKS timeout) { - size_t i; + TF_COUNT i; IdListener *lst; for (i = 0; i < TF_MAX_ID_LST; i++) { lst = &tf.id_listeners[i]; + // test for empty slot if (lst->fn == NULL) { lst->fn = cb; lst->id = msg->frame_id; lst->userdata = msg->userdata; lst->timeout_max = lst->timeout = timeout; if (i >= tf.count_id_lst) { - tf.count_id_lst = i + 1; + tf.count_id_lst = (TF_COUNT) (i + 1); } return true; } @@ -232,15 +279,16 @@ bool _TF_FN TF_AddIdListener(TF_MSG *msg, TF_LISTENER cb, TF_TICKS timeout) bool _TF_FN TF_AddTypeListener(TF_TYPE frame_type, TF_LISTENER cb) { - size_t i; + TF_COUNT i; TypeListener *lst; for (i = 0; i < TF_MAX_TYPE_LST; i++) { lst = &tf.type_listeners[i]; + // test for empty slot if (lst->fn == NULL) { lst->fn = cb; lst->type = frame_type; if (i >= tf.count_type_lst) { - tf.count_type_lst = i + 1; + tf.count_type_lst = (TF_COUNT) (i + 1); } return true; } @@ -250,14 +298,15 @@ bool _TF_FN TF_AddTypeListener(TF_TYPE frame_type, TF_LISTENER cb) bool _TF_FN TF_AddGenericListener(TF_LISTENER cb) { - size_t i; + TF_COUNT i; GenericListener *lst; for (i = 0; i < TF_MAX_GEN_LST; i++) { lst = &tf.generic_listeners[i]; + // test for empty slot if (lst->fn == NULL) { lst->fn = cb; if (i >= tf.count_generic_lst) { - tf.count_generic_lst = i + 1; + tf.count_generic_lst = (TF_COUNT) (i + 1); } return true; } @@ -267,15 +316,13 @@ bool _TF_FN TF_AddGenericListener(TF_LISTENER cb) bool _TF_FN TF_RemoveIdListener(TF_ID frame_id) { - size_t i; + TF_COUNT i; IdListener *lst; for (i = 0; i < tf.count_id_lst; i++) { lst = &tf.id_listeners[i]; + // test if live & matching if (lst->fn != NULL && lst->id == frame_id) { - lst->fn = NULL; - if (i == tf.count_id_lst - 1) { - tf.count_id_lst--; - } + cleanup_id_listener(i, lst); return true; } } @@ -284,15 +331,13 @@ bool _TF_FN TF_RemoveIdListener(TF_ID frame_id) bool _TF_FN TF_RemoveTypeListener(TF_TYPE type) { - size_t i; + TF_COUNT i; TypeListener *lst; for (i = 0; i < tf.count_type_lst; i++) { lst = &tf.type_listeners[i]; + // test if live & matching if (lst->fn != NULL && lst->type == type) { - lst->fn = NULL; - if (i == tf.count_type_lst - 1) { - tf.count_type_lst--; - } + cleanup_type_listener(i, lst); return true; } } @@ -301,15 +346,13 @@ bool _TF_FN TF_RemoveTypeListener(TF_TYPE type) bool _TF_FN TF_RemoveGenericListener(TF_LISTENER cb) { - size_t i; + TF_COUNT i; GenericListener *lst; for (i = 0; i < tf.count_generic_lst; i++) { lst = &tf.generic_listeners[i]; + // test if live & matching if (lst->fn == cb) { - lst->fn = NULL; - if (i == tf.count_generic_lst - 1) { - tf.count_generic_lst--; - } + cleanup_generic_listener(i, lst); return true; } } @@ -319,7 +362,7 @@ bool _TF_FN TF_RemoveGenericListener(TF_LISTENER cb) /** Handle a message that was just collected & verified by the parser */ static void _TF_FN TF_HandleReceivedMessage(void) { - size_t i; + TF_COUNT i; IdListener *ilst; TypeListener *tlst; GenericListener *glst; @@ -342,8 +385,9 @@ static void _TF_FN TF_HandleReceivedMessage(void) for (i = 0; i < tf.count_id_lst; i++) { ilst = &tf.id_listeners[i]; if (ilst->fn && ilst->id == msg.frame_id) { - msg.userdata = ilst->userdata; + msg.userdata = ilst->userdata; // pass userdata pointer to the callback if (ilst->fn(&msg)) return; + ilst->userdata = msg.userdata; // put it back (may have changed the pointer or set to NULL) } } msg.userdata = NULL; @@ -530,24 +574,25 @@ void _TF_FN TF_AcceptChar(unsigned char c) * @param len - payload size in bytes * @param explicit_id - ID to use in the frame (8-bit) * @param use_expl_id - whether to use the previous param - * @return nr of bytes in outbuff used by the frame, TF_ERROR (-1) on failure + * @return nr of bytes in outbuff used by the frame, 0 on failure */ -static inline int _TF_FN TF_Compose(uint8_t *outbuff, TF_ID *id_ptr, +static inline size_t _TF_FN TF_Compose(uint8_t *outbuff, TF_ID *id_ptr, TF_TYPE type, const uint8_t *data, TF_LEN data_len, TF_ID explicit_id, bool use_expl_id) { - int i; + char si; // signed small int + TF_LEN i; uint8_t b; TF_ID id; TF_CKSUM cksum; - int pos = 0; + size_t pos = 0; // can be needed to grow larger than TF_LEN CKSUM_RESET(cksum); // sanitize len if (data_len > TF_MAX_PAYLOAD_TX) { - return TF_ERROR; + return 0; } // Gen ID @@ -566,8 +611,8 @@ static inline int _TF_FN TF_Compose(uint8_t *outbuff, TF_ID *id_ptr, // DRY helper for writing a multi-byte variable to the buffer #define WRITENUM_BASE(type, num, xtra) \ - for (i = sizeof(type)-1; i>=0; i--) { \ - b = (uint8_t)(num >> (i*8) & 0xFF); \ + for (si = sizeof(type)-1; si>=0; si--) { \ + b = (uint8_t)(num >> (si*8) & 0xFF); \ outbuff[pos++] = b; \ xtra; \ } @@ -615,7 +660,7 @@ static inline int _TF_FN TF_Compose(uint8_t *outbuff, TF_ID *id_ptr, bool _TF_FN TF_Send(TF_MSG *msg, TF_LISTENER listener, TF_TICKS timeout) { - int len; + size_t len; len = TF_Compose(tf.sendbuf, &msg->frame_id, msg->type, @@ -624,11 +669,11 @@ bool _TF_FN TF_Send(TF_MSG *msg, TF_LISTENER listener, TF_TICKS timeout) msg->frame_id, msg->is_response); - if (len == TF_ERROR) return false; + if (len == 0) return false; if (listener) TF_AddIdListener(msg, listener, timeout); - TF_WriteImpl((const uint8_t *) tf.sendbuf, (TF_LEN)len); + TF_WriteImpl((const uint8_t *) tf.sendbuf, len); return true; } @@ -644,11 +689,12 @@ bool _TF_FN TF_Respond(TF_MSG *msg, bool renew) bool _TF_FN TF_RenewIdListener(TF_ID id) { - size_t i; + TF_COUNT i; IdListener *lst; for (i = 0; i < tf.count_id_lst; i++) { lst = &tf.id_listeners[i]; - if (lst->fn && lst->id == id) { + // test if live & matching + if (lst->fn != NULL && lst->id == id) { lst->timeout = lst->timeout_max; return true; } @@ -659,8 +705,7 @@ bool _TF_FN TF_RenewIdListener(TF_ID id) /** Timebase hook - for timeouts */ void _TF_FN TF_Tick(void) { - size_t i; - TF_MSG msg; + TF_COUNT i; IdListener *lst; // increment parser timeout (timeout is handled when receiving next byte) @@ -671,15 +716,11 @@ void _TF_FN TF_Tick(void) // decrement and expire ID listeners for (i = 0; i < tf.count_id_lst; i++) { lst = &tf.id_listeners[i]; - if (lst->fn && lst->timeout > 0) { - if (--lst->timeout != 0) continue; - - // Notify listener about timeout - msg.userdata = lst->userdata; - msg.data = NULL; // this is a signal that listener should clean up - - lst->fn(&msg); - lst->fn = NULL; // Discard listener + if (!lst->fn || lst->timeout == 0) continue; + // count down... + if (--lst->timeout == 0) { + // Listener has expired + cleanup_id_listener(i, lst); } } } diff --git a/TinyFrame.h b/TinyFrame.h index 11be373..4db7258 100644 --- a/TinyFrame.h +++ b/TinyFrame.h @@ -56,9 +56,14 @@ // Value of the SOF byte (if TF_USE_SOF_BYTE == 1) #define TF_SOF_BYTE 0x01 +// used for timeout tick counters - should be large enough for all used timeouts +typedef uint16_t TF_TICKS; + +// used in loops iterating over listeners +typedef uint8_t TF_COUNT; + //------------------------- End of user config ------------------------------ -typedef unsigned int TF_TICKS; //region Resolve data types @@ -123,9 +128,6 @@ typedef unsigned int TF_TICKS; //--------------------------------------------------------------------------- -// Return value indicating error state -#define TF_ERROR -1 - // Type-dependent masks for bit manipulation in the ID field #define TF_ID_MASK (TF_ID)(((TF_ID)1 << (sizeof(TF_ID)*8 - 1)) - 1) #define TF_ID_PEERBIT (TF_ID)((TF_ID)1 << ((sizeof(TF_ID)*8) - 1)) @@ -145,8 +147,7 @@ typedef struct _TF_MSG_STRUCT_ { TF_TYPE type; // received or sent message type const uint8_t *data; // buffer of received data or data to send. NULL = listener timed out, free userdata! TF_LEN len; // length of the buffer - void *userdata; // here's a place for custom data; this data will be - // stored with the listener + void *userdata; // here's a place for custom data; this data will be stored with the listener } TF_MSG; /** @@ -282,7 +283,7 @@ void TF_AcceptChar(uint8_t c); * * ! Implement this in your application code ! */ -extern void TF_WriteImpl(const uint8_t *buff, TF_LEN len); +extern void TF_WriteImpl(const uint8_t *buff, size_t len); /** * This function should be called periodically. diff --git a/test.c b/test.c index 2ec775e..e128839 100644 --- a/test.c +++ b/test.c @@ -11,7 +11,7 @@ static void dumpFrame(const uint8_t *buff, TF_LEN len); * This function should be defined in the application code. * It implements the lowest layer - sending bytes to UART (or other) */ -void TF_WriteImpl(const uint8_t *buff, TF_LEN len) +void TF_WriteImpl(const uint8_t *buff, size_t len) { printf("--------------------\n"); printf("\033[32mTF_WriteImpl - sending frame:\033[0m\n"); @@ -42,23 +42,24 @@ bool testIdListener(TF_MSG *msg) void main(void) { + TF_MSG msg; + const char *longstr = "Lorem ipsum dolor sit amet."; + // Set up the TinyFrame library TF_Init(TF_MASTER); // 1 = master, 0 = slave TF_AddGenericListener(myListener); printf("------ Simulate sending a message --------\n"); - TF_MSG msg; TF_ClearMsg(&msg); msg.type = 0x22; msg.data = (pu8)"Hello TinyFrame"; msg.len = 16; TF_Send(&msg, NULL, 0); - const char *longstr = "Lorem ipsum dolor sit amet."; msg.type = 0x33; msg.data = (pu8)longstr; - msg.len = strlen(longstr)+1; // add the null byte + msg.len = (TF_LEN) (strlen(longstr)+1); // add the null byte TF_Send(&msg, NULL, 0); msg.type = 0x44;