Fixx style problems and design errors in main.c

stm-firmware/include/reflow-controller/safety-adc.h

@@ -39,6 +39,12 @@ enum safety_adc_check_result {
 				SAFETY_ADC_INTERNAL_ERROR = (1U<<4),
 				};
 
+extern enum safety_adc_check_result global_safety_adc_status;
+
+enum safety_adc_check_result safety_adc_get_errors();
+
+void safety_adc_clear_errors(void);
+
 void safety_adc_init();
 
 void safety_adc_deinit();

stm-firmware/main.c

@@ -36,18 +36,18 @@
 #include <reflow-controller/shell.h>
 #include <reflow-controller/digio.h>
 #include "fatfs/shimatta_sdio_driver/shimatta_sdio.h"
-#include <reflow-controller/temp-converter.h>
 #include <stm-periph/stm32-gpio-macros.h>
 #include <stm-periph/clock-enable-manager.h>
 #include <stm-periph/uart.h>
 #include <reflow-controller/shell-uart-config.h>
-#include <helper-macros/helper-macros.h>
 #include <reflow-controller/oven-driver.h>
 #include <reflow-controller/safety-adc.h>
 #include <fatfs/ff.h>
 #include <reflow-controller/reflow-menu.h>
 
-static void setup_nvic_priorities()
+bool global_error_state;
+
+static void setup_nvic_priorities(void)
 {
 	/* No sub priorities */
 	NVIC_SetPriorityGrouping(2);
@@ -61,7 +61,7 @@ static void setup_nvic_priorities()
 FATFS fs;
 FATFS * const fs_ptr = &fs;
 
-static inline void uart_gpio_config()
+static inline void uart_gpio_config(void)
 {
 	/*
 	 * In case the application is build in debug mode, use the TX/RX Pins on the debug header
@@ -121,30 +121,16 @@ static bool mount_sd_card_if_avail(bool mounted)
 
 	if (!sdio_check_inserted() && !mounted) {
 		res = f_mount(fs_ptr, "0:/", 1);
-		if (res == FR_OK) {
+		if (res == FR_OK)
 			return true;
-		} else {
+		else
 			return false;
-		}
 	}
 
 	return mounted;
 }
 
-static inline int32_t handle_pid_controller(struct pid_controller *pid, float target_temperature,
+static void setup_unused_pins(void)
-					    float current_temperature)
-{
-	int32_t pid_out;
-
-	pid_out = (int32_t)pid_sample(pid, target_temperature - current_temperature);
-
-	/* Blink green LED */
-	led_set(1, !led_get(1));
-
-	return pid_out;
-}
-
-static void setup_unused_pins()
 {
 	int i;
 
@@ -154,7 +140,7 @@ static void setup_unused_pins()
 		GPIOE->PUPDR |= PULLDOWN(i);
 }
 
-static inline void setup_system()
+static inline void setup_system(void)
 {
 	setup_nvic_priorities();
 	systick_setup();
@@ -185,10 +171,11 @@ static void handle_shell_uart_input(shellmatta_handle_t shell_handle)
 		shell_handle_input(shell_handle, uart_input, uart_input_len);
 }
 
-extern char _sccmram;
-extern char _eccmram;
 static void zero_ccm_ram(void)
 {
+	/* These extern variables are placed in the linker script */
+	extern char _sccmram;
+	extern char _eccmram;
 	uint32_t len;
 	uint32_t i;
 	uint32_t *ptr = (uint32_t *)&_sccmram;
@@ -198,10 +185,31 @@ static void zero_ccm_ram(void)
 		ptr[i] = 0UL;
 }
 
-volatile bool error_state = false;
+/**
-volatile enum safety_adc_check_result safety_adc_status = SAFETY_ADC_CHECK_OK;
+ * @brief This function sets the appropriate error flags in the oven PID controller
+ * depending on the Safety ADC measurements.
+ * The PID controller's error flags have to be cleared via the GUI by either starting a new RUN or explicitly
+ * ack'ing these errors.
+ */
+static void propagate_safety_adc_error_to_oven_pid(void)
+{
+	enum safety_adc_check_result safety_adc_result;
 
-int main()
+	safety_adc_result = safety_adc_get_errors();
+
+	if (safety_adc_result & SAFETY_ADC_CHECK_TEMP_LOW ||
+	    safety_adc_result & SAFETY_ADC_CHECK_TEMP_HIGH)
+		oven_pid_report_error(OVEN_PID_ERR_OVERTEMP);
+
+	if (safety_adc_result & SAFETY_ADC_CHECK_VREF_LOW ||
+	    safety_adc_result & SAFETY_ADC_CHECK_VREF_HIGH)
+		oven_pid_report_error(OVEN_PID_ERR_VREF_TOL);
+
+	if (safety_adc_result & SAFETY_ADC_INTERNAL_ERROR)
+		oven_pid_report_error(0);
+}
+
+int main(void)
 {
 	bool sd_card_mounted = false;
 	shellmatta_handle_t shell_handle;
@@ -213,6 +221,8 @@ int main()
 	zero_ccm_ram();
 	setup_system();
 
+	global_error_state = false;
+
 	shell_handle = shell_init(write_shell_callback);
 	shell_print_motd(shell_handle);
 
@@ -221,42 +231,26 @@ int main()
 
 		pid_status = oven_pid_get_status();
 
-		if(systick_ticks_have_passed(quarter_sec_timestamp, 250)) {
+		if (systick_ticks_have_passed(quarter_sec_timestamp, 250)) {
-			safety_adc_status = handle_safety_adc();
 			quarter_sec_timestamp = systick_get_global_tick();
 
-			if (safety_adc_status & SAFETY_ADC_CHECK_TEMP_LOW ||
+			(void)handle_safety_adc();
-					safety_adc_status & SAFETY_ADC_CHECK_TEMP_HIGH) {
+			propagate_safety_adc_error_to_oven_pid();
-				oven_pid_report_error(OVEN_PID_ERR_OVERTEMP);
-			}
 
-			if (safety_adc_status & SAFETY_ADC_CHECK_VREF_LOW ||
+			if (global_error_state)
-					safety_adc_status & SAFETY_ADC_CHECK_VREF_HIGH) {
-				oven_pid_report_error(OVEN_PID_ERR_VREF_TOL);
-			}
-
-			if (safety_adc_status & SAFETY_ADC_INTERNAL_ERROR) {
-				oven_pid_report_error(0);
-			}
-
-			if (error_state) {
 				led_set(0, !led_get(0));
-			} else {
+			else
 				led_set(0, 0);
-			}
-
 		}
 
 		pt1000_status = adc_pt1000_check_error();
-		error_state = pid_status->aborted || !!safety_adc_status || !!pt1000_status;
+		global_error_state = pid_status->aborted || !!safety_adc_get_errors() || !!pt1000_status;
 
 		menu_wait_request = reflow_menu_handle();
 
 		/* Deactivate oven output in case of error! */
-		if (!pid_status->active || pid_status->aborted || error_state) {
+		if (!pid_status->active || pid_status->aborted || global_error_state)
-			oven_pid_stop();
 			oven_driver_set_power(0U);
-		}
 
 		handle_shell_uart_input(shell_handle);
 
@@ -265,6 +259,7 @@ int main()
 
 	}
 
+	return 0;
 }
 
 void sdio_wait_ms(uint32_t ms)
@@ -272,12 +267,12 @@ void sdio_wait_ms(uint32_t ms)
 	systick_wait_ms(ms);
 }
 
