[sx1509] Use Apply outputs for the LED channels

The LED driver outputs become output::ApplyFloatOutput sharing one apply
function per hub. Codegen registers each pin with the duty the old channel's
setup() wrote through turn_off(), and the hub sets the channels up at the end
of its own setup() in config order. Duplicate pins on one hub are rejected.

esphome/components/output/__init__.py

@@ -132,7 +132,7 @@ BINARY_OUTPUT_ACTION_SCHEMA = maybe_simple_id(
 
 
 def final_validate_unique_channel(
-    *, platform: str, hub_key: str
+    *, platform: str, hub_key: str, channel_key: str = CONF_CHANNEL
 ) -> Callable[[ConfigType], ConfigType]:
     """Reject a second output of platform on the same channel of one hub.
 
@@ -141,7 +141,7 @@ def final_validate_unique_channel(
     return fv.unique_platform_key(
         domain=DOMAIN,
         platform=platform,
-        keys=(hub_key, CONF_CHANNEL),
+        keys=(hub_key, channel_key),
         message="Channel '{1}' of '{0}' is already used by another output",
     )
 

esphome/components/sx1509/output/__init__.py

@@ -1,30 +1,58 @@
+import math
+import struct
+
 import esphome.codegen as cg
 from esphome.components import output
 import esphome.config_validation as cv
-from esphome.const import CONF_ID, CONF_PIN
+from esphome.const import (
+    CONF_ID,
+    CONF_INVERTED,
+    CONF_MAX_POWER,
+    CONF_MIN_POWER,
+    CONF_PIN,
+)
 from esphome.types import ConfigType
 
-from .. import CONF_SX1509_ID, SX1509Component, sx1509_ns
+from .. import CONF_SX1509_ID, SX1509Component
 
 DEPENDENCIES = ["sx1509"]
 
-SX1509FloatOutputChannel = sx1509_ns.class_(
-    "SX1509FloatOutputChannel", output.FloatOutput, cg.Component
-)
-
 CONFIG_SCHEMA = output.FLOAT_OUTPUT_SCHEMA.extend(
     {
-        cv.Required(CONF_ID): cv.declare_id(SX1509FloatOutputChannel),
+        cv.Required(CONF_ID): cv.declare_id(output.ApplyFloatOutput),
         cv.GenerateID(CONF_SX1509_ID): cv.use_id(SX1509Component),
         cv.Required(CONF_PIN): cv.int_range(min=0, max=15),
     }
-).extend(cv.COMPONENT_SCHEMA)
+)
+
+
+FINAL_VALIDATE_SCHEMA = output.final_validate_unique_channel(
+    platform="sx1509", hub_key=CONF_SX1509_ID, channel_key=CONF_PIN
+)
+
+_request_led_slot = cg.slot_counter("SX1509_LED_CHANNEL_COUNT")
+
+
+def _f32(value: float) -> float:
+    return struct.unpack("f", struct.pack("f", value))[0]
+
+
+def initial_duty(config: ConfigType) -> int:
+    """The duty FloatOutput::turn_off() writes, in float math like the C++ code."""
+    # set_max_power() runs first and clamps against the default min_power of 0
+    max_power = min(max(_f32(config.get(CONF_MAX_POWER, 1.0)), 0.0), 1.0)
+    level = 0.0
+    if not config[output.CONF_ZERO_MEANS_ZERO]:
+        level = min(max(_f32(config.get(CONF_MIN_POWER, 0.0)), 0.0), max_power)
+    if config.get(CONF_INVERTED):
+        level = _f32(1.0 - level)
+    return int(math.floor(_f32(level * 255) + 0.5))
 
 
 async def to_code(config: ConfigType) -> None:
-    parent = await cg.get_variable(config[CONF_SX1509_ID])
-    var = cg.new_Pvariable(config[CONF_ID])
-    await cg.register_component(var, config)
-    await output.register_output(var, config)
-    cg.add(var.set_pin(config[CONF_PIN]))
-    cg.add(var.set_parent(parent))
+    hub = await cg.get_variable(config[CONF_SX1509_ID])
+    await output.sub_float_outputs(config, parent=hub).create(
+        config, None, config[CONF_PIN], apply="write_led(index, state)"
+    )
+    cg.add(hub.register_led_channel(config[CONF_PIN], initial_duty(config)))
+    _request_led_slot(str(config[CONF_SX1509_ID]))

esphome/components/sx1509/output/sx1509_float_output.cpp

@@ -1,31 +0,0 @@
-#include "sx1509_float_output.h"
-#include "esphome/core/helpers.h"
-#include "esphome/core/log.h"
-
-namespace esphome::sx1509 {
-
-ESPHOME_LOG_TAG(TAG, "sx1509_float_channel");
-
-void SX1509FloatOutputChannel::write_state(float state) {
-  const uint16_t max_duty = 255;
-  const float duty_rounded = roundf(state * max_duty);
-  auto duty = static_cast<uint16_t>(duty_rounded);
-  this->parent_->set_pin_value(this->pin_, duty);
-}
-
-void SX1509FloatOutputChannel::setup() {
-  ESP_LOGD(TAG, "Pin %d", this->pin_);
-  this->parent_->pin_mode(this->pin_, gpio::FLAG_OUTPUT);
-  this->parent_->setup_led_driver(this->pin_);
-  this->turn_off();
-}
-
-void SX1509FloatOutputChannel::dump_config() {
-  ESP_LOGCONFIG(TAG,
-                "SX1509 PWM:\n"
-                "  sx1509 pin: %d",
-                this->pin_);
-  LOG_FLOAT_OUTPUT(this);
-}
-
-}  // namespace esphome::sx1509

esphome/components/sx1509/output/sx1509_float_output.h

@@ -1,25 +0,0 @@
-#pragma once
-
-#include "esphome/components/sx1509/sx1509.h"
-#include "esphome/components/output/float_output.h"
-
-namespace esphome::sx1509 {
-
-class SX1509Component;
-
-class SX1509FloatOutputChannel final : public output::FloatOutput, public Component {
- public:
-  void set_parent(SX1509Component *parent) { this->parent_ = parent; }
-  void set_pin(uint8_t pin) { pin_ = pin; }
-  void setup() override;
-  void dump_config() override;
-  float get_setup_priority() const override { return setup_priority::HARDWARE; }
-
- protected:
-  void write_state(float state) override;
-
-  SX1509Component *parent_;
-  uint8_t pin_;
-};
-
-}  // namespace esphome::sx1509

esphome/components/sx1509/sx1509.cpp

@@ -27,6 +27,13 @@ void SX1509Component::setup() {
   delayMicroseconds(500);
   if (this->has_keypad_)
     this->setup_keypad_();
+#ifdef SX1509_LED_CHANNEL_COUNT
+  for (const auto &led : this->led_channels_) {
+    this->pin_mode(led.pin, gpio::FLAG_OUTPUT);
+    this->setup_led_driver(led.pin);
+    this->set_pin_value(led.pin, led.initial_duty);
+  }
+#endif
 }
 
 void SX1509Component::dump_config() {

esphome/components/sx1509/sx1509.h

@@ -5,6 +5,7 @@
 #include "esphome/components/gpio_expander/cached_gpio.h"
 #include "esphome/core/component.h"
 #include "esphome/core/hal.h"
+#include "esphome/core/helpers.h"
 #include "esphome/components/gpio_expander/expander_gpio_pin.h"
 #include "sx1509_registers.h"
 
@@ -61,6 +62,11 @@ class SX1509Component final : public Component,
   }
   void register_key_trigger(SX1509KeyTrigger *trig) { this->key_triggers_.push_back(trig); };
   void setup_led_driver(uint8_t pin);
+#ifdef SX1509_LED_CHANNEL_COUNT
+  /// setup() turns `pin` into an LED driver output and writes `initial_duty`, in registration order.
+  void register_led_channel(uint8_t pin, uint8_t initial_duty) { this->led_channels_.push_back({pin, initial_duty}); }
+  void write_led(uint8_t pin, float state) { this->set_pin_value(pin, static_cast<uint8_t>(roundf(state * 255))); }
+#endif
 
  protected:
   // Virtual methods from CachedGpioExpander — names come from base class
@@ -88,6 +94,14 @@ class SX1509Component final : public Component,
   uint32_t last_loop_timestamp_ = 0;
   const uint32_t min_loop_period_ = 15;  // ms
 
+#ifdef SX1509_LED_CHANNEL_COUNT
+  struct LedChannel {
+    uint8_t pin;
+    uint8_t initial_duty;
+  };
+  StaticVector<LedChannel, SX1509_LED_CHANNEL_COUNT> led_channels_;
+#endif
+
   void setup_keypad_();
   void set_debounce_config_(uint8_t config_value);
   void set_debounce_time_(uint8_t time);

esphome/core/defines.h

@@ -264,6 +264,7 @@
 #define SYSTA_BUS_LISTENER_COUNT 2
 #define SERIAL_PROXY_COUNT 2
 #define SNTP_SERVER_COUNT 3
+#define SX1509_LED_CHANNEL_COUNT 2
 #define TCA8418_LISTENER_COUNT 1
 #define USE_MEDIA_PLAYER
 #define USE_MEDIA_SOURCE

tests/component_tests/sx1509/config/outputs.yaml

@@ -0,0 +1,48 @@
+esphome:
+  name: test
+
+esp8266:
+  board: esp01_1m
+
+i2c:
+  sda: 4
+  scl: 5
+
+sx1509:
+  - id: sx_a
+    address: 0x3E
+  - id: sx_b
+    address: 0x3F
+
+output:
+  - platform: sx1509
+    id: plain
+    sx1509_id: sx_a
+    pin: 5
+  - platform: sx1509
+    id: inverted
+    sx1509_id: sx_a
+    pin: 3
+    inverted: true
+  - platform: sx1509
+    id: min_power
+    sx1509_id: sx_a
+    pin: 7
+    min_power: 10%
+  - platform: sx1509
+    id: inverted_min_power
+    sx1509_id: sx_a
+    pin: 8
+    inverted: true
+    min_power: 10%
+  - platform: sx1509
+    id: zero_means_zero
+    sx1509_id: sx_a
+    pin: 9
+    inverted: true
+    min_power: 10%
+    zero_means_zero: true
+  - platform: sx1509
+    id: other_hub
+    sx1509_id: sx_b
+    pin: 5

tests/component_tests/sx1509/test_outputs.py

@@ -0,0 +1,77 @@
+"""SX1509 LED outputs are Apply outputs the hub sets up with the duty turn_off() wrote."""
+
+from collections.abc import Callable
+from pathlib import Path
+
+import pytest
+
+from esphome import config_validation as cv
+from esphome.components.sx1509 import output as sx1509_output
+from esphome.config import Config
+from esphome.const import CONF_PIN, CONF_PLATFORM, PlatformFramework
+from esphome.core import CORE, ID
+
+from ..types import SetCoreConfigCallable
+
+
+def test_led_outputs_are_apply_outputs(
+    generate_main: Callable[[str | Path], str],
+    component_config_path: Callable[[str], Path],
+) -> None:
+    main_cpp = generate_main(component_config_path("outputs.yaml"))
+
+    assert "SX1509FloatOutputChannel" not in main_cpp
+    assert "sx_a->write_led(index, state)" in main_cpp
+    assert "sx_b->write_led(index, state)" in main_cpp
+    # Every channel of a hub shares one apply function, with the pin as its index
+    assert "new(plain) output::ApplyFloatOutput<esphome__plain__fn>(5);" in main_cpp
+    assert "new(inverted) output::ApplyFloatOutput<esphome__plain__fn>(3);" in main_cpp
+    assert "inverted->set_inverted(true);" in main_cpp
+    for call in (
+        "sx_a->register_led_channel(5, 0);",
+        "sx_a->register_led_channel(3, 255);",
+        "sx_a->register_led_channel(7, 26);",
+        "sx_a->register_led_channel(8, 230);",
+        "sx_a->register_led_channel(9, 255);",
+        "sx_b->register_led_channel(5, 0);",
+    ):
+        assert call in main_cpp
+    # The hub sets the channels up in config order, like the channel components did
+    assert main_cpp.index("register_led_channel(5, 0)") < main_cpp.index(
+        "register_led_channel(3, 255)"
+    )
+    defines = {d.name: d.value for d in CORE.defines}
+    assert str(defines["SX1509_LED_CHANNEL_COUNT"]) == "5"
+
+
+@pytest.mark.parametrize(
+    ("config", "duty"),
+    [
+        ({}, 0),
+        ({"inverted": True}, 255),
+        ({"min_power": 0.1}, 26),
+        ({"inverted": True, "min_power": 0.1}, 230),
+        ({"min_power": 0.1, "zero_means_zero": True}, 0),
+        ({"inverted": True, "min_power": 0.1, "zero_means_zero": True}, 255),
+        # set_min_power() clamps to max_power
+        ({"min_power": 0.5, "max_power": 0.2}, 51),
+        ({"inverted": True, "min_power": 0.002}, 254),
+    ],
+)
+def test_initial_duty(config: dict, duty: int) -> None:
+    """The duty FloatOutput::turn_off() wrote through the old channel's setup()."""
+    assert sx1509_output.initial_duty({"zero_means_zero": False, **config}) == duty
+
+
+def test_pin_must_be_unique(set_core_config: SetCoreConfigCallable) -> None:
+    entries = [
+        {CONF_PLATFORM: "sx1509", "sx1509_id": ID(hub), CONF_PIN: pin}
+        for hub, pin in (("a", 5), ("a", 6), ("b", 5), ("a", 5))
+    ]
+    full = Config()
+    full["output"] = entries
+    set_core_config(PlatformFramework.ESP8266_ARDUINO, full_config=full)
+    for entry in entries[1:3]:
+        sx1509_output.FINAL_VALIDATE_SCHEMA(entry)
+    with pytest.raises(cv.Invalid, match="Channel '5' of 'a' is already used"):
+        sx1509_output.FINAL_VALIDATE_SCHEMA(entries[3])

tests/components/sx1509/__init__.py

@@ -0,0 +1,15 @@
+import esphome.codegen as cg
+from esphome.types import ConfigType
+from tests.testing_helpers import ComponentManifestOverride
+
+
+def override_manifest(manifest: ComponentManifestOverride) -> None:
+    # One empty config entry so to_code runs and compiles the LED channel path.
+    manifest.multi_conf = False
+    manifest.dependencies = manifest.dependencies + ["output"]
+
+    async def to_code(config: ConfigType) -> None:
+        cg.add_define("SX1509_LED_CHANNEL_COUNT", 3)
+        cg.add_define("USE_OUTPUT_FLOAT_POWER_SCALING")
+
+    manifest.to_code = to_code

tests/components/sx1509/common.yaml

@@ -56,3 +56,13 @@ output:
     id: sx1509_output
     sx1509_id: sx1509_hub
     pin: 5
+  - platform: sx1509
+    id: sx1509_output_inverted
+    sx1509_id: sx1509_hub
+    pin: 6
+    inverted: true
+    min_power: 10%
+  - platform: sx1509
+    id: sx1509_output_second_hub
+    sx1509_id: sx1509_hub_second
+    pin: 5

tests/components/sx1509/led_output_test.cpp

@@ -0,0 +1,108 @@
+#include <gtest/gtest.h>
+
+#include <array>
+
+#include "esphome/components/output/float_output.h"
+#include "esphome/components/sx1509/sx1509.h"
+
+namespace esphome::sx1509::testing {
+
+/// Register file with the chip's auto-increment; the setup check and LED intensities hold their reset values.
+class RegisterBus final : public i2c::I2CBus {
+ public:
+  RegisterBus() {
+    this->registers[REG_INTERRUPT_MASK_A] = 0xFF;
+    for (uint8_t reg : REG_I_ON)
+      this->registers[reg] = 0xFF;
+  }
+
+  std::array<uint8_t, 256> registers{};
+
+  i2c::ErrorCode write_readv(uint8_t address, const uint8_t *data, size_t size, uint8_t *out,
+                             size_t out_size) override {
+    uint8_t reg = size != 0 ? data[0] : 0;
+    for (size_t i = 1; i < size; i++)
+      this->registers[reg++] = data[i];
+    for (size_t i = 0; i < out_size; i++)
+      out[i] = this->registers[reg++];
+    return i2c::ERROR_OK;
+  }
+
+  uint16_t word(uint8_t reg_b) const { return (this->registers[reg_b] << 8) | this->registers[reg_b + 1]; }
+};
+
+static SX1509Component *hub = nullptr;  // NOLINT(cppcoreguidelines-avoid-non-const-global-variables)
+
+static void write_led(output::FloatOutput *entity, uint16_t index, float state) { hub->write_led(index, state); }
+
+class SX1509LedTest : public ::testing::Test {
+ protected:
+  RegisterBus bus_;
+  SX1509Component chip_;
+
+  void SetUp() override {
+    this->chip_.set_i2c_bus(&this->bus_);
+    hub = &this->chip_;
+  }
+};
+
+TEST_F(SX1509LedTest, SetupDrivesEachRegisteredChannelWithItsBootDuty) {
+  this->chip_.register_led_channel(5, 0);
+  this->chip_.register_led_channel(3, 255);
+  this->chip_.register_led_channel(8, 230);
+  this->chip_.setup();
+
+  ASSERT_FALSE(this->chip_.is_failed());
+  EXPECT_EQ(this->bus_.registers[REG_I_ON[5]], 0);
+  EXPECT_EQ(this->bus_.registers[REG_I_ON[3]], 255);
+  EXPECT_EQ(this->bus_.registers[REG_I_ON[8]], 230);
+  EXPECT_EQ(this->bus_.registers[REG_I_ON[4]], 0xFF);  // not an LED channel
+  constexpr uint16_t led_pins = (1 << 3) | (1 << 5) | (1 << 8);
+  EXPECT_EQ(this->bus_.word(REG_LED_DRIVER_ENABLE_B), led_pins);
+  EXPECT_EQ(this->bus_.word(REG_DIR_B) & led_pins, 0);
+  EXPECT_EQ(this->bus_.word(REG_DATA_B) & led_pins, 0);
+}
+
+// The duty FloatOutput::turn_off() writes must match what codegen registers (sx1509/output initial_duty)
+TEST_F(SX1509LedTest, TurnOffWritesTheDutyCodegenRegisters) {
+  struct Case {
+    bool inverted;
+    float min_power;
+    float max_power;
+    bool zero_means_zero;
+    uint8_t duty;
+  };
+  static constexpr std::array<Case, 8> CASES{{
+      {false, 0.0f, 1.0f, false, 0},
+      {true, 0.0f, 1.0f, false, 255},
+      {false, 0.1f, 1.0f, false, 26},
+      {true, 0.1f, 1.0f, false, 230},
+      {false, 0.1f, 1.0f, true, 0},
+      {true, 0.1f, 1.0f, true, 255},
+      {false, 0.5f, 0.2f, false, 51},
+      {true, 0.002f, 1.0f, false, 254},
+  }};
+  for (const auto &c : CASES) {
+    output::ApplyFloatOutput<write_led> out(7);
+    out.set_inverted(c.inverted);
+    out.set_max_power(c.max_power);
+    out.set_min_power(c.min_power);
+    out.set_zero_means_zero(c.zero_means_zero);
+    this->bus_.registers[REG_I_ON[7]] = 0x42;
+    out.turn_off();
+    EXPECT_EQ(this->bus_.registers[REG_I_ON[7]], c.duty) << "inverted " << c.inverted << " min " << c.min_power;
+  }
+}
+
+TEST_F(SX1509LedTest, LevelsRoundToTheNearestDuty) {
+  output::ApplyFloatOutput<write_led> out(2);
+  out.set_level(0.5f);
+  EXPECT_EQ(this->bus_.registers[REG_I_ON[2]], 128);
+  out.set_level(1.0f);
+  EXPECT_EQ(this->bus_.registers[REG_I_ON[2]], 255);
+  out.set_inverted(true);
+  out.set_level(0.25f);
+  EXPECT_EQ(this->bus_.registers[REG_I_ON[2]], 191);
+}
+
+}  // namespace esphome::sx1509::testing