solving reviews pt4 + some changes

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:
     ...
 }
 

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_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_UDP_QUESTION1 "Where are you?v1.0"                            // Expected question from UDP client
-#define UDP_BROADCAST_FUNC_LEN 7
+#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 (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_MAX_REPLY_SIZE 120          // Code automatically leaves 1 char for '\0' (actual length = length - 1)
+#define UDP_BROADCAST_REPLY_MIDDLE_TEXT "is present and my owner is"
-#define UDP_BROADCAST_MAX_MAC_ADDR_LEN 19
+
-#define UDP_BROADCAST_MAX_EXTRA_REPLY_CHARS 27
+#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)
 
 
 /**

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'
-    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);
+    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
+ * @brief checks what the UDP datagram asked to do
- * and processes the datagram
+ * 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]) {
+static uint8_t udp_broadcast_check_function(const char data[UDP_BROADCAST_MAX_DATA_SIZE]) {
-    char func[UDP_BROADCAST_FUNC_LEN];
+    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,
+        udp_sendto(connection, p_data, addr, 64000);    // QT app listens on port 64000
-                                                         * 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);
-        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);

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");