fix(matter): suggested changes in pr review

Author: SuGlider

Diff for: libraries/Matter/examples/MatterThermostat/MatterThermostat.ino

@@ -43,7 +43,7 @@ const uint32_t decommissioningTimeout = 5000;  // keep the button pressed for 5s
 float getSimulatedTemperature(bool isHeating, bool isCooling) {
   // read sensor temperature and apply heating/cooling
   float simulatedTempHWSensor = SimulatedThermostat.getLocalTemperature();
-  
+
   if (isHeating) {
     // it will increase to simulate a heating system
     simulatedTempHWSensor = simulatedTempHWSensor + 0.5;
@@ -167,64 +167,54 @@ void loop() {
     SimulatedThermostat.setLocalTemperature(localTemperature); // publish the new temperature value
 
     // Simulate the thermostat control system - User has 4 modes: OFF, HEAT, COOL, AUTO
-    if (SimulatedThermostat.getMode() == MatterThermostat::THERMOSTAT_MODE_OFF) {
-      // turn off the heating and cooling systems
-      isHeating = false;
-      isCooling = false;
-    }
-    // User APP has set the thermostat to AUTO mode -- keeping the tempeature between both setpoints
-    if (SimulatedThermostat.getMode() == MatterThermostat::THERMOSTAT_MODE_AUTO) {
-      // check if the heating system should be turned on or off
-      if (localTemperature < SimulatedThermostat.getHeatingSetpoint() + SimulatedThermostat.getDeadBand()) {
-        // turn on the heating system and turn off the cooling system
-        isHeating = true;
-        isCooling = false;
-      }
-      if (localTemperature > SimulatedThermostat.getCoolingSetpoint() - SimulatedThermostat.getDeadBand()) {
-        // turn off the heating system and turn on the cooling system
-        isHeating = false;
-        isCooling = true;
-      }
-    }
-    // User APP has set the thermostat to AUTO mode -- keeping the tempeature between both setpoints
-    if (SimulatedThermostat.getMode() == MatterThermostat::THERMOSTAT_MODE_AUTO) {
-      // check if the heating system should be turned on or off
-      if (localTemperature < SimulatedThermostat.getHeatingSetpoint() + SimulatedThermostat.getDeadBand()) {
-        // turn on the heating system and turn off the cooling system
-        isHeating = true;
-        isCooling = false;
-      }
-      if (localTemperature > SimulatedThermostat.getCoolingSetpoint() - SimulatedThermostat.getDeadBand()) {
-        // turn off the heating system and turn on the cooling system
-        isHeating = false;
-        isCooling = true;
-      }
-    }
-    // Simulate the heating system - User has turned the heating system ON
-    if (SimulatedThermostat.getMode() == MatterThermostat::THERMOSTAT_MODE_HEAT) {
-      isHeating = true;
-      isCooling = false; // keep the cooling system off as it is in heating mode
-      // when the heating system is in HEATING mode, it will be turned off as soon as the local temperature is above the setpoint
-      if (localTemperature > SimulatedThermostat.getHeatingSetpoint()) {
-        // turn off the heating system
+    switch (SimulatedThermostat.getMode()) {
+      case MatterThermostat::THERMOSTAT_MODE_OFF:
+        // turn off the heating and cooling systems
         isHeating = false;
-      }
-    }
-    // Simulate the cooling system - User has turned the cooling system ON
-    if (SimulatedThermostat.getMode() == MatterThermostat::THERMOSTAT_MODE_COOL) {
-      isCooling = true;
-      isHeating = false; // keep the heating system off as it is in cooling mode
-      // when the cooling system is in COOLING mode, it will be turned off as soon as the local temperature is bellow the setpoint
-      if (localTemperature < SimulatedThermostat.getCoolingSetpoint()) {
-        // turn off the cooling system
         isCooling = false;
-      }
+        break;
+      case MatterThermostat::THERMOSTAT_MODE_AUTO:
+        // User APP has set the thermostat to AUTO mode -- keeping the tempeature between both setpoints
+        // check if the heating system should be turned on or off
+        if (localTemperature < SimulatedThermostat.getHeatingSetpoint() + SimulatedThermostat.getDeadBand()) {
+          // turn on the heating system and turn off the cooling system
+          isHeating = true;
+          isCooling = false;
+        }
+        if (localTemperature > SimulatedThermostat.getCoolingSetpoint() - SimulatedThermostat.getDeadBand()) {
+          // turn off the heating system and turn on the cooling system
+          isHeating = false;
+          isCooling = true;
+        }
+        break;
+      case MatterThermostat::THERMOSTAT_MODE_HEAT:
+        // Simulate the heating system - User has turned the heating system ON
+        isHeating = true;
+        isCooling = false; // keep the cooling system off as it is in heating mode
+        // when the heating system is in HEATING mode, it will be turned off as soon as the local temperature is above the setpoint
+        if (localTemperature > SimulatedThermostat.getHeatingSetpoint()) {
+          // turn off the heating system
+          isHeating = false;
+        }
+        break;
+      case MatterThermostat::THERMOSTAT_MODE_COOL:
+        // Simulate the cooling system - User has turned the cooling system ON
+        if (SimulatedThermostat.getMode() == MatterThermostat::THERMOSTAT_MODE_COOL) {
+          isCooling = true;
+          isHeating = false; // keep the heating system off as it is in cooling mode
+          // when the cooling system is in COOLING mode, it will be turned off as soon as the local temperature is bellow the setpoint
+          if (localTemperature < SimulatedThermostat.getCoolingSetpoint()) {
+            // turn off the cooling system
+            isCooling = false;
+          }
+        }
+        break;
+      default:
+        log_e("Invalid Thermostat Mode %d", SimulatedThermostat.getMode());
     }
-
     // Reporting Heating and Cooling status
     Serial.printf("\tThermostat Mode: %s >>> Heater is %s -- Cooler is %s\r\n", MatterThermostat::getThermostatModeString(SimulatedThermostat.getMode()), isHeating ? "ON" : "OFF", isCooling ? "ON" : "OFF");
   }
-
   // Check if the button has been pressed
   if (digitalRead(buttonPin) == LOW && !button_state) {
     // deals with button debouncing

Diff for: libraries/Matter/keywords.txt

@@ -36,7 +36,6 @@ EndPointSpeedCB	KEYWORD1
 EndPointOnOffCB	KEYWORD1
 EndPointBrightnessCB	KEYWORD1
 EndPointRGBColorCB	KEYWORD1
-EndPointTemperatureCB	KEYWORD1
 
 #######################################
 # Methods and Functions (KEYWORD2)

Diff for: libraries/Matter/src/MatterEndpoints/MatterThermostat.cpp

@@ -312,4 +312,57 @@ bool MatterThermostat::setRawTemperature(int16_t _rawTemperature, uint32_t attri
   return true;
 }
 
+  bool MatterThermostat::setCoolingHeatingSetpoints(double _setpointHeatingTemperature, double _setpointCollingTemperature) {
+    // at least one of the setpoints must be valid
+    bool settingCooling = _setpointCollingTemperature != (float)0xffff;
+    bool settingHeating = _setpointHeatingTemperature != (float)0xffff;
+    if (!settingCooling && !settingHeating) {
+      log_e("Invalid Setpoints values. Set correctly at leat one of them in Celsius.");
+      return false;
+    }
+    int16_t _rawHeatValue = static_cast<int16_t>(_setpointHeatingTemperature * 100.0f);
+    int16_t _rawCoolValue = static_cast<int16_t>(_setpointCollingTemperature * 100.0f);
+
+    // check limits for the setpoints
+    if (settingHeating && (_rawHeatValue < kDefaultMinHeatSetpointLimit || _rawHeatValue > kDefaultMaxHeatSetpointLimit)) {
+      log_e("Invalid Heating Setpoint value: %.01fC - valid range %d..%d", _setpointHeatingTemperature, kDefaultMinHeatSetpointLimit / 100, kDefaultMaxHeatSetpointLimit / 100);
+      return false;
+    }
+    if (settingCooling && (_rawCoolValue < kDefaultMinCoolSetpointLimit || _rawCoolValue > kDefaultMaxCoolSetpointLimit)) {
+      log_e("Invalid Cooling Setpoint value: %.01fC - valid range %d..%d", _setpointCollingTemperature, kDefaultMinCoolSetpointLimit / 100, kDefaultMaxCoolSetpointLimit / 100);
+      return false;
+    }
+
+    // AUTO mode requires both setpoints to be valid to each other and respect the deadband
+    if (currentMode ==  THERMOSTAT_MODE_AUTO) {
+#if ARDUHAL_LOG_LEVEL >= ARDUHAL_LOG_LEVEL_ERROR
+      float deadband = getDeadBand();
+#endif
+      // only setting Cooling Setpoint
+      if (settingCooling && !settingHeating && _rawCoolValue < (heatingSetpointTemperature + (kDefaultDeadBand * 10))) {
+        log_e("AutoMode :: Invalid Cooling Setpoint value: %.01fC - must be higher or equal than %.01fC", _setpointCollingTemperature, getHeatingSetpoint() + deadband);
+        return false;
+      }
+      // only setting Heating Setpoint
+      if (!settingCooling && settingHeating && _rawHeatValue > (coolingSetpointTemperature - (kDefaultDeadBand * 10))) {
+        log_e("AutoMode :: Invalid Heating Setpoint value: %.01fC - must be lower or equal than %.01fC", _setpointHeatingTemperature, getCoolingSetpoint() - deadband);
+        return false;
+      }
+      // setting both setpoints
+      if (settingCooling && settingHeating && (_rawCoolValue <= _rawHeatValue || _rawCoolValue - _rawHeatValue < kDefaultDeadBand * 10.0)) {
+        log_e("AutoMode :: Error - Heating Setpoint %.01fC must be lower than Cooling Setpoint %.01fC with a minimum difference of %0.1fC", _setpointHeatingTemperature, _setpointCollingTemperature, deadband);
+        return false;
+      }
+    }
+
+    bool ret = true;
+    if (settingCooling) {
+      ret &= setRawTemperature(_rawCoolValue, Thermostat::Attributes::OccupiedCoolingSetpoint::Id, &coolingSetpointTemperature);
+    }
+    if (settingHeating) {
+      ret &= setRawTemperature(_rawHeatValue, Thermostat::Attributes::OccupiedHeatingSetpoint::Id, &heatingSetpointTemperature);
+    }
+    return ret;
+  }
+
 #endif /* CONFIG_ESP_MATTER_ENABLE_DATA_MODEL */

Diff for: libraries/Matter/src/MatterEndpoints/MatterThermostat.h

@@ -97,58 +97,7 @@ class MatterThermostat : public MatterEndPoint {
   // Heating Setpoint must be lower than Cooling Setpoint
   // When using AUTO mode the Cooling Setpoint must be higher than Heating Setpoint by at least the 2.5C (deadband)
   // Thermostat Matter Server will enforce those rules and the Max/Min setpoints limits as in the Matter Specification 
-  bool setCoolingHeatingSetpoints(double _setpointHeatingTemperature, double _setpointCollingTemperature) {
-    // at least one of the setpoints must be valid
-    bool settingCooling = _setpointCollingTemperature != (float)0xffff;
-    bool settingHeating = _setpointHeatingTemperature != (float)0xffff;
-    if (!settingCooling && !settingHeating) {
-      log_e("Invalid Setpoints values. Set correctly at leat one of them in Celsius.");
-      return false;
-    }
-    int16_t _rawHeatValue = static_cast<int16_t>(_setpointHeatingTemperature * 100.0f);
-    int16_t _rawCoolValue = static_cast<int16_t>(_setpointCollingTemperature * 100.0f);
-
-    // check limits for the setpoints
-    if (settingHeating && (_rawHeatValue < kDefaultMinHeatSetpointLimit || _rawHeatValue > kDefaultMaxHeatSetpointLimit)) {
-      log_e("Invalid Heating Setpoint value: %.01fC - valid range %d..%d", _setpointHeatingTemperature, kDefaultMinHeatSetpointLimit / 100, kDefaultMaxHeatSetpointLimit / 100);
-      return false;
-    }
-    if (settingCooling && (_rawCoolValue < kDefaultMinCoolSetpointLimit || _rawCoolValue > kDefaultMaxCoolSetpointLimit)) {
-      log_e("Invalid Cooling Setpoint value: %.01fC - valid range %d..%d", _setpointCollingTemperature, kDefaultMinCoolSetpointLimit / 100, kDefaultMaxCoolSetpointLimit / 100);
-      return false;
-    }
-
-    // AUTO mode requires both setpoints to be valid to each other and respect the deadband
-    if (currentMode ==  THERMOSTAT_MODE_AUTO) {
-#if ARDUHAL_LOG_LEVEL >= ARDUHAL_LOG_LEVEL_ERROR
-      float deadband = getDeadBand();
-#endif
-      // only setting Cooling Setpoint
-      if (settingCooling && !settingHeating && _rawCoolValue < (heatingSetpointTemperature + (kDefaultDeadBand * 10))) {
-        log_e("AutoMode :: Invalid Cooling Setpoint value: %.01fC - must be higher or equal than %.01fC", _setpointCollingTemperature, getHeatingSetpoint() + deadband);
-        return false;
-      }
-      // only setting Heating Setpoint
-      if (!settingCooling && settingHeating && _rawHeatValue > (coolingSetpointTemperature - (kDefaultDeadBand * 10))) {
-        log_e("AutoMode :: Invalid Heating Setpoint value: %.01fC - must be lower or equal than %.01fC", _setpointHeatingTemperature, getCoolingSetpoint() - deadband);
-        return false;
-      }
-      // setting both setpoints
-      if (settingCooling && settingHeating && (_rawCoolValue <= _rawHeatValue || _rawCoolValue - _rawHeatValue < kDefaultDeadBand * 10.0)) {
-        log_e("AutoMode :: Error - Heating Setpoint %.01fC must be lower than Cooling Setpoint %.01fC with a minimum difference of %0.1fC", _setpointHeatingTemperature, _setpointCollingTemperature, deadband);
-        return false;
-      }
-    }
-
-    bool ret = true;
-    if (settingCooling) {
-      ret &= setRawTemperature(_rawCoolValue, Thermostat::Attributes::OccupiedCoolingSetpoint::Id, &coolingSetpointTemperature);
-    }
-    if (settingHeating) {
-      ret &= setRawTemperature(_rawHeatValue, Thermostat::Attributes::OccupiedHeatingSetpoint::Id, &heatingSetpointTemperature);
-    }
-    return ret;
-  }
+  bool setCoolingHeatingSetpoints(double _setpointHeatingTemperature, double _setpointCollingTemperature);
 
   // set the heating setpoint in 1/100th of a Celsio degree
   bool setHeatingSetpoint(double _setpointHeatingTemperature) {