[iot/goam] Major reordering and commenting of codebase

8 files changed, 239 insertions, 128 deletions

diff --git a/Year 3/Internet of things/goam/main/buttons.c b/Year 3/Internet of things/goam/main/buttons.c
index 18b9df5..9aa3cf4 100644
--- a/Year 3/Internet of things/goam/main/buttons.c
+++ b/Year 3/Internet of things/goam/main/buttons.c
@@ -1,16 +1,17 @@
 #include "freertos/FreeRTOS.h"
 #include "freertos/task.h"
 #include "freertos/queue.h"
+#include "esp_log.h"
 
 #include "driver/gpio.h"
 #include "buttons.h"
 
 #define ESP_INTR_FLAG_DEFAULT 0
 
-#define gpio_to_bitmask(pin) (1ULL << pin)
-
 static QueueHandle_t gpio_button_queue = NULL;
 
+static const char* TAG = "buttons";
+
 /*
  * Handlers
  */
@@ -21,7 +22,7 @@ static void IRAM_ATTR gpio_button_interrupt_handler(void* arg)
 	xQueueSendFromISR(gpio_button_queue, &gpio_num, NULL);
 }
 
-static void (*button_on[40])() = { NULL };
+static void (*button_on[40])()  = { NULL };
 static void (*button_off[40])() = { NULL };
 
 static void on_button_event(void* arg)
@@ -30,7 +31,7 @@ static void on_button_event(void* arg)
 	while (true) {
 		if (!xQueueReceive(gpio_button_queue, &io_num, portMAX_DELAY)) continue;
 
-		printf("GPIO[%ld] intr, val: %d\n", io_num, gpio_get_level(io_num));
+		ESP_LOGD(TAG, "GPIO[%ld] intr, val: %d", io_num, gpio_get_level(io_num));
 
 		if (gpio_get_level(io_num)) {
 			if (button_on[io_num]) button_on[io_num]();
@@ -39,6 +40,12 @@ static void on_button_event(void* arg)
 			if (button_off[io_num]) button_off[io_num]();
 		}
 
+		// If any button was pressed, this effectively blocks all other button presses
+		// that happened in the meantime.
+		// Generally it's bad, but in my case the button would trigger multiple presses
+		// when physically pressed once, and there is only one so its alright.
+		// Better logic would be to remove all repeated occurences of the same current
+		// event, but again, here we don't need to.
 		vTaskDelay(100 / portTICK_PERIOD_MS);
 		xQueueReset(gpio_button_queue);
 	}
@@ -56,8 +63,12 @@ void buttons_init() {
 
 	//install gpio isr service
 	gpio_install_isr_service(ESP_INTR_FLAG_DEFAULT);
+
+	ESP_LOGI(TAG, "Initialized queue and ISR service");
 }
 
