From 45d565db01b84bb39522e859698036b3f4ee58ef Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 29 Oct 2026 18:19:01 -1000 Subject: [PATCH] [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 | 4 +- esphome/components/sx1509/output/__init__.py | 56 ++++++--- .../sx1509/output/sx1509_float_output.cpp | 31 ----- .../sx1509/output/sx1509_float_output.h | 25 ---- esphome/components/sx1509/sx1509.cpp | 7 ++ esphome/components/sx1509/sx1509.h | 14 +++ esphome/core/defines.h | 1 + .../sx1509/config/outputs.yaml | 48 ++++++++ tests/component_tests/sx1509/test_outputs.py | 77 +++++++++++++ tests/components/sx1509/__init__.py | 15 +++ tests/components/sx1509/common.yaml | 10 ++ tests/components/sx1509/led_output_test.cpp | 108 ++++++++++++++++++ 12 files changed, 324 insertions(+), 72 deletions(-) delete mode 100644 esphome/components/sx1509/output/sx1509_float_output.cpp delete mode 100644 esphome/components/sx1509/output/sx1509_float_output.h create mode 100644 tests/component_tests/sx1509/config/outputs.yaml create mode 100644 tests/component_tests/sx1509/test_outputs.py create mode 100644 tests/components/sx1509/__init__.py create mode 100644 tests/components/sx1509/led_output_test.cpp diff --git a/esphome/components/output/__init__.py b/esphome/components/output/__init__.py index 442b573bc05..7b173ef7e40 100644 --- a/esphome/components/output/__init__.py +++ b/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", ) diff --git a/esphome/components/sx1509/output/__init__.py b/esphome/components/sx1509/output/__init__.py index aed5ab7dd47..c6bfa04c395 100644 --- a/esphome/components/sx1509/output/__init__.py +++ b/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])) diff --git a/esphome/components/sx1509/output/sx1509_float_output.cpp b/esphome/components/sx1509/output/sx1509_float_output.cpp deleted file mode 100644 index 5bf47e97bde..00000000000 --- a/esphome/components/sx1509/output/sx1509_float_output.cpp +++ /dev/null @@ -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(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 diff --git a/esphome/components/sx1509/output/sx1509_float_output.h b/esphome/components/sx1509/output/sx1509_float_output.h deleted file mode 100644 index 8790b2fcd75..00000000000 --- a/esphome/components/sx1509/output/sx1509_float_output.h +++ /dev/null @@ -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 diff --git a/esphome/components/sx1509/sx1509.cpp b/esphome/components/sx1509/sx1509.cpp index f5b7c0b0bbe..c227b71b30b 100644 --- a/esphome/components/sx1509/sx1509.cpp +++ b/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() { diff --git a/esphome/components/sx1509/sx1509.h b/esphome/components/sx1509/sx1509.h index 8de5559f01e..9e56c5af19c 100644 --- a/esphome/components/sx1509/sx1509.h +++ b/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(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 led_channels_; +#endif + void setup_keypad_(); void set_debounce_config_(uint8_t config_value); void set_debounce_time_(uint8_t time); diff --git a/esphome/core/defines.h b/esphome/core/defines.h index ecd54323bb2..a9bc9af0466 100644 --- a/esphome/core/defines.h +++ b/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 diff --git a/tests/component_tests/sx1509/config/outputs.yaml b/tests/component_tests/sx1509/config/outputs.yaml new file mode 100644 index 00000000000..06ebd257f9e --- /dev/null +++ b/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 diff --git a/tests/component_tests/sx1509/test_outputs.py b/tests/component_tests/sx1509/test_outputs.py new file mode 100644 index 00000000000..f2ecc90fa2b --- /dev/null +++ b/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(5);" in main_cpp + assert "new(inverted) output::ApplyFloatOutput(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]) diff --git a/tests/components/sx1509/__init__.py b/tests/components/sx1509/__init__.py new file mode 100644 index 00000000000..584ec13fd92 --- /dev/null +++ b/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 diff --git a/tests/components/sx1509/common.yaml b/tests/components/sx1509/common.yaml index 0299b7acd5d..f99db1d6ef7 100644 --- a/tests/components/sx1509/common.yaml +++ b/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 diff --git a/tests/components/sx1509/led_output_test.cpp b/tests/components/sx1509/led_output_test.cpp new file mode 100644 index 00000000000..be6d6213bdd --- /dev/null +++ b/tests/components/sx1509/led_output_test.cpp @@ -0,0 +1,108 @@ +#include + +#include + +#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 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 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 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 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