From 47474bdc70f893cbc18c8ded6f7d7f2933a360bc Mon Sep 17 00:00:00 2001 From: dzollo Date: Wed, 4 Sep 2019 21:33:09 -0700 Subject: [PATCH 1/3] Add allmsg callback API for libsbp C library --- c/include/libsbp/sbp.h | 13 ++++++++ c/src/sbp.c | 67 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 79 insertions(+), 1 deletion(-) diff --git a/c/include/libsbp/sbp.h b/c/include/libsbp/sbp.h index 75db2327b8..debcfa9673 100644 --- a/c/include/libsbp/sbp.h +++ b/c/include/libsbp/sbp.h @@ -28,6 +28,9 @@ extern "C" { #define SBP_OK_CALLBACK_EXECUTED 1 /** Return value indicating message decoded with no associated callback in sbp_process. */ #define SBP_OK_CALLBACK_UNDEFINED 2 +/** Return value indicating message decoded and only an all_msg function was + * executed. */ +#define SBP_OK_ALLMSG_FN_EXECUTED_ONLY 3 /** Return value indicating an error with the callback (function defined). */ #define SBP_CALLBACK_ERROR -1 /** Return value indicating a CRC error. */ @@ -46,6 +49,8 @@ extern "C" { /** SBP callback function prototype definition. */ typedef void (*sbp_msg_callback_t)(u16 sender_id, u8 len, u8 msg[], void *context); +typedef void (*sbp_allmsg_fn_t)(u16 sender_id, u16 msg_type, u8 len, u8 msg[], + void *context); /** SBP callback node. * Forms a linked list of callbacks. @@ -76,12 +81,20 @@ typedef struct { u8 msg_buff[256]; void* io_context; sbp_msg_callbacks_node_t* sbp_msg_callbacks_head; + sbp_allmsg_fn_t allmsg_fn; + void *allmsg_context; } sbp_state_t; /** \} */ +/* registers a callback to be called on a particular msg_type */ s8 sbp_register_callback(sbp_state_t* s, u16 msg_type, sbp_msg_callback_t cb, void* context, sbp_msg_callbacks_node_t *node); +/* sets a function to be called on ANY deframed message */ +s8 sbp_set_allmsg_fn(sbp_state_t *s, sbp_allmsg_fn_t fn, void *context); +/* gets the function to be called on ANY deframed message */ +sbp_allmsg_fn_t sbp_get_allmsg_fn(sbp_state_t *s); +s8 sbp_remove_allmsg_fn(sbp_state_t *s); s8 sbp_remove_callback(sbp_state_t *s, sbp_msg_callbacks_node_t *node); void sbp_clear_callbacks(sbp_state_t* s); void sbp_state_init(sbp_state_t *s); diff --git a/c/src/sbp.c b/c/src/sbp.c index 3e2efb068b..ed31123fa0 100644 --- a/c/src/sbp.c +++ b/c/src/sbp.c @@ -53,6 +53,21 @@ * * where `SBP_MY_MSG_TYPE` is the numerical identifier of your message type. * + * If you'd like to call a function for EVERY sbp message, setup an allmsg function + * with the type sbp_allmsg_fn_t. It must be of the form: + * + * ~~~ + * void my_allmsg_fn(u16 sender_id, u16 msg_type, u8 len, u8 msg[], void *context) + * { + * // Process msg. + * } + * ~~~ + * + * Then register the allmsg callback with the SBP library as follows: + * ~~~ + * sbp_register_allmsg_fn(&sbp_state, &my_allmsg_fn, &context); + * ~~~ + * * You must now call sbp_process() periodically whenever you have received SBP * data to be processed, e.g. from the serial port. Remember sbp_process() may * not use all available data so keep calling sbp_process() until all the @@ -227,6 +242,44 @@ s8 sbp_remove_callback(sbp_state_t *s, sbp_msg_callbacks_node_t *node) return SBP_CALLBACK_ERROR; } +/** Set a function to be called for ANY message type. + * + * \param s pointer to sbp_state_t struct in use + * \param fn Pointer to message callback function + * \param context Pointer to context for callback function + * \return `SBP_OK` (0) if successful, `SBP_NULL_ERROR` if fn is NULL + * + */ +s8 sbp_set_allmsg_fn(sbp_state_t *s, sbp_allmsg_fn_t fn, void *context) { + /* Check our callback function pointer isn't NULL. */ + if (0 == fn) { + return SBP_NULL_ERROR; + } + s->allmsg_fn = fn; + s->allmsg_context = context; + return SBP_OK; +} + +/** Retrieve the callback for ANY message type. + * + * \param s pointer to sbp_state_t struct in use + * \return sbp_allmsg_fn_t* to allmsg fn pointer or NULL if none + * + */ +sbp_allmsg_fn_t sbp_get_allmsg_fn(sbp_state_t *s) { + return s->allmsg_fn; +} + +/** Remove allmsg function pointer and context. + * + * \return `SBP_OK` (0) if successful + */ +s8 sbp_remove_allmsg_fn(sbp_state_t *s) { + s->allmsg_fn = 0; + s->allmsg_context = 0; + return SBP_OK; +} + /** Clear all registered callbacks. * This is probably only useful for testing but who knows! */ @@ -252,6 +305,9 @@ void sbp_state_init(sbp_state_t *s) /* Clear the callbacks, if any, currently in s */ sbp_clear_callbacks(s); + + /* Clear the allmsg_fn, if any, currently set on s */ + sbp_remove_allmsg_fn(s); } @@ -302,7 +358,8 @@ void sbp_state_set_io_context(sbp_state_t *s, void *context) * \return `SBP_OK` (0) if successful but no complete message yet, * `SBP_OK_CALLBACK_EXECUTED` (1) if message decoded and callback executed, * `SBP_OK_CALLBACK_UNDEFINED` (2) if message decoded with no associated - * callback, and `SBP_CRC_ERROR` (-2) if a CRC error + * callback, SBP_OK_ALLMSG_FN_EXECUTED_ONLY (3) if only the generic + * callback for every msg was called, and `SBP_CRC_ERROR` (-2) if a CRC error * has occurred. Thus can check for >0 to ensure good processing. */ s8 sbp_process(sbp_state_t *s, s32 (*read)(u8 *buff, u32 n, void *context)) @@ -412,6 +469,8 @@ s8 sbp_process(sbp_state_t *s, s32 (*read)(u8 *buff, u32 n, void *context)) * \return `SBP_OK_CALLBACK_EXECUTED` (1) if message decoded and callback executed, * `SBP_OK_CALLBACK_UNDEFINED` (2) if message decoded with no associated * callback. + * `SBP_OK_ALLMSG_FN_EXECUTED_ONLY` (3) if message decoded and only + * allmsg callback was called */ s8 sbp_process_payload(sbp_state_t *s, u16 sender_id, u16 msg_type, u8 msg_len, u8 payload[]) { @@ -423,6 +482,12 @@ s8 sbp_process_payload(sbp_state_t *s, u16 sender_id, u16 msg_type, u8 msg_len, ret = SBP_OK_CALLBACK_EXECUTED; } } + if (0 != s->allmsg_fn) { + (s->allmsg_fn)(sender_id, msg_type, msg_len, payload, s->allmsg_context); + if (ret == SBP_OK_CALLBACK_UNDEFINED) { + ret = SBP_OK_ALLMSG_FN_EXECUTED_ONLY; + } + } return ret; } From 710ae269c670af6495cba6dc73fe8583f833df63 Mon Sep 17 00:00:00 2001 From: dzollo Date: Wed, 4 Sep 2019 21:33:24 -0700 Subject: [PATCH 2/3] Add allmsg API tests to c library --- c/test/check_sbp.c | 148 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 148 insertions(+) diff --git a/c/test/check_sbp.c b/c/test/check_sbp.c index ae504cb25c..03edbeda00 100644 --- a/c/test/check_sbp.c +++ b/c/test/check_sbp.c @@ -78,6 +78,7 @@ void printy_callback(u16 sender_id, u8 len, u8 msg[], void* context) u32 n_callbacks_logged; u16 last_sender_id; +u16 last_msg_type; u8 last_len; u8 last_msg[256]; void* last_context; @@ -101,6 +102,18 @@ void logging_callback(u16 sender_id, u8 len, u8 msg[], void* context) /*printy_callback(sender_id, len, msg);*/ } +void logging_allmsg_fn(u16 sender_id, u16 msg_type, u8 len, u8 msg[], void* context) +{ + n_callbacks_logged++; + last_sender_id = sender_id; + last_msg_type = msg_type; + last_len = len; + last_context = context; + memcpy(last_msg, msg, len); + + /*printy_callback(sender_id, len, msg);*/ +} + void test_callback(u16 sender_id, u8 len, u8 msg[], void* context) { /* Do nothing. */ @@ -118,6 +131,17 @@ void test_callback2(u16 sender_id, u8 len, u8 msg[], void* context) (void)context; } +void test_allmsg_fn(u16 sender_id, u16 msg_type, u8 len, u8 msg[], + void* context) +{ + /* Do nothing. */ + (void)sender_id; + (void)msg_type; + (void)len; + (void)msg; + (void)context; +} + sbp_msg_callbacks_node_t* sbp_find_callback(sbp_state_t *s, u16 msg_type) { /* If our list is empty, return NULL. */ @@ -451,6 +475,128 @@ START_TEST(test_callbacks) } END_TEST +START_TEST(test_allmsg_corner_cases) +{ + + sbp_state_t s; + sbp_state_init(&s); + + /* Start with no callbacks registered. */ + sbp_remove_allmsg_fn(&s); + + fail_unless(sbp_set_allmsg_fn(&s, 0, 0) == SBP_NULL_ERROR, + "sbp_register_callback should return an error if cb is NULL"); + + fail_unless(sbp_set_allmsg_fn(&s, &test_allmsg_fn, 0) == SBP_OK, + "sbp_set_allmsg_fn should work"); + + fail_unless(sbp_get_allmsg_fn(&s) == &test_allmsg_fn, + "sbp_get_allmsg_fn should work"); + + sbp_remove_allmsg_fn(&s); + + fail_unless(sbp_get_allmsg_fn(&s) == 0, + "sbp_get_allmsg_fn should now fail"); +} +END_TEST + +START_TEST(test_sbp_set_allmsg_cb) +{ + /* TODO: Tests with different read function behaviour. */ + + sbp_state_t s; + sbp_state_init(&s); + sbp_state_set_io_context(&s, &DUMMY_MEMORY_FOR_IO); + logging_reset(); + dummy_reset(); + u8 ret = sbp_set_allmsg_fn(&s, &logging_allmsg_fn, &DUMMY_MEMORY_FOR_CALLBACKS); + fail_unless(ret == SBP_OK, + "registering all callback didn't work"); + + u8 test_data[] = { 0x01, 0x02, 0x03, 0x04 }; + + sbp_send_message(&s, 0x2269, 0x42, sizeof(test_data), test_data, &dummy_write); + + while (dummy_rd < dummy_wr) { + fail_unless(sbp_process(&s, &dummy_read) >= SBP_OK, + "sbp_process threw an error!"); + } + + fail_unless(n_callbacks_logged == 1, + "one callback should have been logged"); + fail_unless(last_sender_id == 0x42, + "sender_id decoded incorrectly"); + fail_unless(last_len == sizeof(test_data), + "len decoded incorrectly. len is %u", last_len); + fail_unless(last_msg_type == 0x2269, + "msg_id decoded incorrectly"); + fail_unless(memcmp(last_msg, test_data, sizeof(test_data)) + == 0, + "test data decoded incorrectly"); + fail_unless(last_context == &DUMMY_MEMORY_FOR_CALLBACKS, + "context pointer incorrectly passed"); + + /* now add a regular callback and make sure BOTH were called for the same message*/ + static sbp_msg_callbacks_node_t n; + logging_reset(); + dummy_reset(); + sbp_register_callback(&s, 0x2269, &logging_callback, &DUMMY_MEMORY_FOR_CALLBACKS, &n); + + sbp_send_message(&s, 0x2269, 0x42, sizeof(test_data), test_data, &dummy_write); + + while (dummy_rd < dummy_wr) { + fail_unless(sbp_process(&s, &dummy_read) >= SBP_OK, + "sbp_process threw an error!"); + } + + /* should have two callbacks, one for all, one for normal */ + fail_unless(n_callbacks_logged == 2, + "two callback should have been logged"); + fail_unless(last_sender_id == 0x42, + "sender_id decoded incorrectly"); + fail_unless(last_len == sizeof(test_data), + "len decoded incorrectly. len is %u", last_len); + fail_unless(last_msg_type == 0x2269, + "msg_id decoded incorrectly"); + fail_unless(memcmp(last_msg, test_data, sizeof(test_data)) + == 0, + "test data decoded incorrectly"); + fail_unless(last_context == &DUMMY_MEMORY_FOR_CALLBACKS, + "context pointer incorrectly passed"); + + /* now remove the callback for all message types and make sure regular callback works */ + logging_reset(); + dummy_reset(); + ret = sbp_remove_allmsg_fn(&s); + fail_unless(ret == SBP_OK, + "removing the all callback didn't work"); + sbp_send_message(&s, 0x2269, 0x42, sizeof(test_data), test_data, &dummy_write); + + while (dummy_rd < dummy_wr) { + fail_unless(sbp_process(&s, &dummy_read) >= SBP_OK, + "sbp_process threw an error!"); + } + fail_unless(n_callbacks_logged == 1, + "one callbacks should have been logged"); + + /* now remove every callback */ + logging_reset(); + dummy_reset(); + ret = sbp_remove_allmsg_fn(&s); + fail_unless(ret == SBP_OK, + "removing the all callback didn't work"); + sbp_send_message(&s, 0x2269, 0x42, sizeof(test_data), test_data, &dummy_write); + + while (dummy_rd < dummy_wr) { + fail_unless(sbp_process(&s, &dummy_read) >= SBP_OK, + "sbp_process threw an error!"); + } + fail_unless(n_callbacks_logged == 1, + "one callbacks should have been logged"); + +} +END_TEST + Suite* sbp_suite(void) { Suite *s = suite_create("SBP"); @@ -461,6 +607,8 @@ Suite* sbp_suite(void) tcase_add_test(tc_core, test_callbacks); tcase_add_test(tc_core, test_sbp_send_message); tcase_add_test(tc_core, test_sbp_process); + tcase_add_test(tc_core, test_allmsg_corner_cases); + tcase_add_test(tc_core, test_sbp_set_allmsg_cb); suite_add_tcase(s, tc_core); From 7ca15c7adbea0ccc76de4ba8075e6547236053b6 Mon Sep 17 00:00:00 2001 From: dzollo Date: Wed, 4 Sep 2019 21:44:22 -0700 Subject: [PATCH 3/3] update c version.h from latest github tag --- c/include/libsbp/version.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/c/include/libsbp/version.h b/c/include/libsbp/version.h index f4e9c7e583..e8930299a5 100644 --- a/c/include/libsbp/version.h +++ b/c/include/libsbp/version.h @@ -25,7 +25,7 @@ /** Protocol minor version. */ #define SBP_MINOR_VERSION 6 /** Protocol patch version. */ -#define SBP_PATCH_VERSION 4 +#define SBP_PATCH_VERSION 6 /** \} */