Lint and testing fixes

esphome/components/mitsubishi_itp/binary_sensor/mitp_binary-sensor.h

@@ -8,7 +8,7 @@ namespace mitsubishi_itp {
 
 class MITPBinarySensor : public MITPListener, public binary_sensor::BinarySensor {
  public:
-  void publish(bool force = true) {
+  void publish() override {
     if (mitp_binary_sensor_state_.has_value())
       // Binary sensors automatically dedup publishes (I think) and so will only actually publish on change
       publish_state(mitp_binary_sensor_state_.value());

esphome/components/mitsubishi_itp/mitp_listener.h

@@ -9,12 +9,12 @@ static constexpr char LISTENER_TAG[] = "mitsubishi_itp.listener";
 
 class MITPListener : public PacketProcessor {
  public:
-  virtual void publish(bool force = true) = 0;
+  virtual void publish() = 0;  // Publish only if the underlying state has changed
 
   // TODO: These trhee are only used by the TemperatureSourceSelect, so might need to be broken out (putting them here
   // now to get things working)
   virtual void setup(bool thermostat_is_present){};  // Called during hub-component setup();
-  virtual void temperature_source_change(const std::string temp_source){};
+  virtual void temperature_source_change(const std::string &temp_source){};
 };
 
 }  // namespace mitsubishi_itp

esphome/components/mitsubishi_itp/mitsubishi_uart.cpp

@@ -114,7 +114,7 @@ void MitsubishiUART::update() {
 
   // Notify all listeners a publish is happening, they will decide if actual publish is needed.
   for (auto *listener : listeners_) {
-    listener->publish(false);
+    listener->publish();
   }
 
   if (publish_on_update_) {

esphome/components/mitsubishi_itp/select/mitp_select.cpp

@@ -0,0 +1,65 @@
+#include "mitp_select.h"
+
+namespace esphome {
+namespace mitsubishi_itp {
+
+void VanePositionSelect::process_packet(const SettingsGetResponsePacket &packet) {
+  switch (packet.get_vane()) {
+    case SettingsSetRequestPacket::VANE_AUTO:
+      mitp_select_value_ = std::string("Auto");
+      break;
+    case SettingsSetRequestPacket::VANE_1:
+      mitp_select_value_ = std::string("1");
+      break;
+    case SettingsSetRequestPacket::VANE_2:
+      mitp_select_value_ = std::string("2");
+      break;
+    case SettingsSetRequestPacket::VANE_3:
+      mitp_select_value_ = std::string("3");
+      break;
+    case SettingsSetRequestPacket::VANE_4:
+      mitp_select_value_ = std::string("4");
+      break;
+    case SettingsSetRequestPacket::VANE_5:
+      mitp_select_value_ = std::string("5");
+      break;
+    case SettingsSetRequestPacket::VANE_SWING:
+      mitp_select_value_ = std::string("Swing");
+      break;
+    default:
+      ESP_LOGW(TAG, "Vane in unknown position %x", packet.get_vane());
+  }
+}
+
+void HorizontalVanePositionSelect::process_packet(const SettingsGetResponsePacket &packet) {
+  switch (packet.get_horizontal_vane()) {
+    case SettingsSetRequestPacket::HV_AUTO:
+      mitp_select_value_ = std::string("Auto");
+      break;
+    case SettingsSetRequestPacket::HV_LEFT_FULL:
+      mitp_select_value_ = std::string("<<");
+      break;
+    case SettingsSetRequestPacket::HV_LEFT:
+      mitp_select_value_ = std::string("<");
+      break;
+    case SettingsSetRequestPacket::HV_CENTER:
+      mitp_select_value_ = std::string("");
+      break;
+    case SettingsSetRequestPacket::HV_RIGHT:
+      mitp_select_value_ = std::string(">");
+      break;
+    case SettingsSetRequestPacket::HV_RIGHT_FULL:
+      mitp_select_value_ = std::string(">>");
+      break;
+    case SettingsSetRequestPacket::HV_SPLIT:
+      mitp_select_value_ = std::string("<>");
+      break;
+    case SettingsSetRequestPacket::HV_SWING:
+      mitp_select_value_ = std::string("Swing");
+      break;
+    default:
+      ESP_LOGW(TAG, "Vane in unknown horizontal position %x", packet.get_horizontal_vane());
+  }
+}
+}  // namespace mitsubishi_itp
+}  // namespace esphome

esphome/components/mitsubishi_itp/select/mitp_select.h

@@ -10,9 +10,9 @@ class MITPSelect : public select::Select, public Parented<MitsubishiUART>, publi
  public:
   MITPSelect() = default;
   using Parented<MitsubishiUART>::Parented;
-  void publish(bool force = false) {
+  void publish() override {
     // Only publish if force, or a change has occurred and we have a real value
-    if (force || (mitp_select_value_.has_value() && mitp_select_value_.value() != state)) {
+    if (mitp_select_value_.has_value() && mitp_select_value_.value() != state) {
       publish_state(mitp_select_value_.value());
     }
   }
@@ -24,12 +24,12 @@ class MITPSelect : public select::Select, public Parented<MitsubishiUART>, publi
 
 class TemperatureSourceSelect : public MITPSelect {
  public:
-  void publish(bool force = false) override;
+  void publish() override;
   void setup(bool thermostat_is_present) override;
-  void temperature_source_change(const std::string temp_source) override;
+  void temperature_source_change(const std::string &temp_source) override;
 
   // Adds an option to temperature_source_select_
-  void register_temperature_source(std::string temperature_source_name);
+  void register_temperature_source(const std::string &temperature_source_name);
 
  protected:
   void control(const std::string &value) override;
@@ -41,36 +41,10 @@ class TemperatureSourceSelect : public MITPSelect {
 };
 
 class VanePositionSelect : public MITPSelect {
-  void process_packet(const SettingsGetResponsePacket &packet) {
-    switch (packet.get_vane()) {
-      case SettingsSetRequestPacket::VANE_AUTO:
-        mitp_select_value_ = std::string("Auto");
-        break;
-      case SettingsSetRequestPacket::VANE_1:
-        mitp_select_value_ = std::string("1");
-        break;
-      case SettingsSetRequestPacket::VANE_2:
-        mitp_select_value_ = std::string("2");
-        break;
-      case SettingsSetRequestPacket::VANE_3:
-        mitp_select_value_ = std::string("3");
-        break;
-      case SettingsSetRequestPacket::VANE_4:
-        mitp_select_value_ = std::string("4");
-        break;
-      case SettingsSetRequestPacket::VANE_5:
-        mitp_select_value_ = std::string("5");
-        break;
-      case SettingsSetRequestPacket::VANE_SWING:
-        mitp_select_value_ = std::string("Swing");
-        break;
-      default:
-        ESP_LOGW(TAG, "Vane in unknown position %x", packet.get_vane());
-    }
-  }
+  void process_packet(const SettingsGetResponsePacket &packet) override;
 
  protected:
-  void control(const std::string &value) {
+  void control(const std::string &value) override {
     if (parent_->select_vane_position(value)) {
       mitp_select_value_ = value;
       publish();
@@ -79,39 +53,10 @@ class VanePositionSelect : public MITPSelect {
 };
 
 class HorizontalVanePositionSelect : public MITPSelect {
-  void process_packet(const SettingsGetResponsePacket &packet) {
-    switch (packet.get_horizontal_vane()) {
-      case SettingsSetRequestPacket::HV_AUTO:
-        mitp_select_value_ = std::string("Auto");
-        break;
-      case SettingsSetRequestPacket::HV_LEFT_FULL:
-        mitp_select_value_ = std::string("<<");
-        break;
-      case SettingsSetRequestPacket::HV_LEFT:
-        mitp_select_value_ = std::string("<");
-        break;
-      case SettingsSetRequestPacket::HV_CENTER:
-        mitp_select_value_ = std::string("");
-        break;
-      case SettingsSetRequestPacket::HV_RIGHT:
-        mitp_select_value_ = std::string(">");
-        break;
-      case SettingsSetRequestPacket::HV_RIGHT_FULL:
-        mitp_select_value_ = std::string(">>");
-        break;
-      case SettingsSetRequestPacket::HV_SPLIT:
-        mitp_select_value_ = std::string("<>");
-        break;
-      case SettingsSetRequestPacket::HV_SWING:
-        mitp_select_value_ = std::string("Swing");
-        break;
-      default:
-        ESP_LOGW(TAG, "Vane in unknown horizontal position %x", packet.get_horizontal_vane());
-    }
-  }
+  void process_packet(const SettingsGetResponsePacket &packet) override;
 
  protected:
-  void control(const std::string &value) {
+  void control(const std::string &value) override {
     if (parent_->select_horizontal_vane_position(value)) {
       mitp_select_value_ = value;
       publish();

esphome/components/mitsubishi_itp/select/temperature-source_select.cpp

@@ -3,9 +3,9 @@
 namespace esphome {
 namespace mitsubishi_itp {
 
-void TemperatureSourceSelect::publish(bool force) {
+void TemperatureSourceSelect::publish() {
   // Only publish if force, or a change has occurred and we have a real value
-  if (force || (mitp_select_value_.has_value() && mitp_select_value_.value() != state)) {
+  if (mitp_select_value_.has_value() && mitp_select_value_.value() != state) {
     publish_state(mitp_select_value_.value());
     if (active_index().has_value()) {
       preferences_.save(&active_index().value());
@@ -33,11 +33,11 @@ void TemperatureSourceSelect::setup(bool thermostat_is_present) {
   }
 }
 
-void TemperatureSourceSelect::temperature_source_change(const std::string temp_source) {
+void TemperatureSourceSelect::temperature_source_change(const std::string &temp_source) {
   mitp_select_value_ = temp_source;
 }
 
-void TemperatureSourceSelect::register_temperature_source(std::string temperature_source_name) {
+void TemperatureSourceSelect::register_temperature_source(const std::string &temperature_source_name) {
   temp_select_options_.push_back(temperature_source_name);
 }
 

esphome/components/mitsubishi_itp/sensor/mitp_sensor.h

@@ -8,9 +8,9 @@ namespace mitsubishi_itp {
 
 class MITPSensor : public MITPListener, public sensor::Sensor {
  public:
-  void publish(bool force = true) {
+  void publish() override {
     // Only publish if force, or a change has occurred and we have a real value
-    if (force || (!isnan(mitp_sensor_state_) && mitp_sensor_state_ != state)) {
+    if (!isnan(mitp_sensor_state_) && mitp_sensor_state_ != state) {
       publish_state(mitp_sensor_state_);
     }
   }

esphome/components/mitsubishi_itp/text_sensor/mitp_text-sensor.cpp

@@ -0,0 +1,23 @@
+#include "mitp_text-sensor.h"
+
+namespace esphome {
+namespace mitsubishi_itp {
+
+void ErrorCodeSensor::process_packet(const ErrorStateGetResponsePacket &packet) {
+  // TODO: Include friendly text from JSON, somehow.
+  if (!packet.error_present()) {
+    mitp_text_sensor_state_ = std::string("No Error Reported");
+  } else if (auto raw_code = packet.get_raw_short_code() != 0x00) {
+    // Not that it matters, but good for validation I guess.
+    if ((raw_code & 0x1F) > 0x15) {
+      ESP_LOGW(LISTENER_TAG, "Error short code %x had invalid low bits. This is an IT protocol violation!", raw_code);
+    }
+
+    mitp_text_sensor_state_ = "Error " + packet.get_short_code();
+  } else {
+    mitp_text_sensor_state_ = "Error " + to_string(packet.get_error_code());
+  }
+}
+
+}  // namespace mitsubishi_itp
+}  // namespace esphome

esphome/components/mitsubishi_itp/text_sensor/mitp_text-sensor.h

@@ -8,9 +8,9 @@ namespace mitsubishi_itp {
 
 class MITPTextSensor : public MITPListener, public text_sensor::TextSensor {
  public:
-  void publish(bool force = true) {
+  void publish() {
     // Only publish if force, or a change has occurred and we have a real value
-    if (force || (mitp_text_sensor_state_.has_value() && mitp_text_sensor_state_.value() != state)) {
+    if (mitp_text_sensor_state_.has_value() && mitp_text_sensor_state_.value() != state) {
       publish_state(mitp_text_sensor_state_.value());
     }
   }
@@ -26,21 +26,7 @@ class ActualFanSensor : public MITPTextSensor {
 };
 
 class ErrorCodeSensor : public MITPTextSensor {
-  void process_packet(const ErrorStateGetResponsePacket &packet) {
-    // TODO: Include friendly text from JSON, somehow.
-    if (!packet.error_present()) {
-      mitp_text_sensor_state_ = std::string("No Error Reported");
-    } else if (auto raw_code = packet.get_raw_short_code() != 0x00) {
-      // Not that it matters, but good for validation I guess.
-      if ((raw_code & 0x1F) > 0x15) {
-        ESP_LOGW(LISTENER_TAG, "Error short code %x had invalid low bits. This is an IT protocol violation!", raw_code);
-      }
-
-      mitp_text_sensor_state_ = "Error " + packet.get_short_code();
-    } else {
-      mitp_text_sensor_state_ = "Error " + to_string(packet.get_error_code());
-    }
-  }
+  void process_packet(const ErrorStateGetResponsePacket &packet) override;
 };
 
 }  // namespace mitsubishi_itp

tests/components/mitsubishi_itp/common.yaml

@@ -9,6 +9,13 @@ time:
     id: homeassistant_time
     timezone: America/Los_Angeles
 
+select:
+  - platform: mitsubishi_itp
+    temperature_source:
+      name: "Temperature Source"
+      sources:
+        - fake_temp
+
 sensor:
   - platform: template
     id: fake_temp
@@ -38,5 +45,3 @@ climate:
     uart_thermostat: tstat_uart
     time_id: homeassistant_time
     update_interval: 12s
-    temperature_sources:
-      - fake_temp