+#define gpio_to_bitmask(pin) (1ULL << pin)
+
 void add_push_button(unsigned pin, void (*on_press)(), void (*off_press)()) {
 	gpio_config_t o_conf = {
 		.intr_type = GPIO_INTR_POSEDGE,
@@ -71,8 +82,13 @@ void add_push_button(unsigned pin, void (*on_press)(), void (*off_press)()) {
 		.pull_up_en = 0
 	};
 	gpio_config(&o_conf);
-	//hook isr handler for specific gpio pin
+	// Handler has to do as little work as humanly possible, and return execution
+	// as soon as possible, while also being in RAM (IRAM_ATTR)
+	// We get a crash if it takes too long, so we canot register on_press as the
+	// handler
 	gpio_isr_handler_add(pin, gpio_button_interrupt_handler, (void*)pin);
-	button_on[pin] = on_press;
+	button_on[pin]  = on_press;
 	button_off[pin] = off_press;
+
+	ESP_LOGD(TAG, "GPIO[%d] added intr handlers, on_press: %d, off_press: %d", pin, on_press != NULL, off_press != NULL);
 }
diff --git a/Year 3/Internet of things/goam/main/display.c b/Year 3/Internet of things/goam/main/display.c
index 668721e..081e113 100644
--- a/Year 3/Internet of things/goam/main/display.c
+++ b/Year 3/Internet of things/goam/main/display.c
@@ -7,6 +7,10 @@
 
 static const char* TAG = "display";
 
+/*
+ * I2C settings
+ */
+
 #define SCL_IO_PIN 19
 #define SDA_IO_PIN 18
 #define MASTER_FREQUENCY CONFIG_I2C_MASTER_FREQUENCY
@@ -18,32 +22,56 @@ static void i2c_init() {
 		.scl_io_num = SCL_IO_PIN,
 		.sda_pullup_en = GPIO_PULLUP_ENABLE,
 		.scl_pullup_en = GPIO_PULLUP_ENABLE,
-		.master.clk_speed = 400000, // 4kHz; at least 385kHz
+		.master.clk_speed = 400000, // 4kHz; has to be at least 3.85kHz
 	};
 	i2c_param_config(I2C_NUM_0, &conf);
 	i2c_driver_install(I2C_NUM_0, I2C_MODE_MASTER, 0, 0, 0);
 	i2c_set_start_timing(I2C_NUM_0, 10, 10);
 }
 
