Add PWM Hysteresis (#3)

* Update CodeQL (#103)

* Fix codql finding: Multiplication result converted to larger type
* update workflow

* Add CONFigure:FANx:HYSteresis command
Allows user to change the hysteresis of a given fan to limit the
amount of INFO lines sent to LOG and/or SYSLOG.

* Fix to memory memory leak on failed MQTT connections.

* minor cleanup

* Changes CONFigure:FANx:HYSteresis to CONFigure:FANx:HYS_Tacho
Add CONFigure:FANx:HYS_Pwm and CONFigure:FANx:HYSTeresis?
so that hysteresis can be applied to both the tacho and pwm signals.

---------

Co-authored-by: Timo Kokkonen <tjko@iki.fi>

.github/workflows/codeql.yml

@@ -47,6 +47,11 @@ jobs:
     - name: Arm GNU Toolchain (arm-none-eabi-gcc)
       uses: carlosperate/arm-none-eabi-gcc-action@v1
 
+    - name: Checkout repository
+      uses: actions/checkout@v4
+      with:
+        submodules: recursive
+
     - name: Cache Pico-SDK
       id: cache-pico-sdk
       uses: actions/cache@v4
@@ -61,17 +66,12 @@ jobs:
         repository: raspberrypi/pico-sdk
         ref: ${{env.PICO_SDK_REF}}
         path: pico-sdk
-        submodules: true
+        submodules: recursive
 
     - name: Add PICO_SDK_PATH to Environment
       run: |
         echo "PICO_SDK_PATH=${{github.workspace}}/pico-sdk" >> $GITHUB_ENV
 
-    - name: Checkout repository
-      uses: actions/checkout@v4
-      with:
-        path: main
-        submodules: recursive
 
     # Initializes the CodeQL tools for scanning.
     - name: Initialize CodeQL
@@ -102,10 +102,10 @@ jobs:
     #     ./location_of_script_within_repo/buildscript.sh
 
     - name: Configure CMake
-      run: cmake -S main -B main/build -DCMAKE_BUILD_TYPE=${{env.BUILD_TYPE}} -DPICO_BOARD=pico_w -DFANPICO_BOARD=0804D
+      run: cmake -B build -DCMAKE_BUILD_TYPE=${{env.BUILD_TYPE}} -DPICO_BOARD=pico_w -DFANPICO_BOARD=0804D
 
     - name: Build
-      run: cmake --build main/build --config ${{env.BUILD_TYPE}}
+      run: cmake --build build --config ${{env.BUILD_TYPE}}
 
     - name: Perform CodeQL Analysis
       uses: github/codeql-action/analyze@v3

commands.md

@@ -32,6 +32,11 @@ Fanpico supports following commands:
 * [CONFigure:FANx:PWMMap?](#configurefanxpwmmap-1)
 * [CONFigure:FANx:FILTER](#configurefanxfilter)
 * [CONFigure:FANx:FILTER?](#configurefanxfilter-1)
+* [CONFigure:FANx:HYSTeresis?](#CONFigure:FANx:HYSTeresis)
+* [CONFigure:FANx:HYS_Tacho](#CONFigure:FANx:HYS_Tacho)
+* [CONFigure:FANx:HYS_Tacho?](#CONFigure:FANx:HYS_Tacho-1)
+* [CONFigure:FANx:HYS_Pwm](#CONFigure:FANx:HYS_Pwm)
+* [CONFigure:FANx:HYS_Pwm?](#CONFigure:FANx:HYS_Pwm-1)
 * [CONFigure:MBFANx:NAME](#configurembfanxname)
 * [CONFigure:MBFANx:NAME?](#configurembfanxname-1)
 * [CONFigure:MBFANx:MINrpm](#configurembfanxminrpm)
@@ -326,21 +331,47 @@ CONF:FAN1:NAME?
 CPU Fan 1
 ```
 
-#### CONFigure:FANx:HYSteresis
-Set the hysteresis threshold for a given fan (output) port.
+#### CONFigure:FANx:HYSTeresis?
+Returns the hysteresis thresholds for a given tacho fan (output) port.
 
 For example:
 ```
-CONF:FAN1:HYSteresis 2.0
+CONF:FAN8:HYST?
+CONF:FAN8:HYS_Pwm=1.000000      CONF:FAN8:HYS_Tacho=1.000000
 ```
 
-#### CONFigure:FANx:HYSteresis?
-Query the hysteresis threshold for a given fan (output) port.
+#### CONFigure:FANx:HYS_Tacho
+Set the hysteresis threshold for a given tacho fan (output) port.
 
 For example:
 ```
-CONF:FAN8:HYSteresis?
-CONF:FAN8:HYS=1.000000
+CONF:FAN1:HYSteresis:TACho 2.0
+```
+
+#### CONFigure:FANx:HYS_Tacho?
+Query the hysteresis threshold for a given tacho fan (output) port.
+
+For example:
+```
+CONF:FAN8:HYS_Tacho?
+CONF:FAN8:HYS_Tacho=1.000000
+```
+
+#### CONFigure:FANx:HYS_Pwm
+Set the hysteresis threshold for a given tacho PWM (output) port.
+
+For example:
+```
+CONF:FAN1:HYSteresis:PWM 2.0
+```
+
+#### CONFigure:FANx:HYS_Pwm?
+Query the hysteresis threshold for a given PWM fan (output) port.
+
+For example:
+```
+CONF:FAN8:HYS_Pwm?
+CONF:FAN8:HYS_Pwm=1.000000
 ```
 
 #### CONFigure:FANx:MINpwm

src/command.c

@@ -837,20 +837,47 @@ int cmd_fan_source(const char *cmd, const char *args, int query, char *prev_cmd)
 	return ret;
 }
 
-int cmd_fan_info_hys(const char *cmd, const char *args, int query, char *prev_cmd)
+
+int cmd_fan_tacho_hys(const char *cmd, const char *args, int query, char *prev_cmd)
+{
+	int fan;
+	float val;
+
+	fan = atoi(&prev_cmd[3]) - 1;
+	if (fan >= 0 && fan < FAN_COUNT) {
+		if (query) {
+			printf("CONF:FAN%d:HYS_Tacho=%f\n", fan+1, conf->fans[fan].tacho_hyst);
+		} else if (str_to_float(args, &val)) {
+			if (val >= 0.0) {
+				log_msg(LOG_NOTICE, "fan%d: change Hysteresis %f --> %f TAC",
+					fan + 1, conf->fans[fan].tacho_hyst, val);
+				conf->fans[fan].tacho_hyst = val;
+			} else {
+				log_msg(LOG_WARNING, "fan%d: invalid new value for Hysteresis: %f",
+					fan + 1, val);
+				return 2;
+			}
+		}
+		return 0;
+	}
+	return 1;
+}
+
+int cmd_fan_pwm_hys(const char *cmd, const char *args, int query, char *prev_cmd)
 {
 	int fan;
 	float val;
 
+	//printf("in cmd_fan_pwm_hys(%s,%s,%d,%s)\n", cmd, args, query, prev_cmd);
 	fan = atoi(&prev_cmd[3]) - 1;
 	if (fan >= 0 && fan < FAN_COUNT) {
 		if (query) {
-			printf("CONF:FAN%d:HYS=%f\n", fan+1, conf->fans[fan].info_hyst);
+			printf("CONF:FAN%d:HYS_Pwm=%f\n", fan+1, conf->fans[fan].pwm_hyst);
 		} else if (str_to_float(args, &val)) {
 			if (val >= 0.0) {
-				log_msg(LOG_NOTICE, "fan%d: change Hysteresis %f --> %f",
-					fan + 1, conf->fans[fan].info_hyst, val);
-				conf->fans[fan].info_hyst = val;
+				log_msg(LOG_NOTICE, "fan%d: change Hysteresis %f --> %f PWM",
+					fan + 1, conf->fans[fan].pwm_hyst, val);
+				conf->fans[fan].pwm_hyst = val;
 			} else {
 				log_msg(LOG_WARNING, "fan%d: invalid new value for Hysteresis: %f",
 					fan + 1, val);
@@ -862,6 +889,20 @@ int cmd_fan_info_hys(const char *cmd, const char *args, int query, char *prev_cm
 	return 1;
 }
 
+int cmd_hyst_supported(const char *cmd, const char *args, int query, char *prev_cmd)
+{
+	int fan;
+
+	//printf("in cmd_hyst_supported(%s,%s,%d,%s)\n", cmd, args, query, prev_cmd);
+	fan = atoi(&prev_cmd[3]) - 1;
+	if (query) {
+		printf("CONF:FAN%d:HYS_Pwm=%f\t", fan+1, conf->fans[fan].pwm_hyst);
+		printf("CONF:FAN%d:HYS_Tacho=%f\n", fan+1, conf->fans[fan].tacho_hyst);
+		return 0;
+	}
+	return 1;
+}
+
 int cmd_fan_rpm(const char *cmd, const char *args, int query, char *prev_cmd)
 {
 	int fan;
@@ -2707,6 +2748,12 @@ const struct cmd_t system_commands[] = {
 	{ 0, 0, 0, 0 }
 };
 
+const struct cmd_t fan_hyst_commands[] = {
+	{ "TACho",     3, NULL,              cmd_fan_tacho_hys },
+	{ "PWM",       3, NULL,              cmd_fan_pwm_hys   },
+	{ 0, 0, 0, 0 }
+};
+
 const struct cmd_t fan_c_commands[] = {
 	{ "FILTER",    6, NULL,              cmd_fan_filter },
 	{ "MAXpwm",    3, NULL,              cmd_fan_max_pwm },
@@ -2717,7 +2764,9 @@ const struct cmd_t fan_c_commands[] = {
 	{ "RPMFactor", 4, NULL,              cmd_fan_rpm_factor },
 	{ "RPMMOde",   5, NULL,              cmd_fan_rpm_mode },
 	{ "SOUrce",    3, NULL,              cmd_fan_source },
-	{ "HYSteresis",3, NULL,              cmd_fan_info_hys },
+	{ "HYSTeresis",4, NULL,              cmd_hyst_supported },
+	{ "HYS_Tacho", 5, NULL,              cmd_fan_tacho_hys  },
+	{ "HYS_Pwm",   5, NULL,              cmd_fan_pwm_hys    },
 	{ 0, 0, 0, 0 }
 };
 

src/config.c

@@ -489,7 +489,8 @@ void clear_config(struct fanpico_config *cfg)
 		f->rpm_factor = 2;
 		f->filter = FILTER_NONE;
 		f->filter_ctx = NULL;
-		f->info_hyst = FAN_INFO_HYSTERESIS;
+		f->tacho_hyst = FAN_TACHO_HYSTERESIS;
+		f->pwm_hyst = FAN_PWM_HYSTERESIS;
 	}
 
 	for (i = 0; i < MBFAN_MAX_COUNT; i++) {
@@ -745,7 +746,8 @@ cJSON *config_to_json(const struct fanpico_config *cfg)
 		cJSON_AddItemToObject(o, "rpm_factor", cJSON_CreateNumber(f->rpm_factor));
 		cJSON_AddItemToObject(o, "lra_low", cJSON_CreateNumber(f->lra_low));
 		cJSON_AddItemToObject(o, "lra_high", cJSON_CreateNumber(f->lra_high));
-		cJSON_AddItemToObject(o, "hysteresis", cJSON_CreateNumber(f->info_hyst));
+		cJSON_AddItemToObject(o, "tach_hyst", cJSON_CreateNumber(f->tacho_hyst));
+		cJSON_AddItemToObject(o, "pwm_hyst", cJSON_CreateNumber(f->pwm_hyst));
 		cJSON_AddItemToArray(fans, o);
 	}
 	cJSON_AddItemToObject(config, "fans", fans);
@@ -1094,8 +1096,10 @@ int json_to_config(cJSON *config, struct fanpico_config *cfg)
 				f->lra_high = cJSON_GetNumberValue(r);
 			if ((r = cJSON_GetObjectItem(item, "filter")))
 				json2filter(r, &f->filter, &f->filter_ctx);
-			if ((r = cJSON_GetObjectItem(item, "hysteresis")))
-				f->info_hyst = cJSON_GetNumberValue(r);
+			if ((r = cJSON_GetObjectItem(item, "tach_hyst")))
+				f->tacho_hyst = cJSON_GetNumberValue(r);
+			if ((r = cJSON_GetObjectItem(item, "pwm_hyst")))
+				f->pwm_hyst = cJSON_GetNumberValue(r);
 		}
 	}
 

src/fanpico.c

@@ -275,8 +275,9 @@ void update_outputs(struct fanpico_state *state, const struct fanpico_config *co
 
 	/* Update fan PWM signals */
 	for (i = 0; i < FAN_COUNT; i++) {
+		float hyst = cfg->fans[i].pwm_hyst;
 		state->fan_duty[i] = calculate_pwm_duty(state, config, i);
-		if (check_for_change(state->fan_duty_prev[i], state->fan_duty[i], 1.0)) {
+		if (check_for_change(state->fan_duty_prev[i], state->fan_duty[i], hyst)) {
 			log_msg(LOG_INFO, "fan%d: Set output PWM %.1f%% --> %.1f%%",
 				i+1,
 				state->fan_duty_prev[i],

src/fanpico.h

@@ -48,7 +48,8 @@
 #define SENSOR_SERIES_RESISTANCE 10000.0
 
 #define ADC_REF_VOLTAGE 3.0
-#define FAN_INFO_HYSTERESIS 1.0
+#define FAN_TACHO_HYSTERESIS 1.0
+#define FAN_PWM_HYSTERESIS 1.0
 #define ADC_MAX_VALUE   (1 << 12)
 #define ADC_AVG_WINDOW  10
 
@@ -139,7 +140,8 @@ struct temp_map {
 
 struct fan_output {
 	char name[MAX_NAME_LEN];
-	float info_hyst;
+	float tacho_hyst;
+	float pwm_hyst;
 
 	/* output PWM signal settings */
 	uint8_t min_pwm;

src/lwipopts.h

@@ -124,7 +124,7 @@ void pico_set_system_time(long int sec);
 #define DHCP_DEBUG                  LWIP_DBG_OFF
 #define SNTP_DEBUG                  LWIP_DBG_OFF
 #define HTTPD_DEBUG                 LWIP_DBG_OFF
-#define MQTT_DEBUG                  LWIP_DBG_OFF
+#define MQTT_DEBUG                  LWIP_DBG_ON
 #define ALTCP_MBEDTLS_DEBUG         LWIP_DBG_ON
 #define ALTCP_MBEDTLS_MEM_DEBUG     LWIP_DBG_ON
 #define ALTCP_MBEDTLS_LIB_DEBUG     LWIP_DBG_ON

src/mqtt.c

@@ -268,6 +268,9 @@ static void mqtt_dns_resolve_cb(const char *name, const ip_addr_t *ipaddr, void
 
 void mqtt_connect(mqtt_client_t *client)
 {
+#if TLS_SUPPORT
+	static struct altcp_tls_config *tlsconfig = NULL;
+#endif
 	struct mqtt_connect_client_info_t ci;
 	char client_id[32];
 	uint16_t port = MQTT_PORT;
@@ -304,11 +307,14 @@ void mqtt_connect(mqtt_client_t *client)
 	ci.will_qos = 0;
 #if TLS_SUPPORT
 	if (cfg->mqtt_tls) {
-		cyw43_arch_lwip_begin();
-		ci.tls_config = altcp_tls_create_config_client(NULL, 0);
-		cyw43_arch_lwip_end();
-		if (!ci.tls_config)
-			log_msg(LOG_WARNING, "altcp_tls_create_config_client(): failed");
+		if (!tlsconfig) {
+			cyw43_arch_lwip_begin();
+			tlsconfig = altcp_tls_create_config_client(NULL, 0);
+			cyw43_arch_lwip_end();
+			if (!tlsconfig)
+				log_msg(LOG_WARNING, "altcp_tls_create_config_client(): failed");
+		}
+		ci.tls_config = tlsconfig;
 		port = MQTT_TLS_PORT;
 	}
 #endif

src/sensors.c

@@ -60,7 +60,7 @@ double get_temperature(uint8_t input, const struct fanpico_config *config)
 		raw += adc_read();
 	}
 	raw /= ADC_AVG_WINDOW;
-	volt = raw * (config->adc_vref / ADC_MAX_VALUE);
+	volt = raw * ((double)config->adc_vref / ADC_MAX_VALUE);
 
 	if (sensor->type == TEMP_INTERNAL) {
 		t = 27.0 - ((volt - 0.706) / 0.001721);

src/tacho.c

@@ -244,7 +244,7 @@ void read_tacho_inputs()
 void update_tacho_input_freq(struct fanpico_state *st)
 {
 	for (int i = 0; i < FAN_COUNT; i++) {
-		float hyst = cfg->fans[i].info_hyst;
+		float hyst = cfg->fans[i].tacho_hyst;
 		st->fan_freq[i] = roundf(fan_tacho_freq[i]*100)/100.0;
 		if (check_for_change(st->fan_freq_prev[i], st->fan_freq[i], hyst)) {
 			log_msg(LOG_INFO, "fan%d: Input Tacho change %.2fHz --> %.2fHz",