-void DMA2_Stream7_IRQHandler()
+void DMA2_Stream7_IRQHandler(void)
 {
 	uint32_t hisr = DMA2->HISR;
+
 	DMA2->HIFCR = hisr;
 
-	if (hisr & DMA_HISR_TCIF7) {
+	if (hisr & DMA_HISR_TCIF7)
 		uart_tx_dma_complete_int_callback(&shell_uart);
-	}
 }

stm-firmware/safety-adc.c

@@ -23,10 +23,24 @@
 #include <helper-macros/helper-macros.h>
 #include <stm-periph/clock-enable-manager.h>
 
+enum safety_adc_check_result global_safety_adc_status;
+
+enum safety_adc_check_result safety_adc_get_errors()
+{
+	return global_safety_adc_status;
+}
+
+void safety_adc_clear_errors(void)
+{
+	global_safety_adc_status = SAFETY_ADC_CHECK_OK;
+}
+
 void safety_adc_init()
 {
 	rcc_manager_enable_clock(&RCC->APB2ENR, BITMASK_TO_BITNO(SAFETY_ADC_ADC_RCC_MASK));
 
+	safety_adc_clear_errors();
+
 	/* Enable temperature and VREFINT measurement */
 	ADC->CCR |= ADC_CCR_TSVREFE;
 
@@ -117,16 +131,17 @@ void safety_adc_trigger_meas(enum safety_adc_meas_channel measurement)
 	SAFETY_ADC_ADC_PERIPHERAL->CR2 |= ADC_CR2_SWSTART;
 }
 
-static volatile uint16_t safety_vref_meas_raw;
+static uint16_t safety_vref_meas_raw;
-static volatile bool safety_vref_valid = false;
+static bool safety_vref_valid = false;
-static volatile uint16_t safety_temp_meas_raw;
+static uint16_t safety_temp_meas_raw;
-static volatile bool safety_temp_valid = false;
+static bool safety_temp_valid = false;
 static float safety_vref;
 static float safety_temp;
 
 enum safety_adc_check_result handle_safety_adc()
 {
 	static enum safety_adc_meas_channel safety_meas_channel = SAFETY_ADC_MEAS_VREF;
+	enum safety_adc_check_result check_result;
 	uint16_t result;
 	int poll_status;
 
@@ -153,10 +168,13 @@ enum safety_adc_check_result handle_safety_adc()
 	}
 
 	if (safety_temp_valid && safety_vref_valid) {
-		return safety_adc_check_results(safety_vref_meas_raw, safety_temp_meas_raw, &safety_vref, &safety_temp);
+		check_result = safety_adc_check_results(safety_vref_meas_raw, safety_temp_meas_raw, &safety_vref, &safety_temp);
+		global_safety_adc_status |= check_result;
 	} else {
-		return SAFETY_ADC_CHECK_OK;
+		check_result = SAFETY_ADC_CHECK_OK;
 	}
+
+	return check_result;
 }
 
 float safety_adc_get_temp()