-static void i2c_send(const unsigned char* data, size_t size) {
+/*
+ * I2C commands
+ */
+
+static void send_primitive(const unsigned char* data, size_t size, bool blankout) {
 	i2c_cmd_handle_t i2c_handle = i2c_cmd_link_create();
 	ESP_ERROR_CHECK(i2c_master_start(i2c_handle));
+
+	// Slave address
 	ESP_ERROR_CHECK(i2c_master_write_byte(i2c_handle, 0b01111000, true));
+	// Turn display off
+	if (blankout) {
+		ESP_ERROR_CHECK(i2c_master_write_byte(i2c_handle, 0xAE, true));
+	}
+	// Control/data bytes
 	ESP_ERROR_CHECK(i2c_master_write(i2c_handle, data, size, true));
+	// Turn display on
+	if (blankout) {
+		ESP_ERROR_CHECK(i2c_master_write_byte(i2c_handle, 0xAF, true));
+	}
+
+	// "The write mode will be finished when a stop condition is applied"
 	ESP_ERROR_CHECK(i2c_master_stop(i2c_handle));
 	ESP_ERROR_CHECK(i2c_master_cmd_begin(I2C_NUM_0, i2c_handle, 1000));
 	i2c_cmd_link_delete(i2c_handle);
 }
 
+static void send(const unsigned char* data, size_t size) {
+	send_primitive(data, size, true);
+}
+
+/*
+ * Public interface
+ */
+
 static const unsigned char startup[] = { 0x00, 0x8D, 0x14, 0xAF };
 
 void display_init() {
 	i2c_init();
-	i2c_send(startup, 4);
-	ESP_LOGI(TAG, "Initialized display successfully.");
+	send_primitive(startup, 4, false);
+	ESP_LOGI(TAG, "Initialized display");
 }
 
 void display_invertion(enum Inversion inv_byte) {
-	const unsigned char data[] = { 0xAE, inv_byte, 0xAF };
-	i2c_send(&data, 3);
+	send((const unsigned char*)&inv_byte, 1);
 }
diff --git a/Year 3/Internet of things/goam/main/display.h b/Year 3/Internet of things/goam/main/display.h
index 8053ff7..1bc9ea5 100644
--- a/Year 3/Internet of things/goam/main/display.h
+++ b/Year 3/Internet of things/goam/main/display.h
@@ -1,8 +1,9 @@
 #pragma once
 
 void display_init();
+
 enum Inversion {
-	NORMAL = 0xA6,
+	NORMAL  = 0xA6,
 	INVERSE = 0xA7
 };
 void display_invertion(enum Inversion inv_byte);
diff --git a/Year 3/Internet of things/goam/main/ptc06.c b/Year 3/Internet of things/goam/main/ptc06.c
index 717019d..273b0ba 100644
--- a/Year 3/Internet of things/goam/main/ptc06.c
+++ b/Year 3/Internet of things/goam/main/ptc06.c
@@ -34,7 +34,9 @@ static size_t arr4ToNum(const char arr[4]) {
 static const char* TAG = "ptc06";
 
 static void debug_print(const char* prepend, char* message, size_t size) {
-	printf("%s: ", prepend);
+	if (esp_log_level_get(TAG) < ESP_LOG_DEBUG) return;
+
+	printf("%s: %s ", TAG, prepend);
 	for (int i = 0; i < size; i++) {
 		printf("%X ", message[i]);
 	}
@@ -72,12 +74,17 @@ static void init_uart() {
 	// Set UART pins(TX: IO17 (UART2 default), RX: IO16 (UART2 default), RTS: IO18, CTS: IO19)
 	ESP_ERROR_CHECK(uart_set_pin(UART_NUM, UART_TX, UART_RX, UART_PIN_NO_CHANGE, UART_PIN_NO_CHANGE));
 
-	// Install UART driver using an event queue here
+	// Install UART driver using an event queue
 	ESP_ERROR_CHECK(uart_driver_install(UART_NUM, UART_BUFFER_SIZE, UART_BUFFER_SIZE, QUEUE_SIZE, &uart_queue, 0));
 
 	uart_flush(UART_NUM);
 }
 
+// On init, the UART may be full of data and the camera may be outputting data to it.
+// So the flush command isn't entirely effective, with this function we make sure it is clean,
+// even if we take a time penalty.
+//
+// It also seems that these wait periods are the perfect values, anything less and it has failed.
 static void dynamic_flush_uart() {
 	wait(500);
 	size_t length;
@@ -90,14 +97,6 @@ static void dynamic_flush_uart() {
 }
 
 /*
- * Camera special
- */
-
-static void init_camera() {
-	wait(2500);
-}
-
-/*
  * Generic UART communiction
  */
 
@@ -143,18 +142,23 @@ enum Command {
 };
 
 static void send(enum Command comm_byte, const char data_lengths, char data[16]) {
+	// Format:
+	// Protocol sign (1byte) + Serial number (1byte) + Command (1byte) + Data-lengths (1byte) + Data (0~16bytes)
 	char command[20] = { 0x56, serial_num, comm_byte, data_lengths, };
 	append(command + 4, data, data_lengths);
 
 	debug_print("sent", command, 4+data_lengths);
 
 	uart_write_bytes(UART_NUM, command, 4 + data_lengths);
-	ESP_ERROR_CHECK(uart_wait_tx_done(UART_NUM, 100));
+	ESP_ERROR_CHECK(uart_wait_tx_done(UART_NUM, 100)); // if it takes too long, then we have communication issues
 }
 
-#define byte_check(varname, compare) esp_err_t varname = (compare) ? ESP_OK : ESP_ERR_INVALID_RESPONSE; ESP_ERROR_CHECK(varname);
+#define byte_check(varname, compare) \
+	esp_err_t varname = (compare) ? ESP_OK : ESP_ERR_INVALID_RESPONSE; ESP_ERROR_CHECK(varname);
 
 static void recv(enum Command comm_byte, const char expectedDataSize, char ack[21]) {
+	// Format:
+	// Protocol sign (1byte) + Serial number (1byte) + Command (1byte) + Status (1byte) + Data-lengths (1byte) + Data (0~16bytes)
 	uart_read_bytes(UART_NUM, ack, 5 + expectedDataSize, 100);
 	ESP_ERROR_CHECK(uart_wait_tx_done(UART_NUM, 100));
 
@@ -164,11 +168,11 @@ static void recv(enum Command comm_byte, const char expectedDataSize, char ack[2
 	byte_check(BAD_SERIAL_NUM,     ack[1] == serial_num);
 	byte_check(BAD_RETURN_COMMAND, ack[2] == comm_byte);
 	byte_check(BAD_RETURN_STATUS,  ack[3] == 0);
-	// ack[4] is data-lengths
+	// ack[4] is data-lengths, no need to check
 }
 
 /*
- * Command implementation
+ * VC0706 command implementation
  */
 
 static char temp_ack[21], temp_data[16];
@@ -213,7 +217,6 @@ enum ControlMode {
 static size_t vc0706_read_fbuf(enum FBufType fbt_byte, enum ControlMode cm_byte, const char start[4], const char data_length[4], const char delay[2], char** image) {
 	size_t delayby = arr2ToNum(delay);
 	size_t length  = arr4ToNum(data_length);
-	/* printf("%d\n", length); */
 
 	*image = (char*)malloc(length);
 	memset(*image, 0xFA, length); // So it's easily recogniseable if the whole array wasn't filled with anything
@@ -235,8 +238,6 @@ static size_t vc0706_read_fbuf(enum FBufType fbt_byte, enum ControlMode cm_byte,
 		ESP_ERROR_CHECK(uart_get_buffered_data_len(UART_NUM, &available));
 		/* size_t old = current; */
 		current += uart_read_bytes(UART_NUM, *image + current, min(available, length - current), 500);
-		/* debug_print("image", *image + old, current - old); */
-		/* printf("%d %d\n", current, available); */
 		ESP_ERROR_CHECK(uart_wait_tx_done(UART_NUM, 500));
 		wait(20);
 	}
@@ -274,6 +275,14 @@ static void vc0706_write_data(enum DeviceType dt_byte, char write_data_len, cons
 }
 
 /*
+ * PTC06 camera module commands
+ */
+
+static void init_camera() {
+	wait(2500);
+}
+
+/*
  * Public interface
  */
 
@@ -302,7 +311,7 @@ size_t ptc06_get_image(char** image) {
 	size_t length = vc0706_read_fbuf(CURRENT_FRAME, DMA, start, temp_ack + 5, delay, image);
 
 	vc0706_fbuf_ctrl(RESUME_FRAME);
-	vc0706_system_reset();
+	vc0706_system_reset(); // Practice has shown this is mandatory
 	return length;
 }
 
diff --git a/Year 3/Internet of things/goam/main/spiffs.c b/Year 3/Internet of things/goam/main/spiffs.c
index 9fcd2ba..8e11722 100644
--- a/Year 3/Internet of things/goam/main/spiffs.c
+++ b/Year 3/Internet of things/goam/main/spiffs.c
@@ -1,65 +1,70 @@
-#include <stdio.h>
-#include <string.h>
 #include <sys/unistd.h>
 #include <sys/stat.h>
-#include "esp_err.h"
+#include <fcntl.h>
 #include "esp_log.h"
 #include "esp_spiffs.h"
-#include <fcntl.h>  // open, close, O_RDONLY, ...
+
+#include "spiffs.h"
 
 static const char *TAG = "example";
 
-void spiffs_init() {
-	ESP_LOGI(TAG, "Initializing SPIFFS");
+/*
+ * Public interface
+ */
 
-    esp_vfs_spiffs_conf_t conf = {
-      .base_path = SPIFFS_PREFIX,
-      .partition_label = NULL,
-      .max_files = 1,
-      .format_if_mount_failed = true
-    };
+void spiffs_init() {
+	esp_vfs_spiffs_conf_t conf = {
+		.base_path              = SPIFFS_PREFIX,
+		.partition_label        = NULL,
+		.max_files              = 1,
+		.format_if_mount_failed = true
+	};
 
-    // Use settings defined above to initialize and mount SPIFFS filesystem.
-    // Note: esp_vfs_spiffs_register is an all-in-one convenience function.
-    ESP_ERROR_CHECK(esp_vfs_spiffs_register(&conf));
+	// Use settings defined above to initialize and mount SPIFFS filesystem.
+	// Note: esp_vfs_spiffs_register is an all-in-one convenience function.
+	ESP_ERROR_CHECK(esp_vfs_spiffs_register(&conf));
 
-#ifdef CONFIG_EXAMPLE_SPIFFS_CHECK_ON_START
-    ESP_LOGI(TAG, "Performing SPIFFS_check().");
-    ESP_ERROR_CHECK(esp_spiffs_check(conf.partition_label));
+#ifdef CONFIG_SPIFFS_CHECK_ON_START
+	ESP_LOGI(TAG, "Performing SPIFFS_check().");
+	ESP_ERROR_CHECK(esp_spiffs_check(conf.partition_label));
 #endif
 
-    size_t total = 0, used = 0;
-    esp_err_t ret = esp_spiffs_info(conf.partition_label, &total, &used);
-    if (ret != ESP_OK) {
-        ESP_LOGE(TAG, "Failed to get SPIFFS partition information (%s). Formatting...", esp_err_to_name(ret));
-        esp_spiffs_format(conf.partition_label);
+	size_t total = 0, used = 0;
+	esp_err_t ret = esp_spiffs_info(conf.partition_label, &total, &used);
+	if (ret != ESP_OK) {
+		ESP_LOGW(TAG, "Failed to get SPIFFS partition information (%s). Formatting...", esp_err_to_name(ret));
+		esp_spiffs_format(conf.partition_label);
 		ESP_ERROR_CHECK(esp_spiffs_info(conf.partition_label, &total, &used));
-    }
+	}
 
 	ESP_LOGI(TAG, "Partition size: total: %d, used: %d", total, used);
 
-    // Check consistency of reported partiton size info.
-    if (used > total) {
-        ESP_LOGW(TAG, "Number of used bytes cannot be larger than total. Performing SPIFFS_check().");
-        ret = esp_spiffs_check(conf.partition_label);
-        // Could be also used to mend broken files, to clean unreferenced pages, etc.
-        // More info at https://github.com/pellepl/spiffs/wiki/FAQ#powerlosses-contd-when-should-i-run-spiffs_check
-        if (ret != ESP_OK) {
-            ESP_LOGE(TAG, "SPIFFS_check() failed (%s)", esp_err_to_name(ret));
-        } else {
-            ESP_LOGI(TAG, "SPIFFS_check() successful");
-        }
-    }
+	// Check consistency of reported partiton size info.
+	if (used > total) {
+		ESP_LOGW(TAG, "Number of used bytes cannot be larger than total. Performing SPIFFS_check().");
+		ret = esp_spiffs_check(conf.partition_label);
+		// Could be also used to mend broken files, to clean unreferenced pages, etc.
+		// More info at https://github.com/pellepl/spiffs/wiki/FAQ#powerlosses-contd-when-should-i-run-spiffs_check
+		if (ret != ESP_OK) {
+			ESP_LOGE(TAG, "SPIFFS_check() failed (%s)", esp_err_to_name(ret));
+		} else {
+			ESP_LOGI(TAG, "SPIFFS_check() successful");
+		}
+	}
 }
 
-void spiffs_write(const char* filename, const char* data, size_t data_len) {
+size_t spiffs_write(const char* filename, const char* data, size_t data_len) {
 	int outf = open(filename, O_WRONLY | O_CREAT | O_TRUNC);
 	if (outf < 0) {
 		ESP_LOGE(TAG, "Couldn't write to file \"%s\"!", filename);
-		return;
+		return 0;
 	}
-	write(outf, data, data_len);
+
+	size_t length = write(outf, data, data_len);
+
 	close(outf);
+	ESP_LOGD(TAG, "wrote %d bytes from \"%s\"", length, filename);
+	return length;
 }
 
 size_t spiffs_read(const char* filename, char** data) {
@@ -70,11 +75,17 @@ size_t spiffs_read(const char* filename, char** data) {
 	}
 
 	struct stat buf;
-	if (fstat(inf, &buf) < 0);
+	if (fstat(inf, &buf) < 0) {
+		close(inf);
+		ESP_LOGE(TAG, "Couldn't read stat of file \"%s\"!", filename);
+		return 0;
+	}
 
 	size_t length = buf.st_size;
 	*data = (char*)malloc(length);
 	read(inf, *data, length);
+
 	close(inf);
+	ESP_LOGD(TAG, "read %d bytes from \"%s\"", length, filename);
 	return length;
 }
diff --git a/Year 3/Internet of things/goam/main/spiffs.h b/Year 3/Internet of things/goam/main/spiffs.h
index 540310c..b88e24b 100644
--- a/Year 3/Internet of things/goam/main/spiffs.h
+++ b/Year 3/Internet of things/goam/main/spiffs.h
@@ -4,5 +4,5 @@
 #define SPIFFS_PREFIX_LEN (sizeof(SPIFFS_PREFIX)-1)
 
 void spiffs_init();
-void spiffs_write(const char* filename, const char* data, size_t data_len);
+size_t spiffs_write(const char* filename, const char* data, size_t data_len);
 size_t spiffs_read(const char* filename, char** data);
diff --git a/Year 3/Internet of things/goam/main/webserver.c b/Year 3/Internet of things/goam/main/webserver.c
index 3815b7f..8d5e18b 100644
--- a/Year 3/Internet of things/goam/main/webserver.c
+++ b/Year 3/Internet of things/goam/main/webserver.c
@@ -21,6 +21,10 @@ static const char *TAG = "webserver";
 
 static httpd_handle_t server = NULL;
 
+/*
+ * Request parsers
+ */
+
 static bool request_get_header(httpd_req_t *req, const char* header_name, char* buf, size_t* buf_len) {
 	/* char* buf; */
 	/* size_t buf_len; */
@@ -63,6 +67,22 @@ static bool request_get_query(httpd_req_t *req, const char* query_name, char* bu
 	}
 }
 
+/*
+ * Page handlers
+ */
+
+static void logi_get(httpd_req_t *req) {
+	/* After sending the HTTP response the old HTTP request
+	 * headers are lost. Check if HTTP request headers can be read now. */
+	if (httpd_req_get_hdr_value_len(req, "Host") == 0) {
+		ESP_LOGI(TAG, "Served %s", req->uri);
+	}
+	else {
+		ESP_LOGE(TAG, "Couldn't serve %s", req->uri);
+	}
+}
+
+// Index page is embedded in binary
 static char*  index_page = NULL;
 static size_t index_page_len = 0;
 
@@ -70,15 +90,11 @@ static esp_err_t get_handler(httpd_req_t *req)
 {
 	httpd_resp_send(req, index_page, index_page_len);
 
-	/* After sending the HTTP response the old HTTP request
-	 * headers are lost. Check if HTTP request headers can be read now. */
-	if (httpd_req_get_hdr_value_len(req, "Host") == 0) {
-		ESP_LOGI(TAG, "Request headers lost");
-	}
+	logi_get(req);
 	return ESP_OK;
 }
-static const httpd_uri_t get_page = {
-	.uri       = "/*",
+static const httpd_uri_t get_index = {
+	.uri       = "/",
 	.method    = HTTP_GET,
 	.handler   = get_handler,
 	.user_ctx  = NULL
@@ -94,11 +110,7 @@ static esp_err_t get_image_handler(httpd_req_t *req)
 	httpd_resp_send(req, resp_str, length);
 	free(resp_str);
 
-	/* After sending the HTTP response the old HTTP request
-	 * headers are lost. Check if HTTP request headers can be read now. */
-	if (httpd_req_get_hdr_value_len(req, "Host") == 0) {
-		ESP_LOGI(TAG, "Request headers lost");
-	}
+	logi_get(req);
 	return ESP_OK;
 }
 static const httpd_uri_t get_image = {
@@ -108,6 +120,14 @@ static const httpd_uri_t get_image = {
 	.user_ctx  = NULL
 };
 
+/*
+ * On network connection/disconnect
+ */
+
+static void connect_handler(void* arg, esp_event_base_t event_base, int32_t event_id, void* event_data) {
+	webserver_start();
+}
+
 
 /*
 static void disconnect_handler(void* arg, esp_event_base_t event_base,
@@ -125,11 +145,9 @@ static void disconnect_handler(void* arg, esp_event_base_t event_base,
 }
 */
 
-static void connect_handler(void* arg, esp_event_base_t event_base, int32_t event_id, void* event_data) {
-	if (server == NULL) {
-		webserver_start();
-	}
-}
+/*
+ * Public interface
+ */
 
 #define SET_EMBEDDED_PTR(var, var_size, conv_filename) \
 	extern const unsigned char var ## _start[] asm("_binary_" conv_filename "_start"); \
@@ -145,21 +163,23 @@ void webserver_init() {
 
 void webserver_start()
 {
-	server = NULL;
+	if (server != NULL) {
+		ESP_LOGE(TAG, "Tried starting the server while it was already started");
+		return;
+	}
 
 	httpd_config_t config = HTTPD_DEFAULT_CONFIG();
 	config.lru_purge_enable = true;
 	config.uri_match_fn = httpd_uri_match_wildcard;
 
-	ESP_LOGI(TAG, "Starting server on port: '%d'", config.server_port);
-
 	if (httpd_start(&server, &config) != ESP_OK) {
-		ESP_LOGI(TAG, "Error starting server!");
+		ESP_LOGE(TAG, "Couldn't start server");
+		server = NULL;
 		return;
 	}
 
-	ESP_LOGI(TAG, "Registering URI handlers");
-
+	httpd_register_uri_handler(server, &get_index);
 	httpd_register_uri_handler(server, &get_image);
-	httpd_register_uri_handler(server, &get_page);
+
+	ESP_LOGI(TAG, "Started server on port %d", config.server_port);
 }
diff --git a/Year 3/Internet of things/goam/main/wifi.c b/Year 3/Internet of things/goam/main/wifi.c
index 5c76096..3a049e5 100644
--- a/Year 3/Internet of things/goam/main/wifi.c
+++ b/Year 3/Internet of things/goam/main/wifi.c
@@ -1,46 +1,23 @@
-#include "wifi.h"
-#include <string.h>
 #include "freertos/FreeRTOS.h"
 #include "freertos/task.h"
 #include "freertos/event_groups.h"
+#include "nvs_flash.h"
 #include "esp_system.h"
 #include "esp_wifi.h"
 #include "esp_event.h"
 #include "esp_log.h"
-#include "freertos/task.h"
-#include "nvs_flash.h"
 
-#define EXAMPLE_ESP_MAXIMUM_RETRY  3
+#include "wifi.h"
 
 #define ESP_WIFI_SCAN_AUTH_MODE_THRESHOLD WIFI_AUTH_WPA2_PSK
 
-static const char *TAG = "wifi_station";
+#define ESP_MAXIMUM_RETRY  3
 
-static int s_retry_num = 0;
+static const char *TAG = "wifi_station";
 
-static void event_handler(void* arg, esp_event_base_t event_base,
-								int32_t event_id, void* event_data)
-{
-	if (event_base == WIFI_EVENT && event_id == WIFI_EVENT_STA_START) {
-		esp_wifi_connect();
-	} else if (event_base == WIFI_EVENT && event_id == WIFI_EVENT_STA_DISCONNECTED) {
-		if (s_retry_num < EXAMPLE_ESP_MAXIMUM_RETRY) {
-			esp_wifi_connect();
-			s_retry_num++;
-			ESP_LOGI(TAG, "retry to connect to the AP");
-		} else {
-			ESP_LOGI(TAG, "Failed to connect to SSID:%s, password:%s",
-					 EXAMPLE_ESP_WIFI_SSID, EXAMPLE_ESP_WIFI_PASS);
-		}
-		ESP_LOGI(TAG,"connect to the AP fail");
-	} else if (event_base == IP_EVENT && event_id == IP_EVENT_STA_GOT_IP) {
-		ip_event_got_ip_t* event = (ip_event_got_ip_t*) event_data;
-		ESP_LOGI(TAG, "got ip:" IPSTR, IP2STR(&event->ip_info.ip));
-		s_retry_num = 0;
-		ESP_LOGI(TAG, "connected to ap SSID:%s password:%s",
-				 EXAMPLE_ESP_WIFI_SSID, EXAMPLE_ESP_WIFI_PASS);
-	}
-}
+/*
+ * NVS
+ */
 
 static void nvs_init() {
 	esp_err_t ret = nvs_flash_init();
@@ -51,14 +28,55 @@ static void nvs_init() {
 	}
 
 	ESP_ERROR_CHECK(ret);
+	ESP_LOGI(TAG, "NVS initialized");
 }
 
+/*
+ * NetIF
+ */
+
+// HAS to be ran after nvs_init
 static void netif_init() {
 	ESP_ERROR_CHECK(esp_netif_init());
 
 	esp_netif_create_default_wifi_sta();
+	ESP_LOGI(TAG, "NETIF initialized");
+}
+
+/*
+ * WiFi events
+ */
+
+static int connect_tries = 0;
+
+static void event_handler(void* arg, esp_event_base_t event_base, int32_t event_id, void* event_data)
+{
+	if (event_base == WIFI_EVENT && event_id == WIFI_EVENT_STA_START) {
+		esp_wifi_connect();
+		return;
+	}
+
+	if (event_base == WIFI_EVENT && event_id == WIFI_EVENT_STA_DISCONNECTED) {
+		if (connect_tries < ESP_MAXIMUM_RETRY) {
+			esp_wifi_connect();
+			connect_tries++;
+			ESP_LOGW(TAG, "Retrying to connect... Try %d/%d", connect_tries, ESP_MAXIMUM_RETRY);
+		}
+		else {
+			ESP_LOGE(TAG, "Failed to connect to SSID:%s, password:%s", ESP_WIFI_SSID, ESP_WIFI_PASS);
+		}
+		return;
+	}
+
+	if (event_base == IP_EVENT && event_id == IP_EVENT_STA_GOT_IP) {
+		connect_tries = 0;
+
+		ip_event_got_ip_t* event = (ip_event_got_ip_t*) event_data;
+		ESP_LOGI(TAG, "Connected to SSID:%s password:%s, got ip:" IPSTR, ESP_WIFI_SSID, ESP_WIFI_PASS, IP2STR(&event->ip_info.ip));
+	}
 }
 
+
 static esp_event_handler_instance_t instance_any_id;
 static esp_event_handler_instance_t instance_got_ip;
 
@@ -75,13 +93,17 @@ static void wifi_init_events() {
 														&instance_got_ip));
 }
 
+/*
+ * Config
+ */
+
 static void wifi_init_static_station() {
 	ESP_ERROR_CHECK(esp_wifi_set_mode(WIFI_MODE_STA));
 
 	wifi_config_t wifi_config = {
 		.sta = {
-			.ssid     = EXAMPLE_ESP_WIFI_SSID,
-			.password = EXAMPLE_ESP_WIFI_PASS,
+			.ssid     = ESP_WIFI_SSID,
+			.password = ESP_WIFI_PASS,
 			/* Authmode threshold resets to WPA2 as default if password matches WPA2 standards (pasword len => 8).
 			 * If you want to connect the device to deprecated WEP/WPA networks, Please set the threshold value
 			 * to WIFI_AUTH_WEP/WIFI_AUTH_WPA_PSK and set the password with length and format matching to
@@ -93,6 +115,10 @@ static void wifi_init_static_station() {
 	ESP_ERROR_CHECK(esp_wifi_set_config(WIFI_IF_STA, &wifi_config));
 }
 
+/*
+ * Public interface
+ */
+
 void wifi_init() {
 	nvs_init();