[mk2pvrouter] Add tag-based sensor defaults (#18870)

Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com>
Co-authored-by: J. Nick Koston <nick@koston.org>
Co-authored-by: J. Nick Koston <nick@home-assistant.io>

esphome/components/mk2pvrouter/mk2pvrouter.h

@@ -33,7 +33,7 @@ class Mk2PVRouterListener {
 };
 
 // Reads frames via UART, validates their CRC, and publishes tag/value pairs to listeners.
-class Mk2PVRouter final : public Component, public uart::UARTDevice {
+class Mk2PVRouter : public Component, public uart::UARTDevice {
  public:
 #ifdef MK2PVROUTER_LISTENER_COUNT
   void register_mk2pvrouter_listener(Mk2PVRouterListener *listener);

esphome/components/mk2pvrouter/sensor/__init__.py

@@ -1,6 +1,31 @@
+from dataclasses import dataclass
+from typing import Any
+
 import esphome.codegen as cg
 from esphome.components import sensor
-from esphome.const import CONF_ID, CONF_TAG
+import esphome.config_validation as cv
+from esphome.const import (
+    CONF_ACCURACY_DECIMALS,
+    CONF_DEVICE_CLASS,
+    CONF_ID,
+    CONF_STATE_CLASS,
+    CONF_TAG,
+    CONF_UNIT_OF_MEASUREMENT,
+    DEVICE_CLASS_EMPTY,
+    DEVICE_CLASS_ENERGY,
+    DEVICE_CLASS_POWER,
+    DEVICE_CLASS_TEMPERATURE,
+    DEVICE_CLASS_VOLTAGE,
+    STATE_CLASS_MEASUREMENT,
+    STATE_CLASS_NONE,
+    STATE_CLASS_TOTAL_INCREASING,
+    UNIT_CELSIUS,
+    UNIT_EMPTY,
+    UNIT_PERCENT,
+    UNIT_VOLT,
+    UNIT_WATT,
+    UNIT_WATT_HOURS,
+)
 from esphome.types import ConfigType
 
 from .. import (
@@ -14,13 +39,89 @@ Mk2PVRouterSensor = mk2pvrouter_ns.class_(
     "Mk2PVRouterSensor", sensor.Sensor, cg.Component
 )
 
-CONFIG_SCHEMA = sensor.sensor_schema(Mk2PVRouterSensor).extend(
-    MK2PVROUTER_LISTENER_SCHEMA
+
+@dataclass(frozen=True)
+class TagKind:
+    """Sensor defaults for one kind of Mk2PVRouter output."""
+
+    unit_of_measurement: str
+    device_class: str
+    state_class: str
+    accuracy_decimals: int
+    # The device sends the value * 100; Mk2PVRouterSensor::publish_val() corrects it.
+    scale_centi: bool = False
+
+    def defaults(self) -> dict[str, Any]:
+        return {
+            CONF_UNIT_OF_MEASUREMENT: self.unit_of_measurement,
+            CONF_DEVICE_CLASS: self.device_class,
+            CONF_STATE_CLASS: self.state_class,
+            CONF_ACCURACY_DECIMALS: self.accuracy_decimals,
+        }
+
+
+POWER = TagKind(UNIT_WATT, DEVICE_CLASS_POWER, STATE_CLASS_MEASUREMENT, 0)
+VOLTAGE = TagKind(
+    UNIT_VOLT, DEVICE_CLASS_VOLTAGE, STATE_CLASS_MEASUREMENT, 2, scale_centi=True
+)
+ENERGY = TagKind(UNIT_WATT_HOURS, DEVICE_CLASS_ENERGY, STATE_CLASS_TOTAL_INCREASING, 0)
+TEMPERATURE = TagKind(
+    UNIT_CELSIUS, DEVICE_CLASS_TEMPERATURE, STATE_CLASS_MEASUREMENT, 2, scale_centi=True
+)
+RELAY_STATE = TagKind(UNIT_EMPTY, DEVICE_CLASS_EMPTY, STATE_CLASS_NONE, 0)
+DIVERSION_RATE = TagKind(UNIT_PERCENT, DEVICE_CLASS_EMPTY, STATE_CLASS_MEASUREMENT, 0)
+
+# Keyed by (letter, indexed). Per the Mk2PVRouter firmware protocol, D and R mean different
+# things bare and indexed: D is diverted power (W) but D1, D2, ... are diversion rates (%);
+# R is mean relay power (W) but R1, R2, ... are relay states. T is always indexed, E never.
+TAG_KINDS = {
+    ("P", False): POWER,
+    ("P", True): POWER,
+    ("D", False): POWER,
+    ("D", True): DIVERSION_RATE,
+    ("V", False): VOLTAGE,
+    ("V", True): VOLTAGE,
+    ("E", False): ENERGY,
+    ("T", True): TEMPERATURE,
+    ("R", False): POWER,
+    ("R", True): RELAY_STATE,
+}
+
+
+def tag_kind(tag: str) -> TagKind | None:
+    """The kind of a tag such as P, V1 or R10, or None for a tag that is not a known output."""
+    tag = tag.upper()
+    index = tag[1:]
+    if index and not index.isdigit():
+        return None
+    return TAG_KINDS.get((tag[:1], bool(index)))
+
+
+def _inject_tag_defaults(config: ConfigType) -> ConfigType:
+    """Fill in the tag's defaults for keys the user did not set; the sensor schema validates them."""
+    if (
+        isinstance(config, dict)
+        and isinstance(config.get(CONF_TAG), str)
+        and (kind := tag_kind(config[CONF_TAG])) is not None
+    ):
+        config = {**kind.defaults(), **config}
+    return config
+
+
+CONFIG_SCHEMA = cv.All(
+    _inject_tag_defaults,
+    sensor.sensor_schema(
+        Mk2PVRouterSensor,
+        accuracy_decimals=0,
+        state_class=STATE_CLASS_MEASUREMENT,
+    ).extend(MK2PVROUTER_LISTENER_SCHEMA),
 )
 
 
 async def to_code(config: ConfigType) -> None:
-    var = cg.new_Pvariable(config[CONF_ID], config[CONF_TAG])
+    tag = config[CONF_TAG]
+    kind = tag_kind(tag)
+    var = cg.new_Pvariable(config[CONF_ID], tag, kind is not None and kind.scale_centi)
     await cg.register_component(var, config)
     await sensor.register_sensor(var, config)
     mk2pvrouter = await cg.get_variable(config[CONF_MK2PVROUTER_ID])

esphome/components/mk2pvrouter/sensor/mk2pvrouter_sensor.cpp

@@ -5,7 +5,8 @@ namespace esphome::mk2pvrouter {
 
 static const char *const TAG = "mk2pvrouter_sensor";
 
-Mk2PVRouterSensor::Mk2PVRouterSensor(const char *tag) : Mk2PVRouterListener(tag) {}
+Mk2PVRouterSensor::Mk2PVRouterSensor(const char *tag, bool scale_centi)
+    : Mk2PVRouterListener(tag), scale_centi_(scale_centi) {}
 
 void Mk2PVRouterSensor::publish_val(const char *val) {
   auto result = parse_number<float>(val);
@@ -13,7 +14,11 @@ void Mk2PVRouterSensor::publish_val(const char *val) {
     ESP_LOGW(TAG, "Failed to parse value '%s' for tag '%s'", val, this->get_tag());
     return;
   }
-  this->publish_state(result.value());
+  float value = result.value();
+  if (this->scale_centi_) {
+    value *= 0.01f;
+  }
+  this->publish_state(value);
 }
 
 void Mk2PVRouterSensor::dump_config() {

esphome/components/mk2pvrouter/sensor/mk2pvrouter_sensor.h

@@ -5,11 +5,15 @@
 
 namespace esphome::mk2pvrouter {
 
-class Mk2PVRouterSensor final : public Mk2PVRouterListener, public sensor::Sensor, public Component {
+// Component before Sensor so the flag lands in Sensor's tail padding
+class Mk2PVRouterSensor final : public Mk2PVRouterListener, public Component, public sensor::Sensor {
  public:
-  explicit Mk2PVRouterSensor(const char *tag);
+  Mk2PVRouterSensor(const char *tag, bool scale_centi);
   void publish_val(const char *val) override;
   void dump_config() override;
+
+ protected:
+  bool scale_centi_;
 };
 
 }  // namespace esphome::mk2pvrouter

tests/component_tests/mk2pvrouter/test_sensor_defaults.py

@@ -0,0 +1,84 @@
+"""Tag-based sensor defaults, driven through the real CONFIG_SCHEMA."""
+
+import pytest
+
+from esphome.components import sensor
+from esphome.components.mk2pvrouter.sensor import CONFIG_SCHEMA, tag_kind
+from esphome.const import (
+    CONF_ACCURACY_DECIMALS,
+    CONF_DEVICE_CLASS,
+    CONF_STATE_CLASS,
+    CONF_UNIT_OF_MEASUREMENT,
+    DEVICE_CLASS_EMPTY,
+    STATE_CLASS_MEASUREMENT,
+)
+from esphome.types import ConfigType
+
+
+def _sensor(tag: str, **extra: object) -> ConfigType:
+    return CONFIG_SCHEMA(
+        {"tag": tag, "mk2pvrouter_id": "hub", "name": f"{tag} sensor", **extra}
+    )
+
+
+@pytest.mark.parametrize(
+    ("tag", "unit", "device_class", "state_class", "decimals"),
+    [
+        ("P", "W", "power", "measurement", 0),
+        ("P1", "W", "power", "measurement", 0),
+        ("D", "W", "power", "measurement", 0),
+        ("D1", "%", "", "measurement", 0),
+        ("V", "V", "voltage", "measurement", 2),
+        ("v1", "V", "voltage", "measurement", 2),
+        ("E", "Wh", "energy", "total_increasing", 0),
+        ("T1", "°C", "temperature", "measurement", 2),
+        ("R", "W", "power", "measurement", 0),
+        ("R1", "", "", "", 0),
+        ("R10", "", "", "", 0),
+    ],
+)
+def test_tag_defaults(
+    tag: str, unit: str, device_class: str, state_class: str, decimals: int
+) -> None:
+    config = _sensor(tag)
+    assert config[CONF_UNIT_OF_MEASUREMENT] == unit
+    assert config[CONF_DEVICE_CLASS] == device_class
+    assert config[CONF_STATE_CLASS] == sensor.validate_state_class(state_class)
+    assert config[CONF_ACCURACY_DECIMALS] == decimals
+
+
+@pytest.mark.parametrize("tag", ["S_MC", "STATUS", "X9", "Z", "T", "E1"])
+def test_unknown_tag_gets_only_the_schema_defaults(tag: str) -> None:
+    config = _sensor(tag)
+    assert CONF_UNIT_OF_MEASUREMENT not in config
+    assert CONF_DEVICE_CLASS not in config
+    assert config[CONF_STATE_CLASS] == sensor.validate_state_class(
+        STATE_CLASS_MEASUREMENT
+    )
+    assert config[CONF_ACCURACY_DECIMALS] == 0
+
+
+def test_explicit_values_win_over_tag_defaults() -> None:
+    config = _sensor("P", device_class=DEVICE_CLASS_EMPTY, accuracy_decimals=3)
+    assert config[CONF_DEVICE_CLASS] == DEVICE_CLASS_EMPTY
+    assert config[CONF_ACCURACY_DECIMALS] == 3
+    assert config[CONF_UNIT_OF_MEASUREMENT] == "W"
+
+
+@pytest.mark.parametrize(
+    ("tag", "scale_centi"),
+    [
+        ("V", True),
+        ("V1", True),
+        ("v2", True),
+        ("T1", True),
+        ("P", False),
+        ("D1", False),
+        ("E", False),
+        ("T", False),
+        ("X9", False),
+    ],
+)
+def test_centi_scale_follows_the_tag(tag: str, scale_centi: bool) -> None:
+    kind = tag_kind(tag)
+    assert (kind is not None and kind.scale_centi) is scale_centi

tests/components/mk2pvrouter/__init__.py

@@ -0,0 +1,11 @@
+import esphome.codegen as cg
+from esphome.types import ConfigType
+from tests.testing_helpers import ComponentManifestOverride
+
+
+def override_manifest(manifest: ComponentManifestOverride) -> None:
+    async def to_code_testing(config: ConfigType) -> None:
+        # Listener storage is sized by code generation; the gtests register one listener per hub.
+        cg.add_define("MK2PVROUTER_LISTENER_COUNT", 1)
+
+    manifest.to_code = to_code_testing

tests/components/mk2pvrouter/common.h

@@ -0,0 +1,55 @@
+#pragma once
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+#include <algorithm>
+#include <cstdint>
+#include <initializer_list>
+#include <vector>
+#include "esphome/components/uart/uart_component.h"
+#include "esphome/components/mk2pvrouter/mk2pvrouter.h"
+
+namespace esphome::mk2pvrouter::testing {
+
+class MockUARTComponent : public uart::UARTComponent {
+ public:
+  std::vector<uint8_t> tx;
+  std::vector<uint8_t> rx;
+
+  void push_rx(std::initializer_list<uint8_t> data) { this->rx.insert(this->rx.end(), data.begin(), data.end()); }
+
+  // UARTComponent
+  void write_array(const uint8_t *data, size_t len) override { this->tx.insert(this->tx.end(), data, data + len); }
+
+  bool read_array(uint8_t *data, size_t len) override {
+    if (this->rx.size() < len) {
+      return false;
+    }
+
+    std::copy(this->rx.begin(), this->rx.begin() + len, data);
+    this->rx.erase(this->rx.begin(), this->rx.begin() + len);
+    return true;
+  }
+
+  size_t available() override { return this->rx.size(); }
+
+  MOCK_METHOD(bool, peek_byte, (uint8_t * data), (override));
+  MOCK_METHOD(uart::UARTFlushResult, flush, (), (override));
+  MOCK_METHOD(void, check_logger_conflict, (), (override));
+#if defined(USE_ESP8266) || defined(USE_ESP32)
+  void load_settings(bool dump_config) override {}
+#endif  // defined(USE_ESP8266) || defined(USE_ESP32)
+};
+
+class TestableMk2PVRouter : public Mk2PVRouter {
+ public:
+  using Mk2PVRouter::Mk2PVRouter;
+  using Mk2PVRouter::State;
+  using Mk2PVRouter::buf_index_;
+  using Mk2PVRouter::calculate_crc_;
+  using Mk2PVRouter::check_crc_;
+  using Mk2PVRouter::process_group_;
+  using Mk2PVRouter::state_;
+};
+
+}  // namespace esphome::mk2pvrouter::testing

tests/components/mk2pvrouter/common.yaml

@@ -3,44 +3,59 @@ mk2pvrouter:
   uart_id: uart_bus
 
 sensor:
+  # Exact tags
   - platform: mk2pvrouter
     name: Power
     tag: P
     mk2pvrouter_id: test_mk2pvrouter
-    unit_of_measurement: W
-    device_class: power
-    state_class: measurement
-    accuracy_decimals: 0
-
-  - platform: mk2pvrouter
-    name: Voltage
-    tag: V
-    mk2pvrouter_id: test_mk2pvrouter
-    unit_of_measurement: V
-    device_class: voltage
-    state_class: measurement
-    accuracy_decimals: 2
-    filters:
-      # Device sends voltage * 100
-      - multiply: 0.01
+    # An explicit value must win over the tag default
+    device_class: ""
 
   - platform: mk2pvrouter
     name: Energy
     tag: E
     mk2pvrouter_id: test_mk2pvrouter
-    unit_of_measurement: Wh
-    device_class: energy
-    state_class: total_increasing
-    accuracy_decimals: 0
+
+  # Indexed tags
+  - platform: mk2pvrouter
+    name: Power Phase 1
+    tag: P1
+    mk2pvrouter_id: test_mk2pvrouter
 
   - platform: mk2pvrouter
-    name: Temperature
+    name: Power Phase 2
+    tag: P2
+    mk2pvrouter_id: test_mk2pvrouter
+
+  - platform: mk2pvrouter
+    name: Voltage Phase 1
+    tag: V1
+    mk2pvrouter_id: test_mk2pvrouter
+
+  # The x0.01 correction is applied before filters, so a user filter stacks on the corrected value
+  - platform: mk2pvrouter
+    name: Voltage Phase 2 Scaled
+    tag: V2
+    mk2pvrouter_id: test_mk2pvrouter
+    filters:
+      - multiply: 1.0
+
+  - platform: mk2pvrouter
+    name: Temperature 1
     tag: T1
     mk2pvrouter_id: test_mk2pvrouter
-    unit_of_measurement: "°C"
-    device_class: temperature
-    state_class: measurement
-    accuracy_decimals: 2
-    filters:
-      # Device sends temperature * 100
-      - multiply: 0.01
+
+  - platform: mk2pvrouter
+    name: Diversion Rate 1
+    tag: D1
+    mk2pvrouter_id: test_mk2pvrouter
+
+  - platform: mk2pvrouter
+    name: Relay 1 State
+    tag: R1
+    mk2pvrouter_id: test_mk2pvrouter
+
+  - platform: mk2pvrouter
+    name: Relay 10 State
+    tag: R10
+    mk2pvrouter_id: test_mk2pvrouter

tests/components/mk2pvrouter/mk2pvrouter_test.cpp

@@ -0,0 +1,112 @@
+#include <gtest/gtest.h>
+
+#include <string>
+
+#include "esphome/components/mk2pvrouter/mk2pvrouter.h"
+#include "common.h"
+
+namespace esphome::mk2pvrouter::testing {
+
+namespace {
+class TestListener final : public Mk2PVRouterListener {
+ public:
+  explicit TestListener(const char *tag) : Mk2PVRouterListener(tag) {}
+  void publish_val(const char *val) override {
+    this->published_ = true;
+    this->last_val_ = val;
+  }
+
+  bool published_{false};
+  std::string last_val_;
+};
+
+class Mk2PVRouterTest : public ::testing::Test {
+ protected:
+  void SetUp() override { this->sut_.register_mk2pvrouter_listener(&this->listener_); }
+
+  // Feeds one "tag<TAB>value<TAB>crc" literal, as the frame parser does after END_FRAME.
+  template<size_t N> void process_(const char (&group)[N]) { this->sut_.process_group_(group, group + N - 1); }
+
+  TestableMk2PVRouter sut_;
+  TestListener listener_{"P1"};
+};
+}  // namespace
+
+TEST(Mk2PVRouterListenerTest, GetTagReturnsConstructorTag) {
+  TestListener listener("P1");
+  EXPECT_STREQ(listener.get_tag(), "P1");
+}
+
+TEST_F(Mk2PVRouterTest, CalculateCrcMatchesKnownGroup) {
+  // "P1\t1234\t" sums to 0x3D ('=') per the mk2pvrouter CRC algorithm.
+  const char grp[] = "P1\t1234\t=";
+  EXPECT_EQ(this->sut_.calculate_crc_(grp, sizeof(grp) - 1), '=');
+}
+
+TEST_F(Mk2PVRouterTest, CheckCrcAcceptsMatchingCrc) {
+  const char grp[] = "P1\t1234\t=";
+  EXPECT_TRUE(this->sut_.check_crc_(grp, grp + sizeof(grp) - 1));
+}
+
+TEST_F(Mk2PVRouterTest, CheckCrcRejectsMismatchedCrc) {
+  const char grp[] = "P1\t1234\t!";
+  EXPECT_FALSE(this->sut_.check_crc_(grp, grp + sizeof(grp) - 1));
+}
+
+TEST_F(Mk2PVRouterTest, CheckCrcRejectsEmptyGroup) {
+  const char grp[] = "";
+  EXPECT_FALSE(this->sut_.check_crc_(grp, grp));
+}
+
+TEST_F(Mk2PVRouterTest, ProcessGroupPublishesValidGroupToMatchingListener) {
+  this->process_("P1\t1234\t=");
+  EXPECT_TRUE(this->listener_.published_);
+  EXPECT_EQ(this->listener_.last_val_, "1234");
+}
+
+TEST_F(Mk2PVRouterTest, ProcessGroupDropsGroupWithBadCrc) {
+  this->process_("P1\t1234\t!");
+  EXPECT_FALSE(this->listener_.published_);
+}
+
+TEST_F(Mk2PVRouterTest, ProcessGroupDropsGroupMissingValue) {
+  // "P1\t" sums to 0x2A ('*'), so the CRC passes, but there is no second TAB for the value.
+  this->process_("P1\t*");
+  EXPECT_FALSE(this->listener_.published_);
+}
+
+TEST_F(Mk2PVRouterTest, LoopParsesFullFrameAndPublishesToListener) {
+  MockUARTComponent uart;
+  this->sut_.set_uart_parent(&uart);
+
+  EXPECT_EQ(this->sut_.state_, TestableMk2PVRouter::State::WAITING_FOR_START);
+
+  // STX, LF "P1\t1234\t=" CR, ETX
+  uart.push_rx({0x02, 0x0a, 'P', '1', '\t', '1', '2', '3', '4', '\t', '=', 0x0d, 0x03});
+
+  this->sut_.loop();  // Consume bytes up to and including START_FRAME.
+  EXPECT_EQ(this->sut_.state_, TestableMk2PVRouter::State::START_FRAME_RECEIVED);
+
+  this->sut_.loop();  // Buffer the frame body up to END_FRAME.
+  EXPECT_EQ(this->sut_.state_, TestableMk2PVRouter::State::END_FRAME_RECEIVED);
+
+  this->sut_.loop();  // Parse the buffered groups and publish them.
+  EXPECT_EQ(this->sut_.state_, TestableMk2PVRouter::State::WAITING_FOR_START);
+  EXPECT_EQ(this->sut_.buf_index_, 0);
+  EXPECT_TRUE(this->listener_.published_);
+  EXPECT_EQ(this->listener_.last_val_, "1234");
+}
+
+TEST_F(Mk2PVRouterTest, LoopIgnoresBytesBeforeStartFrame) {
+  MockUARTComponent uart;
+  this->sut_.set_uart_parent(&uart);
+
+  uart.push_rx({'g', 'a', 'r', 'b', 'a', 'g', 'e', 0x02});
+
+  this->sut_.loop();
+
+  EXPECT_EQ(this->sut_.state_, TestableMk2PVRouter::State::START_FRAME_RECEIVED);
+  EXPECT_EQ(this->sut_.buf_index_, 0);
+}
+
+}  // namespace esphome::mk2pvrouter::testing

tests/components/mk2pvrouter/sensor/mk2pvrouter_sensor_test.cpp

@@ -0,0 +1,25 @@
+#include <gtest/gtest.h>
+
+#include "esphome/components/mk2pvrouter/sensor/mk2pvrouter_sensor.h"
+
+namespace esphome::mk2pvrouter::testing {
+
+TEST(Mk2PVRouterSensorTest, ScaleCentiTrueScalesByOneHundredth) {
+  Mk2PVRouterSensor sensor("V", true);
+  sensor.publish_val("23042");
+  EXPECT_FLOAT_EQ(sensor.get_raw_state(), 230.42f);
+}
+
+TEST(Mk2PVRouterSensorTest, ScaleCentiFalseDoesNotScale) {
+  Mk2PVRouterSensor sensor("P1", false);
+  sensor.publish_val("1234");
+  EXPECT_FLOAT_EQ(sensor.get_raw_state(), 1234.0f);
+}
+
+TEST(Mk2PVRouterSensorTest, UnparseableValueIsNotPublished) {
+  Mk2PVRouterSensor sensor("V1", true);
+  sensor.publish_val("not-a-number");
+  EXPECT_FALSE(sensor.has_state());
+}
+
+}  // namespace esphome::mk2pvrouter::testing