Made some requested changes from reviews

project/Core/Inc/mqtt_application.h

@@ -10,6 +10,6 @@
 /**
  *  @brief Initialize MQTT application
  */
-void mqtt_application_init(void);
+uint8_t mqtt_application_init(void);
 
 #endif /* INC_MQTTA_H_ */

project/Core/Src/main.c

@@ -28,7 +28,7 @@
 #include "log.h"
 #include "llfs.h"
 #include "lcd_api.h"
-#include <mqtt_application.h>
+#include "mqtt_application.h"
 #include "tftp.h"
 
 /* USER CODE END Includes */

project/Core/Src/mqtt_application.c

@@ -4,7 +4,6 @@
  * @author RobinVdB
  */
 
-#include <mqtt_application.h>
 #include <string.h>
 #include "httpd.h"
 #include "lcd_api.h"
@@ -12,6 +11,7 @@
 #include "lwip/ip_addr.h"
 #include "mdns.h"
 #include "mqtt.h"
+#include "mqtt_application.h"
 
 #define LOGGER_LEVEL_INFO
 #include "log.h"
@@ -64,9 +64,7 @@ static const char* TAG = "MQTT";
  * @param[in] result    Whether the publish was successful or not
  */
 static void mqtt_pub_request_cb(void* arg, err_t result) {
-    if (result != ERR_OK) {
     LOG_DEBUG(TAG, "Publish result: %d", result);
-    }
 }
 
 /**
@@ -79,15 +77,14 @@ static void mqtt_pub_request_cb(void* arg, err_t result) {
 static void publish_data(mqtt_client_t* client, void* arg) {
     char pub_payload[200] = {0};
     err_t err;
-    size_t max_files = MAX_FILES;
     size_t num_files;
-    llfs_file_t file_list[max_files];
+    llfs_file_t file_list[MAX_FILES];
     u8_t qos = PUBLISH_QOS;
     u8_t retain = PUBLISH_RETAIN;
 
     LOG_DEBUG(TAG, "Entering publish");
 
-    num_files = llfs_file_list(file_list, max_files, "*.bmp");
+    num_files = llfs_file_list(file_list, MAX_FILES, "*.bmp");
 
     if (num_files == 0) {
         strncpy(pub_payload, "No images found", sizeof(pub_payload));
@@ -108,12 +105,12 @@ static void publish_data(mqtt_client_t* client, void* arg) {
 
     pub_payload[0] = '\0';
 
-    num_files = llfs_file_list(file_list, max_files, "*.gif");
+    num_files = llfs_file_list(file_list, MAX_FILES, "*.gif");
 
     if (num_files == 0) {
-        strcpy(pub_payload, "No gifs found");
+        strncpy(pub_payload, "No gifs found", sizeof(pub_payload));
     } else {
-        strcpy(pub_payload, "Available gifs: ");
+        strncpy(pub_payload, "Available gifs: ", sizeof(pub_payload));
         for (size_t i = 0; i < num_files; i++) {
             // Concatenate file names into the payload string
             strncat(pub_payload, file_list[i].name, sizeof(pub_payload) - strlen(pub_payload) - 1);
@@ -140,16 +137,22 @@ static void mqtt_incoming_publish_cb(void* arg, const char* topic, uint32_t tot_
     // Check for which topic a publish was received
     if (strcmp(topic, "input/setText") == 0) {
         inpub_id = set_text;
-    } else if (strcmp(topic, "input/setImage") == 0) {
+        return;
+    }
+    if (strcmp(topic, "input/setImage") == 0) {
         inpub_id = set_image;
-    } else if (strcmp(topic, "input/setTextColor") == 0) {
+        return;
+    }
+    if (strcmp(topic, "input/setTextColor") == 0) {
         inpub_id = set_text_color;
-    } else if (strcmp(topic, "input/setColor") == 0) {
+        return;
+    }
+    if (strcmp(topic, "input/setColor") == 0) {
         inpub_id = set_color;
-    } else {
+        return;
+    }
     // In case of wrong topic
     inpub_id = other_topic;
-    }
 }
 
 /**
@@ -165,8 +168,11 @@ static void mqtt_incoming_data_cb(void* arg, const uint8_t* data, uint16_t len,
     char data_buffer[len + 1];
     lcd_gif_t* gif;
 
-    LOG_INFO(TAG, "Incoming publish payload with length %d, flags %u", len, (unsigned int)flags);
-    if (flags & MQTT_DATA_FLAG_LAST) {
+    LOG_INFO(TAG, "Incoming publish payload with length %d, flags %d", len, flags);
+    if (!(flags & MQTT_DATA_FLAG_LAST)) {
+       LOG_WARN(TAG, "incoming data too big to fit in buffer.");
+       return;
+    }
     memcpy(data_buffer, data, len);
     data_buffer[len] = '\0';
     switch (inpub_id) {
@@ -204,8 +210,6 @@ static void mqtt_incoming_data_cb(void* arg, const uint8_t* data, uint16_t len,
         default:
             LOG_INFO(TAG, "Publish received on wrong topic, incoming data ignored.");
     }
-    } else {
-    }
 }
 
 /**
@@ -230,7 +234,16 @@ static void mqtt_sub_request_cb(void* arg, err_t result) {
 static void mqtt_connection_cb(mqtt_client_t* client, void* arg, mqtt_connection_status_t status) {
     err_t err;
 
-    if (status == MQTT_CONNECT_ACCEPTED) {
+    if (status != MQTT_CONNECT_ACCEPTED) {
+        LOG_INFO(TAG, "mqtt_connection_cb: Disconnected, reason: %d", status);
+
+        while (connection_attempt_counter < ATTEMPT_RECONNECT_AMOUNT) {
+            connection_attempt_counter++;
+            // Try to reconnect
+            mosquitto_connect(client);
+        }
+        return;
+    }
     LOG_INFO(TAG, "Successfully connected");
 
     connection_attempt_counter = 0;
@@ -245,15 +258,6 @@ static void mqtt_connection_cb(mqtt_client_t* client, void* arg, mqtt_connection
 
     // Publish list of images here
     publish_data(client, NULL);
-    } else {
-        LOG_INFO(TAG, "mqtt_connection_cb: Disconnected, reason: %d", status);
-
-        while (connection_attempt_counter < ATTEMPT_RECONNECT_AMOUNT) {
-            connection_attempt_counter++;
-            // Try to reconnect
-            mosquitto_connect(client);
-        }
-    }
 }
 
 /**
@@ -277,17 +281,16 @@ static void mosquitto_connect(mqtt_client_t* client) {
     err = mqtt_client_connect(client, &server_ip, server_port, mqtt_connection_cb, 0, &ci);
     if (err != ERR_OK) {
         LOG_DEBUG(TAG, "mqtt_connect return %d", err);
+        return;
     }
-    if (err == ERR_OK) {
     LOG_DEBUG(TAG, "Went into mqtt_client_connect; mqtt_connect return %d", err);
-    }
 }
 
 /**
  * @brief Init function for the mosquitto application of the assignment
  * Gives the global variables a value and calls the mosquitto_connect function
  */
-void mqtt_application_init(void) {
+uint8_t mqtt_application_init(void) {
     color = LCD_BLACK;
     bgcolor = LCD_WHITE;
     font = LCD_FONT16;
@@ -296,10 +299,14 @@ void mqtt_application_init(void) {
     connection_attempt_counter = 0;
 
     mqtt_client_t* client = mqtt_client_new();
-    if (client != NULL) {
+    if (client == NULL) {
+        LOG_CRIT(TAG, "%s: client == NULL", __func__);
+        return 1;
+    }
     LOG_DEBUG(TAG, "Starting connection test");
     mosquitto_connect(client);
-    }
+
+    return 0;
 }
 
 /**
@@ -309,8 +316,6 @@ void mqtt_application_init(void) {
  * @return color        Define to use with the LCD_API
  */
 uint32_t color_picker(char* color_in) {
-    uint32_t output = LCD_BLACK;
-
     if (strcmp((const char*)color_in, "blue") == 0) {
         return LCD_BLUE;
     }
@@ -390,5 +395,5 @@ uint32_t color_picker(char* color_in) {
         return LCD_TRANSPARENT;
     }
 
-    return output;
+    return LCD_BLACK;
 }