From 2b0330cd861f29385acf0e2de487aab7634c55e1 Mon Sep 17 00:00:00 2001 From: Steve Wagner Date: Thu, 8 Feb 2024 16:37:03 +0100 Subject: [PATCH] Refactor to use one request object (#84) * test * Tests for ProtocolRequest * Min * use esphome optioanl and fake it for tests * Start impl it * Add test impl for non nasa code * Add swingmode to ProtocolRequest * Add alt_mode to ProtocolRequest * Add fan_mode to ProtocolRequest * Add target temp to ProtocolRequest * Impl ProtocolRequest for non Nasa * Remove oldstuff * Fix tests * Fix device access * Fix publishing --- components/samsung_ac/conversions.h | 1 - components/samsung_ac/protocol.h | 23 +++-- components/samsung_ac/protocol_nasa.cpp | 100 +++++++++++--------- components/samsung_ac/protocol_nasa.h | 7 +- components/samsung_ac/protocol_non_nasa.cpp | 61 ++++++------ components/samsung_ac/protocol_non_nasa.h | 10 +- components/samsung_ac/samsung_ac_device.cpp | 49 +++------- components/samsung_ac/samsung_ac_device.h | 42 ++++---- test/esphome/core/optional.h | 13 +++ test/main_test_nasa.cpp | 2 - test/main_test_non_nasa.cpp | 8 +- test/test_stuff.h | 1 + 12 files changed, 156 insertions(+), 161 deletions(-) create mode 100644 test/esphome/core/optional.h diff --git a/components/samsung_ac/conversions.h b/components/samsung_ac/conversions.h index ee94bc55..f38c22e5 100644 --- a/components/samsung_ac/conversions.h +++ b/components/samsung_ac/conversions.h @@ -26,6 +26,5 @@ namespace esphome climate::ClimateSwingMode swingmode_to_climateswingmode(SwingMode swingMode); SwingMode climateswingmode_to_swingmode(climate::ClimateSwingMode swingMode); - } // namespace samsung_ac } // namespace esphome diff --git a/components/samsung_ac/protocol.h b/components/samsung_ac/protocol.h index c7fd2a16..779be9c8 100644 --- a/components/samsung_ac/protocol.h +++ b/components/samsung_ac/protocol.h @@ -1,7 +1,7 @@ #pragma once -#include -#include +#include "esphome/core/optional.h" +#include "util.h" namespace esphome { @@ -41,7 +41,6 @@ namespace esphome Off = 5 }; - enum class AltMode { Unknown = -1, @@ -78,15 +77,21 @@ namespace esphome virtual void set_swing_horizontal(const std::string address, bool horizontal) = 0; }; + struct ProtocolRequest + { + public: + optional power; + optional mode; + optional target_temp; + optional fan_mode; + optional swing_mode; + optional alt_mode; + }; + class Protocol { public: - virtual void publish_power_message(MessageTarget *target, const std::string &address, bool value) = 0; - virtual void publish_target_temp_message(MessageTarget *target, const std::string &address, float value) = 0; - virtual void publish_mode_message(MessageTarget *target, const std::string &address, Mode value) = 0; - virtual void publish_fanmode_message(MessageTarget *target, const std::string &address, FanMode value) = 0; - virtual void publish_altmode_message(MessageTarget *target, const std::string &address, AltMode value) = 0; - virtual void publish_swing_mode_message(MessageTarget *target, const std::string &address, SwingMode value) = 0; + virtual void publish_request(MessageTarget *target, const std::string &address, ProtocolRequest &request) = 0; }; enum class DataResult diff --git a/components/samsung_ac/protocol_nasa.cpp b/components/samsung_ac/protocol_nasa.cpp index 4a24cb45..fd3f4ec5 100644 --- a/components/samsung_ac/protocol_nasa.cpp +++ b/components/samsung_ac/protocol_nasa.cpp @@ -347,30 +347,6 @@ namespace esphome return str; } - void NasaProtocol::publish_power_message(MessageTarget *target, const std::string &address, bool value) - { - auto packet = Packet::create(Address::parse(address), DataType::Request, MessageNumber::ENUM_in_operation_power, value ? 1 : 0); - auto data = packet.encode(); - target->publish_data(data); - } - - void NasaProtocol::publish_target_temp_message(MessageTarget *target, const std::string &address, float value) - { - auto packet = Packet::create(Address::parse(address), DataType::Request, MessageNumber::VAR_in_temp_target_f, value * 10.0); - auto data = packet.encode(); - target->publish_data(data); - } - - void NasaProtocol::publish_mode_message(MessageTarget *target, const std::string &address, Mode value) - { - auto packet = Packet::create(Address::parse(address), DataType::Request, MessageNumber::ENUM_in_operation_mode, (int)value); - MessageSet power(MessageNumber::ENUM_in_operation_power); - power.value = 1; - packet.messages.push_back(power); - auto data = packet.encode(); - target->publish_data(data); - } - int fanmode_to_nasa_fanmode(FanMode mode) { // This stuff did not exists in XML only in Remcode.dll @@ -390,14 +366,6 @@ namespace esphome } } - void NasaProtocol::publish_fanmode_message(MessageTarget *target, const std::string &address, FanMode value) - { - auto packet = Packet::create(Address::parse(address), DataType::Request, MessageNumber::ENUM_in_fan_mode, fanmode_to_nasa_fanmode(value)); - ESP_LOGW(TAG, "publish_fanmode_message %s", packet.to_string().c_str()); - auto data = packet.encode(); - target->publish_data(data); - } - int altmode_to_nasa_altmode(AltMode mode) { switch (mode) @@ -418,22 +386,64 @@ namespace esphome } } - void NasaProtocol::publish_altmode_message(MessageTarget *target, const std::string &address, AltMode value) + void NasaProtocol::publish_request(MessageTarget *target, const std::string &address, ProtocolRequest &request) { - auto packet = Packet::create(Address::parse(address), DataType::Request, MessageNumber::ENUM_in_alt_mode, altmode_to_nasa_altmode(value)); - ESP_LOGW(TAG, "publish_altmode_message %s", packet.to_string().c_str()); - auto data = packet.encode(); - target->publish_data(data); - } + Packet packet = Packet::createa_partial(Address::parse(address), DataType::Request); - void NasaProtocol::publish_swing_mode_message(MessageTarget *target, const std::string &address, SwingMode value) - { - auto packet = Packet::create(Address::parse(address), DataType::Request, MessageNumber::ENUM_in_louver_hl_swing, static_cast(value) & 1); - ESP_LOGW(TAG, "publish_swing_mode_message %s", packet.to_string().c_str()); + if (request.mode) + { + request.power = true; // ensure system turns on when mode is set + + MessageSet mode(MessageNumber::ENUM_in_operation_mode); + mode.value = (int)request.mode.value(); + packet.messages.push_back(mode); + } + + if (request.power) + { + MessageSet power(MessageNumber::ENUM_in_operation_power); + power.value = request.power.value() ? 1 : 0; + packet.messages.push_back(power); + } + + if (request.target_temp) + { + MessageSet targettemp(MessageNumber::VAR_in_temp_target_f); + targettemp.value = request.target_temp.value() * 10.0; + packet.messages.push_back(targettemp); + } + + if (request.fan_mode) + { + MessageSet fanmode(MessageNumber::ENUM_in_fan_mode); + fanmode.value = fanmode_to_nasa_fanmode(request.fan_mode.value()); + packet.messages.push_back(fanmode); + } - MessageSet lr_swing(MessageNumber::ENUM_in_louver_lr_swing); - lr_swing.value = (static_cast(value) >> 1) & 1; - packet.messages.push_back(lr_swing); + if (request.alt_mode) + { + MessageSet altmode(MessageNumber::ENUM_in_alt_mode); + altmode.value = altmode_to_nasa_altmode(request.alt_mode.value()); + packet.messages.push_back(altmode); + } + + if (request.swing_mode) + { + MessageSet hl_swing(MessageNumber::ENUM_in_louver_hl_swing); + hl_swing.value = static_cast(request.swing_mode.value()) & 1; + packet.messages.push_back(hl_swing); + + MessageSet lr_swing(MessageNumber::ENUM_in_louver_lr_swing); + lr_swing.value = (static_cast(request.swing_mode.value()) >> 1) & 1; + packet.messages.push_back(lr_swing); + } + + if (packet.messages.size() == 0) + return; + + ESP_LOGW(TAG, "publish packet %s", packet.to_string().c_str()); + + out.push_back(packet); auto data = packet.encode(); target->publish_data(data); diff --git a/components/samsung_ac/protocol_nasa.h b/components/samsung_ac/protocol_nasa.h index 17f4d53f..baa03deb 100644 --- a/components/samsung_ac/protocol_nasa.h +++ b/components/samsung_ac/protocol_nasa.h @@ -167,12 +167,7 @@ namespace esphome public: NasaProtocol() = default; - void publish_power_message(MessageTarget *target, const std::string &address, bool value) override; - void publish_target_temp_message(MessageTarget *target, const std::string &address, float value) override; - void publish_mode_message(MessageTarget *target, const std::string &address, Mode value) override; - void publish_fanmode_message(MessageTarget *target, const std::string &address, FanMode value) override; - void publish_altmode_message(MessageTarget *target, const std::string &address, AltMode value) override; - void publish_swing_mode_message(MessageTarget *target, const std::string &address, SwingMode value) override; + void publish_request(MessageTarget *target, const std::string &address, ProtocolRequest &request) override; }; } // namespace samsung_ac diff --git a/components/samsung_ac/protocol_non_nasa.cpp b/components/samsung_ac/protocol_non_nasa.cpp index 9a31f8c3..6a87b0e2 100644 --- a/components/samsung_ac/protocol_non_nasa.cpp +++ b/components/samsung_ac/protocol_non_nasa.cpp @@ -359,20 +359,6 @@ namespace esphome std::queue nonnasa_requests; - void NonNasaProtocol::publish_power_message(MessageTarget *target, const std::string &address, bool value) - { - auto request = NonNasaRequest::create(address); - request.power = value; - nonnasa_requests.push(request); - } - - void NonNasaProtocol::publish_target_temp_message(MessageTarget *target, const std::string &address, float value) - { - auto request = NonNasaRequest::create(address); - request.target_temp = value; - nonnasa_requests.push(request); - } - NonNasaMode mode_to_nonnasa_mode(Mode value) { switch (value) @@ -392,14 +378,6 @@ namespace esphome } } - void NonNasaProtocol::publish_mode_message(MessageTarget *target, const std::string &address, Mode value) - { - auto request = NonNasaRequest::create(address); - request.power = true; - request.mode = mode_to_nonnasa_mode(value); - nonnasa_requests.push(request); - } - NonNasaFanspeed fanmode_to_nonnasa_fanspeed(FanMode value) { switch (value) @@ -416,21 +394,36 @@ namespace esphome } } - void NonNasaProtocol::publish_fanmode_message(MessageTarget *target, const std::string &address, FanMode value) + void NonNasaProtocol::publish_request(MessageTarget *target, const std::string &address, ProtocolRequest &request) { - auto request = NonNasaRequest::create(address); - request.fanspeed = fanmode_to_nonnasa_fanspeed(value); - nonnasa_requests.push(request); - } + auto req = NonNasaRequest::create(address); - void NonNasaProtocol::publish_altmode_message(MessageTarget *target, const std::string &address, AltMode value) - { - ESP_LOGW(TAG, "change altmode is currently not implemented"); - } + if (request.mode) + { + request.power = true; // ensure system turns on when mode is set + req.mode = mode_to_nonnasa_mode(request.mode.value()); + } - void NonNasaProtocol::publish_swing_mode_message(MessageTarget *target, const std::string &address, SwingMode value) - { - ESP_LOGW(TAG, "change swingmode is currently not implemented"); + if (request.power) + req.power = request.power.value(); + + if (request.target_temp) + req.target_temp = request.target_temp.value(); + + if (request.fan_mode) + req.fanspeed = fanmode_to_nonnasa_fanspeed(request.fan_mode.value()); + + if (request.alt_mode) + { + ESP_LOGW(TAG, "change altmode is currently not implemented"); + } + + if (request.swing_mode) + { + ESP_LOGW(TAG, "change swingmode is currently not implemented"); + } + + nonnasa_requests.push(req); } Mode nonnasa_mode_to_mode(NonNasaMode value) diff --git a/components/samsung_ac/protocol_non_nasa.h b/components/samsung_ac/protocol_non_nasa.h index e426dc6f..e89c9c46 100644 --- a/components/samsung_ac/protocol_non_nasa.h +++ b/components/samsung_ac/protocol_non_nasa.h @@ -109,7 +109,6 @@ namespace esphome std::string to_string(); }; - struct NonNasaCommandF3 // from outdoor unit { uint8_t inverter_max_frequency_hz = 0; @@ -141,7 +140,7 @@ namespace esphome CmdC6 = 0xc6, CmdF0 = 0xf0, CmdF1 = 0xf1, - CmdF3 = 0xf3, + CmdF3 = 0xf3, CmdF8 = 0xF8, }; @@ -197,12 +196,7 @@ namespace esphome public: NonNasaProtocol() = default; - void publish_power_message(MessageTarget *target, const std::string &address, bool value) override; - void publish_target_temp_message(MessageTarget *target, const std::string &address, float value) override; - void publish_mode_message(MessageTarget *target, const std::string &address, Mode value) override; - void publish_fanmode_message(MessageTarget *target, const std::string &address, FanMode value) override; - void publish_altmode_message(MessageTarget *target, const std::string &address, AltMode value) override; - void publish_swing_mode_message(MessageTarget *target, const std::string &address, SwingMode value) override; + void publish_request(MessageTarget *target, const std::string &address, ProtocolRequest &request) override; }; } // namespace samsung_ac } // namespace esphome diff --git a/components/samsung_ac/samsung_ac_device.cpp b/components/samsung_ac/samsung_ac_device.cpp index af34acb5..fb03c82e 100644 --- a/components/samsung_ac/samsung_ac_device.cpp +++ b/components/samsung_ac/samsung_ac_device.cpp @@ -74,83 +74,56 @@ namespace esphome { traits(); + ProtocolRequest request; + auto targetTempOpt = call.get_target_temperature(); if (targetTempOpt.has_value()) - device->write_target_temperature(targetTempOpt.value()); + request.target_temp = targetTempOpt.value(); auto modeOpt = call.get_mode(); if (modeOpt.has_value()) { if (modeOpt.value() == climate::ClimateMode::CLIMATE_MODE_OFF) { - device->write_power(false); + request.power = false; } else { - device->write_mode(climatemode_to_mode(modeOpt.value())); + request.mode = climatemode_to_mode(modeOpt.value()); } } auto fanmodeOpt = call.get_fan_mode(); if (fanmodeOpt.has_value()) { - device->write_fanmode(climatefanmode_to_fanmode(fanmodeOpt.value())); + request.fan_mode = climatefanmode_to_fanmode(fanmodeOpt.value()); } auto customFanmodeOpt = call.get_custom_fan_mode(); if (customFanmodeOpt.has_value()) { - device->write_fanmode(customfanmode_to_fanmode(customFanmodeOpt.value())); + request.fan_mode = customfanmode_to_fanmode(customFanmodeOpt.value()); } auto presetOpt = call.get_preset(); if (presetOpt.has_value()) { - device->write_altmode(preset_to_altmode(presetOpt.value())); + request.alt_mode = preset_to_altmode(presetOpt.value()); } auto customPresetOpt = call.get_custom_preset(); if (customPresetOpt.has_value()) { - device->write_altmode(custompreset_to_altmode(customPresetOpt.value())); + request.alt_mode = custompreset_to_altmode(customPresetOpt.value()); } auto swingModeOpt = call.get_swing_mode(); if (swingModeOpt.has_value()) { - device->write_swing_mode(climateswingmode_to_swingmode(swingModeOpt.value())); + request.swing_mode = climateswingmode_to_swingmode(swingModeOpt.value()); } - } - - void Samsung_AC_Device::write_target_temperature(float value) - { - protocol->publish_target_temp_message(samsung_ac, address, value); - } - - void Samsung_AC_Device::write_mode(Mode value) - { - protocol->publish_mode_message(samsung_ac, address, value); - } - - void Samsung_AC_Device::write_fanmode(FanMode value) - { - protocol->publish_fanmode_message(samsung_ac, address, value); - } - - void Samsung_AC_Device::write_altmode(AltMode value) - { - protocol->publish_altmode_message(samsung_ac, address, value); - } - - void Samsung_AC_Device::write_swing_mode(SwingMode value) - { - protocol->publish_swing_mode_message(samsung_ac, address, value); - } - - void Samsung_AC_Device::write_power(bool value) - { - protocol->publish_power_message(samsung_ac, address, value); + device->publish_request(request); } } // namespace samsung_ac } // namespace esphome diff --git a/components/samsung_ac/samsung_ac_device.h b/components/samsung_ac/samsung_ac_device.h index 55cd6c76..f1d69cba 100644 --- a/components/samsung_ac/samsung_ac_device.h +++ b/components/samsung_ac/samsung_ac_device.h @@ -68,10 +68,10 @@ namespace esphome class Samsung_AC_Device { public: - Samsung_AC_Device(const std::string &address, Samsung_AC *samsung_ac) + Samsung_AC_Device(const std::string &address, MessageTarget *target) { this->address = address; - this->samsung_ac = samsung_ac; + this->target = target; this->protocol = get_protocol(address); } @@ -98,7 +98,9 @@ namespace esphome power = switch_; power->write_state_ = [this](bool value) { - write_power(value); + ProtocolRequest request; + request.power = value; + publish_request(request); }; } @@ -107,7 +109,9 @@ namespace esphome mode = select; mode->write_state_ = [this](Mode value) { - write_mode(value); + ProtocolRequest request; + request.mode = value; + publish_request(request); }; } @@ -116,7 +120,9 @@ namespace esphome target_temperature = number; target_temperature->write_state_ = [this](float value) { - write_target_temperature(value); + ProtocolRequest request; + request.target_temp = value; + publish_request(request); }; }; @@ -165,10 +171,13 @@ namespace esphome climate->fan_mode = fanmode_to_climatefanmode(value); auto fanmode = fanmode_to_climatefanmode(value); - if (fanmode.has_value()) { + if (fanmode.has_value()) + { climate->fan_mode = fanmode; climate->custom_fan_mode.reset(); - } else { + } + else + { climate->fan_mode.reset(); climate->custom_fan_mode = fanmode_to_custom_climatefanmode(value); } @@ -181,10 +190,13 @@ namespace esphome if (climate != nullptr) { auto preset = altmode_to_preset(value); - if (preset.has_value()) { + if (preset.has_value()) + { climate->preset = preset; climate->custom_preset.reset(); - } else { + } + else + { climate->preset.reset(); climate->custom_preset = altmode_to_custompreset(value); } @@ -233,16 +245,14 @@ namespace esphome room_humidity->publish_state(value); } - void write_target_temperature(float value); - void write_mode(Mode value); - void write_fanmode(FanMode value); - void write_altmode(AltMode value); - void write_swing_mode(SwingMode value); - void write_power(bool value); + void publish_request(ProtocolRequest &request) + { + protocol->publish_request(target, address, request); + } protected: Protocol *protocol{nullptr}; - Samsung_AC *samsung_ac{nullptr}; + MessageTarget *target{nullptr}; void calc_and_publish_mode() { diff --git a/test/esphome/core/optional.h b/test/esphome/core/optional.h new file mode 100644 index 00000000..3518c53a --- /dev/null +++ b/test/esphome/core/optional.h @@ -0,0 +1,13 @@ +#pragma once + +#include + +// Fakes the esphome optional type with std experimental/optional + +namespace esphome +{ + template + using optional = std::experimental::optional; + /* using opt_null_t = std::experimental::nullopt_t; + constexpr auto nullopt = std::experimental::nullopt;*/ +} \ No newline at end of file diff --git a/test/main_test_nasa.cpp b/test/main_test_nasa.cpp index 5d0dc6d4..e779ff04 100644 --- a/test/main_test_nasa.cpp +++ b/test/main_test_nasa.cpp @@ -42,8 +42,6 @@ void test_process_data() { } -// - int main(int argc, char *argv[]) { test_nasa_1(); diff --git a/test/main_test_non_nasa.cpp b/test/main_test_non_nasa.cpp index 5db36ab4..bbc2e9f3 100644 --- a/test/main_test_non_nasa.cpp +++ b/test/main_test_non_nasa.cpp @@ -264,7 +264,9 @@ void test_previous_data_is_used_correctly() // prepare last values test_process_data("3200c8204d51500001100051e434", target); - get_protocol("00")->publish_power_message(&target, "00", false); + ProtocolRequest req1; + req1.power = false; + get_protocol("00")->publish_request(&target, "00", req1); test_process_data("32c8f0f80345f0c913000000ac34", target); // trigger publish NonNasaRequest request1; @@ -281,7 +283,9 @@ void test_previous_data_is_used_correctly() // prepare last values test_process_data("3201c8204f4f4efd821c004e8a34", target); - get_protocol("01")->publish_power_message(&target, "01", true); + ProtocolRequest req2; + req2.power = true; + get_protocol("01")->publish_request(&target, "01", req2); test_process_data("32c8f0f80345f0c913000000ac34", target); // trigger publish NonNasaRequest request2; diff --git a/test/test_stuff.h b/test/test_stuff.h index 7ce53d7c..cd1d0bff 100644 --- a/test/test_stuff.h +++ b/test/test_stuff.h @@ -3,6 +3,7 @@ #include #include #include +#include #include "../components/samsung_ac/util.h" #include "../components/samsung_ac/protocol.h"