Added sanity check for MODBUS holding registers

Also changed default led smoothing to 1

fw/Core/Inc/config.h

@@ -49,10 +49,10 @@ config_read(&config);
  * Device configuration data can be accessed using config_t struct.
  */
 #define CONFIG_DEFAULT_LED_ON					1
-#define CONFIG_DEFAULT_LED_BRIGHTNESS			100 /* TODO: set according to the timers when this will be implemented */
+#define CONFIG_DEFAULT_LED_BRIGHTNESS			100
 #define CONFIG_DEFAULT_LED_ALERT1_LIMIT			1500
 #define CONFIG_DEFAULT_LED_ALERT2_LIMIT			3000
-#define CONFIG_DEFAULT_LED_SMOOTH				0
+#define CONFIG_DEFAULT_LED_SMOOTH				1
 #define CONFIG_DEFAULT_SCD4x_T_OFFSET			0
 #define CONFIG_DEFAULT_BAUDRATE_INDEX			0
 

fw/Core/Src/main.c

@@ -355,7 +355,7 @@ int main(void)
 					  rgbled_set_color(RGBLED_RED);
 				  }
 			  } else {
-				  /* */
+				  /* interpolate color according to CO2 level */
 				  CO2_to_color(CO2, sensor_config.led_co2_alert_limit1, sensor_config.led_co2_alert_limit2);
 			  }
 		  } else {
@@ -849,34 +849,75 @@ int8_t modbus_slave_callback(modbus_transaction_t *transaction)
 				switch (register_number)
 				{
 					case REGISTER_NUM_LED_ON:
+						/* allowed values: 0 (off), 1 (on) */
+						if (transaction->holding_registers[i] > 1) {
+							return MODBUS_ERROR_OUT_OF_BOUNDS; // todo is this the right error?
+						}
 						sensor_config.led_on = (uint8_t) transaction->holding_registers[i];
 						break;
 					case REGISTER_NUM_LED_BRIGHTNESS:
+						/* allowed values: [0, 100] (from 0 to 100, where 0 is off and 100 is full power) */
+						if (transaction->holding_registers[i] > 100) {
+							return MODBUS_ERROR_OUT_OF_BOUNDS;
+						}
 						sensor_config.led_brightness = (uint16_t) transaction->holding_registers[i];
 						rgbled_set_intensity(sensor_config.led_brightness);
 						break;
 					case REGISTER_NUM_LED_SMOOTH:
+						/* allowed values: 0 (off), 1 (on) */
+						if (transaction->holding_registers[i] > 1) {
+							return MODBUS_ERROR_OUT_OF_BOUNDS;
+						}
 						sensor_config.led_smooth = (uint16_t) transaction->holding_registers[i];
 						break;
 					case REGISTER_NUM_CO2_ALERT_LIMIT1:
+						/* allowed values: [0, 65536] ppm (from 0 to uint16_t maximum value)
+						 * Note that alert limit 1 must ALWAYS be lower than alert limit 2 */
+						if (transaction->holding_registers[i] >= sensor_config.led_co2_alert_limit2) {
+							/* trying to set alert limit 1 higher than alert limit 2, return error */
+							return MODBUS_ERROR_OUT_OF_BOUNDS;
+						}
 						sensor_config.led_co2_alert_limit1 = (uint16_t) transaction->holding_registers[i];
 						break;
 					case REGISTER_NUM_CO2_ALERT_LIMIT2:
+						/* allowed values: [0, 65536] ppm (from 0 to uint16_t maximum value)
+						 * Note that alert limit 1 must ALWAYS be lower than alert limit 2 */
+						if (transaction->holding_registers[i] <= sensor_config.led_co2_alert_limit1) {
+							/* trying to set alert limit 1 higher than alert limit 2, return error */
+							return MODBUS_ERROR_OUT_OF_BOUNDS;
+						}
 						sensor_config.led_co2_alert_limit2 = (uint16_t) transaction->holding_registers[i];
 						break;
 					case REGISTER_NUM_SCD4x_T_OFFSET:
+						/* allowed values: [0, 65536]
+						 * Received value is interpreted as 16-bit signed (two's complement) number,
+						 * multiplied by 10. Examples:
+						 * +---------------+------------+----------------+
+						 * | Offset [deg C] | 16-bit hex | decimal signed |
+						 * +---------------+------------+----------------+
+						 * |     -1.5      |   0xfff1    |      -15       |
+						 * +---------------+------------+----------------+
+						 * |      0.9      |   0x0009   |        9       |
+						 * +---------------+------------+----------------+
+						 */
 						sensor_config.scd4x_t_offset = (int16_t) transaction->holding_registers[i];
 						break;
 					case REGISTER_NUM_MODBUS_ADDR:
+						/* allowed values: [1, 247] (from 1 to 247) as given in section 2.1 of document
+						 * MODBUS over Serial Line: Specification and Implementation Guide (V1.02) */
+						if (transaction->holding_registers[i] < 1 || transaction->holding_registers[i] > 247) {
+							return MODBUS_ERROR_OUT_OF_BOUNDS;
+						}
 						sensor_config.modbus_addr = (uint16_t) transaction->holding_registers[i];
 						modbus_address_changed = 1;
 						break;
 					case REGISTER_NUM_BAUDRATE:
-						if (transaction->holding_registers[0] < config_baudrates_length)
+						/* allowed values: [0, 9] (from 0 to 9, where 9 == config_baudrates_length) */
-						{
+						if (transaction->holding_registers[i] >= config_baudrates_length) {
+							return MODBUS_ERROR_OUT_OF_BOUNDS;
+						}
 						sensor_config.baudrate_index = (uint16_t) (transaction->holding_registers[i]);
 						baudrate_changed = 1;
-						}
 						break;
 					default:
 						return MODBUS_ERROR_FUNCTION_NOT_IMPLEMENTED;