From 0da6259ae504a78407fe18c7a2d44f66eb9f2e88 Mon Sep 17 00:00:00 2001 From: joran2738 <101818067+joran2738@users.noreply.github.com> Date: Sat, 25 Nov 2023 13:33:36 +0100 Subject: [PATCH] solving reviews pt4 + some changes --- docs/udp_broadcast.md | 17 ++++-- project/Core/Inc/UDP_broadcast.h | 19 ++++--- project/Core/Src/UDP_broadcast.c | 89 +++++++++++++++++++++----------- project/Core/Src/main.c | 2 +- 4 files changed, 83 insertions(+), 44 deletions(-) diff --git a/docs/udp_broadcast.md b/docs/udp_broadcast.md index d8aa96d..c9d53aa 100644 --- a/docs/udp_broadcast.md +++ b/docs/udp_broadcast.md @@ -34,8 +34,8 @@ The 'udp_broadcast_init(uint16_t x_pos, uint16_t y_pos)' function does 4 things: void main(void){ ... if (udp_broadcast_init(270,255) != ERR_OK){ - ... - } + ... + } ... } ``` @@ -53,9 +53,16 @@ This function can be used seperately from [err_t udp_broadcast_init(uint16_t x_p void main(void){ ... - if (udp_broadcast_init(270,255) != ERR_OK || udp_broadcast_connection_init() != ERR_OK){ - ... - } + if (udp_broadcast_init(10,255) == ERR_OK){ + goto connected; + } + LOG_WARN(TAG,"error initializing udp connection, trying again in 500ms"); + HAL_Delay(500); + if(udp_broadcast_connection_init() != ERR_OK){ + LOG_WARN(TAG,"error initializing udp connection, check warnings from udp_broadcast_init() or udp_broadcast_connection_init()"); + } + +connected: ... } diff --git a/project/Core/Inc/UDP_broadcast.h b/project/Core/Inc/UDP_broadcast.h index a5f0047..ff26d10 100644 --- a/project/Core/Inc/UDP_broadcast.h +++ b/project/Core/Inc/UDP_broadcast.h @@ -20,16 +20,21 @@ #include "lcd_api.h" // Defines used by UDP callback -#define UDP_BROADCAST_MAX_DATA_SIZE 63 // Define the maximum expected data size -#define UDP_BROADCAST_UDP_QUESTION1 "Where are you?v1.0" // Expected question from UDP client -#define UDP_BROADCAST_FUNC_LEN 7 +#define UDP_BROADCAST_MAX_DATA_SIZE ((UDP_BROADCAST_MAX_NAME_SIZE * 2) - 2 + 25) // Define the maximum expected data size +#define UDP_BROADCAST_UDP_QUESTION1 "Where are you?v1.0" // Expected question from UDP client +#define UDP_BROADCAST_MAX_FUNC_LEN 7 #define UDP_BROADCAST_MAX_COLON_COMMA_COUNT 4 // Defines used by owner details -#define UDP_BROADCAST_MAX_NAME_SIZE 21 // Code automatically leaves 1 char for '\0' (actual length = length - 1) -#define UDP_BROADCAST_MAX_REPLY_SIZE 120 // Code automatically leaves 1 char for '\0' (actual length = length - 1) -#define UDP_BROADCAST_MAX_MAC_ADDR_LEN 19 -#define UDP_BROADCAST_MAX_EXTRA_REPLY_CHARS 27 +#define UDP_BROADCAST_MAX_REPLY_SIZE (UDP_BROADCAST_MAX_MAC_ADDR_LEN + sizeof(UDP_BROADCAST_REPLY_MIDDLE_TEXT) + (UDP_BROADCAST_MAX_NAME_SIZE * 2) + UDP_BROADCAST_MAX_REPLY_SIZE_EXTRA) +#define UDP_BROADCAST_REPLY_MIDDLE_TEXT "is present and my owner is" + +#define UDP_BROADCAST_MAX_MAC_ADDR_LEN 19 // Format is: "xx:xx:xx:xx:xx:xx" +#define UDP_BROADCAST_MAX_NAME_SIZE 21 // Code automatically leaves 1 char for '\0' (actual length = length - 1) +#define UDP_BROADCAST_MAX_REPLY_SIZE_EXTRA 20 // Just a bit extra + +#define UDP_BROADCAST_LCD_NAME_PRE_TEXT "New owner: " +#define UDP_BROADCAST_LCD_TEXT_SIZE (strlen(UDP_BROADCAST_LCD_NAME_PRE_TEXT) + UDP_BROADCAST_MAX_NAME_SIZE) /** diff --git a/project/Core/Src/UDP_broadcast.c b/project/Core/Src/UDP_broadcast.c index ae823f5..b24ed9d 100644 --- a/project/Core/Src/UDP_broadcast.c +++ b/project/Core/Src/UDP_broadcast.c @@ -20,6 +20,7 @@ static uint16_t owner_name_y_pos; // Functions static void udp_broadcast_set_owner_details_mac(void); +static void udp_broadcast_name_to_lcd(void); static uint8_t udp_broadcast_set_owner_details_name(const char* name); static uint8_t udp_broadcast_set_owner_details_surname(const char* surname); static uint8_t udp_broadcast_set_owner_details_reply(const char* reply); @@ -42,6 +43,26 @@ static void udp_broadcast_set_owner_details_mac(void) { } } +/** + * @fn void udp_broadcast_name_to_lcd(void) + * @brief prints the owner's name with + * @see UDP_BROADCAST_LCD_NAME_PRE_TEXT in front of it + */ + +static void udp_broadcast_name_to_lcd(void){ + char text[UDP_BROADCAST_LCD_TEXT_SIZE]; + + memset(text,' ',UDP_BROADCAST_LCD_TEXT_SIZE); // Fill with spaces + text[UDP_BROADCAST_LCD_TEXT_SIZE - 1] = '\0'; // Make the last a NULL byte + lcd_display_text(text, owner_name_x_pos, owner_name_y_pos, LCD_BLACK, LCD_WHITE, LCD_FONT12); + + snprintf(text, UDP_BROADCAST_LCD_TEXT_SIZE, "%s%s",UDP_BROADCAST_LCD_NAME_PRE_TEXT, + udp_owner.name); + + lcd_display_text(text, owner_name_x_pos, owner_name_y_pos, LCD_BLACK, LCD_WHITE, LCD_FONT12); +} + + /** * @fn uint8_t udp_broadcast_set_owner_details_name(owner_details_t*, const char*) * @brief set_owner_details_name() sets the owner's name in the owner_details_t struct @@ -55,14 +76,15 @@ static void udp_broadcast_set_owner_details_mac(void) { */ static uint8_t udp_broadcast_set_owner_details_name(const char* name) { + if (name == NULL) { LOG_WARN(TAG, "set_owner_details_name: string given is a NULL pointer"); return 1; } LOG_DEBUG(TAG, "set: %s", name); - lcd_display_text(" ", owner_name_x_pos, owner_name_y_pos, LCD_GREEN,LCD_FONT16); - strncpy(udp_owner.name, name, sizeof(udp_owner.name)-1); // -1: compensate for '\0' - lcd_display_text(name, owner_name_x_pos, owner_name_y_pos, LCD_GREEN, LCD_FONT16); + strncpy(udp_owner.name, name, sizeof(udp_owner.name) - 1); // -1: compensate for '\0' + + udp_broadcast_name_to_lcd(); return 0; } @@ -103,7 +125,7 @@ static uint8_t udp_broadcast_set_owner_details_reply(const char* reply) { return 1; } LOG_DEBUG(TAG, "set: %s", reply); - strncpy(udp_owner.reply, reply, sizeof(udp_owner.reply)-1); // -1: compensate for '\0' + strncpy(udp_owner.reply, reply, sizeof(udp_owner.reply) - 1); // -1: compensate for '\0' return 0; } @@ -122,7 +144,7 @@ static void udp_broadcast_format_reply(void) { udp_owner.mac_address[1], udp_owner.mac_address[2], udp_owner.mac_address[3], udp_owner.mac_address[4], udp_owner.mac_address[5]); - snprintf(reply_buf, UDP_BROADCAST_MAX_REPLY_SIZE, "%s is present and my owner is %s %s", mac_addr_str, udp_owner.surname, + snprintf(reply_buf, UDP_BROADCAST_MAX_REPLY_SIZE, "%s %s %s %s", mac_addr_str, UDP_BROADCAST_REPLY_MIDDLE_TEXT, udp_owner.surname, udp_owner.name); udp_broadcast_set_owner_details_reply(reply_buf); } @@ -188,26 +210,34 @@ char* udp_broadcast_get_owner_details_reply(void) { /** * @fn void udp_broadcast_check_function(const char[]) - * @brief checks what the UDP datagram asked to do if it was not @see UDP_QUESTION1 - * and processes the datagram + * @brief checks what the UDP datagram asked to do + * and processes the datagram if it was not @see UDP_QUESTION1 * * @param[in] data the datagram received on port 64000 + * + * @return checked + * - 0: a function was found and processed if necessary + * - 1: datagram didn't have a known function */ -static void udp_broadcast_check_function(const char data[UDP_BROADCAST_MAX_DATA_SIZE]) { - char func[UDP_BROADCAST_FUNC_LEN]; +static uint8_t udp_broadcast_check_function(const char data[UDP_BROADCAST_MAX_DATA_SIZE]) { + char func[UDP_BROADCAST_MAX_FUNC_LEN]; char buffer[UDP_BROADCAST_MAX_NAME_SIZE]; uint8_t enders[UDP_BROADCAST_MAX_COLON_COMMA_COUNT]; uint8_t counter = 0; uint8_t data_len = strlen(data); + if (strcmp(data, UDP_BROADCAST_UDP_QUESTION1) == 0) { + return 0; + } + memset(func, 0, sizeof(func)); memset(buffer, 0, sizeof(buffer)); - memcpy(func,data,UDP_BROADCAST_FUNC_LEN-1); + memcpy(func,data,UDP_BROADCAST_MAX_FUNC_LEN - 1); if (strcmp(func, "func1:") != 0) { LOG_WARN(TAG, "udp_broadcast_check_function: datagram does not contain function that's currently available"); - return; + return 1; } for (uint8_t i = 0; i < data_len && counter < UDP_BROADCAST_MAX_COLON_COMMA_COUNT; i++) { if ((data[i] == ',' || data[i] == ':')) { @@ -215,7 +245,7 @@ static void udp_broadcast_check_function(const char data[UDP_BROADCAST_MAX_DATA_ counter++; } } - if (enders[2] - enders[1] < UDP_BROADCAST_MAX_NAME_SIZE + 1 && data_len - enders[3] < UDP_BROADCAST_MAX_NAME_SIZE + 1 + if (enders[2] - enders[1] < UDP_BROADCAST_MAX_NAME_SIZE + 2 && data_len - enders[3] < UDP_BROADCAST_MAX_NAME_SIZE + 2 && strncmp(data + enders[0], ":name", 5) == 0 && strncmp(data + enders[2], ", surname", 9) == 0) { counter = 0; @@ -224,7 +254,7 @@ static void udp_broadcast_check_function(const char data[UDP_BROADCAST_MAX_DATA_ counter++; } if (buffer[0]=='\0') { - strncpy(buffer, "name", sizeof(buffer)-1); // -1: compensate for '\0', just for safety, not really needed + strncpy(buffer, "name", sizeof(buffer) - 1); // -1: compensate for '\0' } LOG_INFO(TAG, "new owner name:%s", buffer); udp_broadcast_set_owner_details_name(buffer); @@ -235,11 +265,15 @@ static void udp_broadcast_check_function(const char data[UDP_BROADCAST_MAX_DATA_ counter++; } if (buffer[0]=='\0') { - strncpy(buffer, "default", sizeof(buffer)-1); // -1: compensate for '\0', just for safety, not really needed + strncpy(buffer, "default", sizeof(buffer) - 1); // -1: compensate for '\0' } LOG_INFO(TAG, "new owner surname:%s", buffer); udp_broadcast_set_owner_details_surname(buffer); udp_broadcast_format_reply(); + return 0; + }else{ + LOG_WARN(TAG,"udp_broadcast_check_function: function didn't receive the right formatting"); + return 1; } } @@ -278,38 +312,31 @@ static void udp_receive_callback(void* arg, } pc = (char*)p->payload; len = p->tot_len; + if (len >= UDP_BROADCAST_MAX_DATA_SIZE) { // >= : only if it's smaller to compensate for '\0' + LOG_WARN(TAG, "udp_receive_callback: input buffer was bigger than max size %d", UDP_BROADCAST_MAX_DATA_SIZE); + return; + } + p_data = pbuf_alloc(PBUF_TRANSPORT, sizeof(udp_owner.reply), PBUF_RAM); if (p_data == NULL) { LOG_WARN(TAG, "udp_receive_callback: unable to allocate data buffer for reply"); goto defer; - } if (len > UDP_BROADCAST_MAX_DATA_SIZE) { - LOG_WARN(TAG, "udp_receive_callback: input buffer was bigger than max size %d", UDP_BROADCAST_MAX_DATA_SIZE); - goto defer; } for (size_t i = 0; i < len; i++) { data[i] = pc[i]; } LOG_INFO(TAG, "udp_receive_callback: received data from %s at port: %d: %s", source_ip_str, port, data); - if (strcmp(data, UDP_BROADCAST_UDP_QUESTION1) == 0) { + LOG_INFO(TAG, "udp_receive_callback: checking which function was called"); + + if(!udp_broadcast_check_function(data)){ // Should return 0 to reply p_data->payload = udp_owner.reply; p_data->len = strlen(udp_owner.reply); p_data->tot_len = strlen(udp_owner.reply); - udp_sendto(connection, p_data, addr, 64000); /* Was using the sending port of the pc, - * this is not the port that Qt is listening to - */ + udp_sendto(connection, p_data, addr, 64000); // QT app listens on port 64000 LOG_INFO(TAG, "udp_receive_callback: tried to reply to %s at port: %d: %s", source_ip_str, 64000, udp_owner.reply); - goto defer; } - LOG_INFO(TAG, "udp_receive_callback: checking which function was called"); - udp_broadcast_check_function(data); - p_data->payload = udp_owner.reply; - p_data->len = strlen(udp_owner.reply); - p_data->tot_len = strlen(udp_owner.reply); - udp_sendto(connection, p_data, addr, 64000); /* Was using the sending port of the pc, - * this is not the port that Qt is listening to - */ - LOG_INFO(TAG, "udp_receive_callback: tried to reply to %s at port: %d: %s", source_ip_str, 64000, udp_owner.reply); + defer: pbuf_free(p); diff --git a/project/Core/Src/main.c b/project/Core/Src/main.c index 53afcb7..4c3890a 100644 --- a/project/Core/Src/main.c +++ b/project/Core/Src/main.c @@ -136,7 +136,7 @@ int main(void) // Initialize the UDP broadcast service - if (udp_broadcast_init(270,255) == ERR_OK){ + if (udp_broadcast_init(10,255) == ERR_OK){ goto connected; } LOG_WARN(TAG,"error initializing udp connection, trying again in 500ms");