From e21c21f928d7530c71f4b2d64785c334e162740d Mon Sep 17 00:00:00 2001 From: Tobias Stanzel Date: Sat, 26 Sep 2026 15:56:17 +0200 Subject: [PATCH 1/4] test(levoit): add compile tests + CI, and fix the build failures they found Adds three self-contained configs under components/levoit/tests that build the component without a secrets.yaml, plus a GitHub Actions workflow running them on every PR touching components/. The tests failed immediately on two real bugs, both now fixed: - A levoit config with only a fan did not build. core_commands.cpp, sprout_commands.cpp, superior_commands.cpp and superior_status.cpp included number/levoit_number.h and sensor/levoit_sensor.h unguarded, and core_status.cpp / vital_status.cpp included fan/levoit_fan.h the same way. ESPHome only copies levoit// into the build when a levoit entity of that platform is configured, so those headers were simply absent. - levoit.cpp guarded its includes on the generic USE_SWITCH / USE_NUMBER / ... macros, which ANY component defines. A config with a template switch but no levoit switch took the include and failed the same way - the same flaw already fixed in the philips component. Each platform now defines its own USE_LEVOIT_* macro. Rather than scatter guards through switch-cases, the entity classes are confined to levoit.cpp behind new accessors - try_get_number_state(), try_get_select_index(), try_get_switch_state() and apply_fan_status() - so the *_commands.cpp and *_status.cpp sources no longer reference those types at all. Each accessor returns false when the entity is absent or has no state, preserving the existing "leave the default alone" behaviour at every call site. The 1.5.0 changelog entry claimed the USE_LEVOIT_* guards were already in place. They were not - that was only ever done for philips - so the entry is corrected here. Verified: all three test configs compile, and Core 200S/400S, Vital 200S and Sprout device configs still build. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/compile-tests.yml | 66 ++++ components/levoit/binary_sensor/__init__.py | 3 + components/levoit/button/__init__.py | 3 + components/levoit/core_commands.cpp | 13 +- components/levoit/core_status.cpp | 26 +- components/levoit/fan/__init__.py | 3 + components/levoit/levoit.cpp | 98 +++++- components/levoit/levoit.h | 10 + components/levoit/light/__init__.py | 3 + components/levoit/number/__init__.py | 3 + components/levoit/select/__init__.py | 3 + components/levoit/sensor/__init__.py | 3 + components/levoit/sprout_commands.cpp | 27 +- components/levoit/superior_commands.cpp | 18 +- components/levoit/superior_status.cpp | 15 +- components/levoit/switch/__init__.py | 3 + components/levoit/tests/.gitignore | 5 + components/levoit/tests/README.md | 40 +++ .../levoit/tests/test-all-entities.yaml | 320 ++++++++++++++++++ components/levoit/tests/test-fan-only.yaml | 42 +++ .../levoit/tests/test-foreign-platforms.yaml | 81 +++++ components/levoit/text_sensor/__init__.py | 3 + components/levoit/vital_commands.cpp | 60 +--- components/levoit/vital_status.cpp | 10 +- 24 files changed, 732 insertions(+), 126 deletions(-) create mode 100644 .github/workflows/compile-tests.yml create mode 100644 components/levoit/tests/.gitignore create mode 100644 components/levoit/tests/README.md create mode 100644 components/levoit/tests/test-all-entities.yaml create mode 100644 components/levoit/tests/test-fan-only.yaml create mode 100644 components/levoit/tests/test-foreign-platforms.yaml diff --git a/.github/workflows/compile-tests.yml b/.github/workflows/compile-tests.yml new file mode 100644 index 0000000..44b3b5c --- /dev/null +++ b/.github/workflows/compile-tests.yml @@ -0,0 +1,66 @@ +name: Compile tests + +# Runs on every PR, and on pushes to main so the badge reflects the branch +# everyone builds from. Paths are filtered so doc-only changes skip the build. +on: + pull_request: + paths: + - 'components/**' + - '.github/workflows/compile-tests.yml' + push: + branches: [main] + paths: + - 'components/**' + - '.github/workflows/compile-tests.yml' + workflow_dispatch: + +# A new push to the same PR cancels the previous run. +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + levoit: + name: levoit · ${{ matrix.config }} + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + config: + # every platform and every entity type - maximum codegen coverage + - test-all-entities.yaml + # only a fan: every optional platform absent. Catches sources that + # include a platform header without guarding on USE_LEVOIT_*. + - test-fan-only.yaml + # levoit fan plus template entities on the other platforms. Defines the + # generic USE_SWITCH / USE_SELECT / ... without any levoit entity of + # that kind, which is the mismatch that broke the build before. + - test-foreign-platforms.yaml + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-python@v5 + with: + python-version: '3.12' + + # PlatformIO toolchains and the ESP-IDF sources are the slow part, not the + # compile itself, so cache them across runs. + - name: Cache PlatformIO + uses: actions/cache@v4 + with: + path: | + ~/.platformio + ~/.cache/pip + key: pio-${{ runner.os }}-${{ hashFiles('components/levoit/tests/*.yaml') }} + restore-keys: pio-${{ runner.os }}- + + - name: Install ESPHome + run: pip install esphome + + - name: esphome config + working-directory: components/levoit/tests + run: esphome config ${{ matrix.config }} + + - name: esphome compile + working-directory: components/levoit/tests + run: esphome compile ${{ matrix.config }} diff --git a/components/levoit/binary_sensor/__init__.py b/components/levoit/binary_sensor/__init__.py index f00961e..a934276 100644 --- a/components/levoit/binary_sensor/__init__.py +++ b/components/levoit/binary_sensor/__init__.py @@ -46,6 +46,9 @@ ) async def to_code(config): + # Tells the levoit sources this platform is in the build - see the note + # at the top of levoit.cpp on why USE_BINARY_SENSOR is not sufficient. + cg.add_define("USE_LEVOIT_BINARY_SENSOR") parent = await cg.get_variable(config[CONF_LEVOIT_ID]) var = cg.new_Pvariable(config[CONF_ID]) diff --git a/components/levoit/button/__init__.py b/components/levoit/button/__init__.py index e4008ec..7ead241 100644 --- a/components/levoit/button/__init__.py +++ b/components/levoit/button/__init__.py @@ -29,6 +29,9 @@ ) async def to_code(config): + # Tells the levoit sources this platform is in the build - see the note + # at the top of levoit.cpp on why USE_BUTTON is not sufficient. + cg.add_define("USE_LEVOIT_BUTTON") parent = await cg.get_variable(config[CONF_LEVOIT_ID]) var = cg.new_Pvariable(config[CONF_ID]) diff --git a/components/levoit/core_commands.cpp b/components/levoit/core_commands.cpp index 4162346..c727bc9 100644 --- a/components/levoit/core_commands.cpp +++ b/components/levoit/core_commands.cpp @@ -1,8 +1,8 @@ #include "core_commands.h" #include "levoit_message.h" #include "levoit.h" -#include "number/levoit_number.h" #include "decoder_helpers.h" +#include "esphome/core/defines.h" #include "esphome/core/log.h" namespace esphome @@ -120,10 +120,9 @@ namespace esphome case CommandType::setAutoModeEfficient: { msg_type = {0x01, 0xE6, 0xA5}; - auto *num = self->get_number(NumberType::EFFICIENCY_ROOM_SIZE); - if (num != nullptr) + float m2; + if (self->try_get_number_state(NumberType::EFFICIENCY_ROOM_SIZE, m2)) { - float m2 = num->state; uint32_t raw = encode_core_room_size_raw(m2); uint8_t size_low = raw & 0xFF; uint8_t size_high = (raw >> 8) & 0xFF; @@ -149,10 +148,10 @@ namespace esphome case CommandType::setTimerMinutes: { msg_type = {0x01, 0x64, 0xA2}; - auto *num = self->get_number(NumberType::TIMER); - if (num != nullptr) + float mins; + if (self->try_get_number_state(NumberType::TIMER, mins)) { - uint32_t secs = static_cast(num->state) * 60; + uint32_t secs = static_cast(mins) * 60; uint8_t b0 = (secs >> 0) & 0xFF; uint8_t b1 = (secs >> 8) & 0xFF; uint8_t b2 = (secs >> 16) & 0xFF; diff --git a/components/levoit/core_status.cpp b/components/levoit/core_status.cpp index 2f8eba8..241f76a 100644 --- a/components/levoit/core_status.cpp +++ b/components/levoit/core_status.cpp @@ -1,8 +1,8 @@ #include "core_status.h" +#include "esphome/core/defines.h" #include "esphome/core/log.h" #include "tlv.h" // <- TLV extraction #include "types.h" -#include "fan/levoit_fan.h" #include "decoder_helpers.h" #include #include @@ -106,12 +106,8 @@ namespace esphome self->publish_switch(SwitchType::CHILD_LOCK, child_lock); self->publish_select(SelectType::NIGHTLIGHT, nightlight_idx); - auto *fan = self->get_fan(); - if (fan != nullptr) - { - ESP_LOGV(TAG_CORE, "Core200S fan: power=%d speed=%d mode=%d", power, fan_speed, (int)fan_mode); - fan->apply_device_status(power, fan_speed, fan_mode); - } + ESP_LOGV(TAG_CORE, "Core200S fan: power=%d speed=%d mode=%d", power, fan_speed, (int)fan_mode); + self->apply_fan_status(power, fan_speed, fan_mode); return; } @@ -144,12 +140,8 @@ namespace esphome self->publish_select(SelectType::AUTO_MODE, fan_auto_mode); self->publish_number(NumberType::EFFICIENCY_ROOM_SIZE, efficency_area_m2); - auto *fan = self->get_fan(); - if (fan != nullptr) - { - ESP_LOGV(TAG_CORE, "Core600S fan: power=%d speed=%d mode=%d", power, fan_speed, (int)fan_mode); - fan->apply_device_status(power, fan_speed, fan_mode); - } + ESP_LOGV(TAG_CORE, "Core600S fan: power=%d speed=%d mode=%d", power, fan_speed, (int)fan_mode); + self->apply_fan_status(power, fan_speed, fan_mode); return; } @@ -189,12 +181,8 @@ namespace esphome self->publish_select(SelectType::AUTO_MODE, fan_auto_mode); self->publish_number(NumberType::EFFICIENCY_ROOM_SIZE, efficency_area_m2); - auto *fan = self->get_fan(); - if (fan != nullptr) - { - ESP_LOGV(TAG_CORE, "Applying to fan: power=%d speed=%d mode=%d", power, fan_speed, (int)fan_mode); - fan->apply_device_status(power, fan_speed, fan_mode); - } + ESP_LOGV(TAG_CORE, "Applying to fan: power=%d speed=%d mode=%d", power, fan_speed, (int)fan_mode); + self->apply_fan_status(power, fan_speed, fan_mode); } } // namespace levoit diff --git a/components/levoit/fan/__init__.py b/components/levoit/fan/__init__.py index 74d33d4..699d32c 100644 --- a/components/levoit/fan/__init__.py +++ b/components/levoit/fan/__init__.py @@ -19,6 +19,9 @@ async def to_code(config): + # Tells the levoit sources this platform is in the build - see the note + # at the top of levoit.cpp on why USE_FAN is not sufficient. + cg.add_define("USE_LEVOIT_FAN") var = await fan.new_fan(config) await cg.register_component(var, config) diff --git a/components/levoit/levoit.cpp b/components/levoit/levoit.cpp index d6e9436..35c0347 100644 --- a/components/levoit/levoit.cpp +++ b/components/levoit/levoit.cpp @@ -24,23 +24,25 @@ #include "superior_commands.h" #include "decoder_helpers.h" // format_duration_minutes #include "sprout_status.h" -#ifdef USE_LIGHT +#ifdef USE_LEVOIT_LIGHT #include "light/levoit_light.h" #endif -#ifdef USE_SWITCH +#ifdef USE_LEVOIT_SWITCH #include "switch/levoit_switch.h" #endif +#ifdef USE_LEVOIT_FAN #include "fan/levoit_fan.h" -#ifdef USE_NUMBER +#endif +#ifdef USE_LEVOIT_NUMBER #include "number/levoit_number.h" #endif -#ifdef USE_SENSOR +#ifdef USE_LEVOIT_SENSOR #include "sensor/levoit_sensor.h" #endif -#ifdef USE_SELECT +#ifdef USE_LEVOIT_SELECT #include "select/levoit_select.h" #endif -#ifdef USE_TEXT_SENSOR +#ifdef USE_LEVOIT_TEXT_SENSOR #include "text_sensor/levoit_text_sensor.h" #endif @@ -123,7 +125,7 @@ namespace esphome { if (type == SwitchType::DISPLAY) display_on_ = state; -#ifdef USE_SWITCH +#ifdef USE_LEVOIT_SWITCH auto *sw = switches_[st_idx_(type)]; if (!sw) return; @@ -142,7 +144,7 @@ namespace esphome void Levoit::publish_sensor(SensorType type, float value) { -#ifdef USE_SENSOR +#ifdef USE_LEVOIT_SENSOR auto *se = sensors_[st_idx_(type)]; if (!se) return; @@ -154,7 +156,7 @@ namespace esphome void Levoit::publish_number(NumberType type, float value) { -#ifdef USE_NUMBER +#ifdef USE_LEVOIT_NUMBER auto *nm = numbers_[nt_idx_(type)]; if (!nm) return; @@ -165,7 +167,7 @@ namespace esphome } void Levoit::publish_select(SelectType type, uint32_t value) { -#ifdef USE_SELECT +#ifdef USE_LEVOIT_SELECT auto *sl = selects_[sl_idx_(type)]; if (!sl) return; @@ -202,7 +204,7 @@ namespace esphome } void Levoit::publish_text_sensor(TextSensorType type, const std::string &value) { -#ifdef USE_TEXT_SENSOR +#ifdef USE_LEVOIT_TEXT_SENSOR auto *tsl = text_sensor_[static_cast(type)]; if (!tsl) return; @@ -256,7 +258,7 @@ namespace esphome bulk_prefs_.dt_enabled, bulk_prefs_.dt_mode, bulk_prefs_.dt_level); } } -#ifdef USE_LIGHT +#ifdef USE_LEVOIT_LIGHT void Levoit::publish_sprout_light(bool on, float brightness, float color_temp, bool breathing) { if (sprout_light_ != nullptr) @@ -292,6 +294,67 @@ namespace esphome // caller goes through here so the MCU-owned guard cannot be forgotten in one // of them - which is exactly what happened when the periodic CADR path kept // overwriting the Core200S value once a minute. + + bool Levoit::try_get_number_state(NumberType type, float &out) const + { +#ifdef USE_LEVOIT_NUMBER + auto *n = this->numbers_[nt_idx_(type)]; + if (n == nullptr || !n->has_state()) + return false; + out = n->state; + return true; +#else + (void) type; + (void) out; + return false; +#endif + } + + bool Levoit::try_get_select_index(SelectType type, size_t &out) const + { +#ifdef USE_LEVOIT_SELECT + auto *sel = this->selects_[sl_idx_(type)]; + if (sel == nullptr || !sel->has_state()) + return false; + auto idx = sel->active_index(); + if (!idx.has_value()) + return false; + out = idx.value(); + return true; +#else + (void) type; + (void) out; + return false; +#endif + } + + bool Levoit::try_get_switch_state(SwitchType type, bool &out) const + { +#ifdef USE_LEVOIT_SWITCH + auto *sw = this->switches_[st_idx_(type)]; + if (sw == nullptr || !sw->has_state()) + return false; + out = sw->state; + return true; +#else + (void) type; + (void) out; + return false; +#endif + } + + void Levoit::apply_fan_status(bool power, uint8_t speed, uint32_t mode) + { +#ifdef USE_LEVOIT_FAN + if (this->fan_ != nullptr) + this->fan_->apply_device_status(power, speed, mode); +#else + (void) power; + (void) speed; + (void) mode; +#endif + } + void Levoit::publish_filter_stats_now() { // On models where the MCU reports filter life, leave the sensor alone - @@ -300,7 +363,7 @@ namespace esphome if (this->filter_life_from_mcu()) return; float filter_left = this->calculate_filter_life_left_percent(); -#ifdef USE_SENSOR +#ifdef USE_LEVOIT_SENSOR auto *se = this->sensors_[st_idx_(SensorType::FILTER_LIFE_LEFT)]; if (se != nullptr) se->publish_state(filter_left); @@ -521,10 +584,9 @@ namespace esphome // Inject the remembered room size target so the command // builder sends the right value, not the last MCU-echoed one. { - auto *input = this->get_number(NumberType::AUTO_PROFILE_ROOM_SIZE_INPUT); - auto *eff = this->get_number(NumberType::EFFICIENCY_ROOM_SIZE); - if (input != nullptr && eff != nullptr && input->has_state()) - eff->publish_state(input->state); + float room_size; + if (this->try_get_number_state(NumberType::AUTO_PROFILE_ROOM_SIZE_INPUT, room_size)) + this->publish_number(NumberType::EFFICIENCY_ROOM_SIZE, room_size); } this->sendCommand(setAutoModeEfficient); sent_auto_profile = true; @@ -847,7 +909,7 @@ namespace esphome float Levoit::calculate_filter_life_left_percent() const { -#ifndef USE_NUMBER +#ifndef USE_LEVOIT_NUMBER return 100.0f; #else auto *filter_lifetime_num = this->get_number(NumberType::FILTER_LIFETIME_MONTHS); diff --git a/components/levoit/levoit.h b/components/levoit/levoit.h index a3cc7dd..aac287e 100644 --- a/components/levoit/levoit.h +++ b/components/levoit/levoit.h @@ -103,6 +103,16 @@ class Levoit : public Component, public uart::UARTDevice { LevoitSwitch *get_switch(SwitchType type) const { return switches_[st_idx_(type)]; } class LevoitBinarySensor *get_binary_sensor(BinarySensorType type) const { return binary_sensors_[bs_idx_(type)]; } bool get_binary_sensor_state(BinarySensorType type) const { return binary_sensor_states_[bs_idx_(type)]; } + // Value accessors that keep the platform entity classes confined to + // levoit.cpp. The *_commands.cpp / *_status.cpp sources must not include + // number/levoit_number.h and friends: those directories only exist in the + // build when the matching levoit platform is configured. Each returns false + // when the entity is absent or has no state yet, leaving `out` untouched. + bool try_get_number_state(NumberType type, float &out) const; + bool try_get_select_index(SelectType type, size_t &out) const; + bool try_get_switch_state(SwitchType type, bool &out) const; + void apply_fan_status(bool power, uint8_t speed, uint32_t mode); + void start_timer(){this->timer_active_ = true; this->timer_stop_pending_ = false;}; void stop_timer(){this->timer_active_ = false;}; bool is_timer_active() const { return this->timer_active_; }; diff --git a/components/levoit/light/__init__.py b/components/levoit/light/__init__.py index 771078b..48b61a1 100644 --- a/components/levoit/light/__init__.py +++ b/components/levoit/light/__init__.py @@ -16,6 +16,9 @@ async def to_code(config): + # Tells the levoit sources this platform is in the build - see the note + # at the top of levoit.cpp on why USE_LIGHT is not sufficient. + cg.add_define("USE_LEVOIT_LIGHT") parent = await cg.get_variable(config[CONF_LEVOIT_ID]) var = cg.new_Pvariable(config[CONF_OUTPUT_ID]) await light.register_light(var, config) diff --git a/components/levoit/number/__init__.py b/components/levoit/number/__init__.py index 6e32ba4..d3d2a9c 100644 --- a/components/levoit/number/__init__.py +++ b/components/levoit/number/__init__.py @@ -129,6 +129,9 @@ ) async def to_code(config): + # Tells the levoit sources this platform is in the build - see the note + # at the top of levoit.cpp on why USE_NUMBER is not sufficient. + cg.add_define("USE_LEVOIT_NUMBER") parent = await cg.get_variable(config[CONF_LEVOIT_ID]) ntype = config[CONF_TYPE] diff --git a/components/levoit/select/__init__.py b/components/levoit/select/__init__.py index 575ed4d..4f56345 100644 --- a/components/levoit/select/__init__.py +++ b/components/levoit/select/__init__.py @@ -40,6 +40,9 @@ ) async def to_code(config): + # Tells the levoit sources this platform is in the build - see the note + # at the top of levoit.cpp on why USE_SELECT is not sufficient. + cg.add_define("USE_LEVOIT_SELECT") parent = await cg.get_variable(config[CONF_LEVOIT_ID]) var = cg.new_Pvariable(config[CONF_ID]) ntype = config[CONF_TYPE] diff --git a/components/levoit/sensor/__init__.py b/components/levoit/sensor/__init__.py index c7520cb..6defcb1 100644 --- a/components/levoit/sensor/__init__.py +++ b/components/levoit/sensor/__init__.py @@ -95,6 +95,9 @@ ) async def to_code(config): + # Tells the levoit sources this platform is in the build - see the note + # at the top of levoit.cpp on why USE_SENSOR is not sufficient. + cg.add_define("USE_LEVOIT_SENSOR") parent = await cg.get_variable(config[CONF_LEVOIT_ID]) var = cg.new_Pvariable(config[CONF_ID]) diff --git a/components/levoit/sprout_commands.cpp b/components/levoit/sprout_commands.cpp index 4ec4a30..b4f7193 100644 --- a/components/levoit/sprout_commands.cpp +++ b/components/levoit/sprout_commands.cpp @@ -1,8 +1,7 @@ #include "sprout_commands.h" #include "levoit_message.h" #include "levoit.h" -#include "number/levoit_number.h" -#include "select/levoit_select.h" +#include "esphome/core/defines.h" #include "esphome/core/log.h" #include @@ -88,10 +87,11 @@ namespace esphome { uint8_t max_pct = self->get_pending_led_bri(); uint16_t ct_k = self->get_pending_led_ct(); - auto *n_min = self->get_number(NumberType::LED_BRIGHTNESS_MIN); - auto *n_spd = self->get_number(NumberType::LED_SPEED); - uint8_t min_pct = (n_min != nullptr) ? static_cast(n_min->state) : 5; - uint8_t speed = (n_spd != nullptr) ? static_cast(n_spd->state) : 5; + float v_min, v_spd; + uint8_t min_pct = self->try_get_number_state(NumberType::LED_BRIGHTNESS_MIN, v_min) + ? static_cast(v_min) : 5; + uint8_t speed = self->try_get_number_state(NumberType::LED_SPEED, v_spd) + ? static_cast(v_spd) : 5; // Clamp speed to minimum 1 if (speed < 1) speed = 1; // If min >= max, pull min down to max-20 (floor 0) @@ -105,18 +105,15 @@ namespace esphome case CommandType::setSproutWhiteNoiseOff: { bool active = (cmd == CommandType::setSproutWhiteNoiseOn); - auto *n_vol = self->get_number(NumberType::WHITE_NOISE_VOLUME); - uint8_t volume = (n_vol != nullptr) ? static_cast(n_vol->state) : 128; + float v_vol; + uint8_t volume = self->try_get_number_state(NumberType::WHITE_NOISE_VOLUME, v_vol) + ? static_cast(v_vol) : 128; // Get current sound index from select state uint8_t sound_index = 0; - auto *s_sound = self->get_select(SelectType::WHITE_NOISE_SOUND); - if (s_sound != nullptr && s_sound->has_state()) { - const auto &opts = s_sound->traits.get_options(); - auto it = std::find(opts.begin(), opts.end(), s_sound->current_option()); - if (it != opts.end()) - sound_index = static_cast(it - opts.begin()); - } + size_t sel_idx; + if (self->try_get_select_index(SelectType::WHITE_NOISE_SOUND, sel_idx)) + sound_index = static_cast(sel_idx); ESP_LOGD(TAG_SPROUT_CMD, "%s: vol=%u sound=%u", active ? "setSproutWhiteNoiseOn" : "setSproutWhiteNoiseOff", diff --git a/components/levoit/superior_commands.cpp b/components/levoit/superior_commands.cpp index d696791..2baeec7 100644 --- a/components/levoit/superior_commands.cpp +++ b/components/levoit/superior_commands.cpp @@ -1,7 +1,7 @@ #include "superior_commands.h" #include "levoit_message.h" #include "levoit.h" -#include "number/levoit_number.h" +#include "esphome/core/defines.h" #include "esphome/core/log.h" namespace esphome @@ -143,10 +143,12 @@ namespace esphome case CommandType::setHumidityTarget: { msg_type = {0x02, 0x36, 0x55}; - auto *num = self->get_number(NumberType::HUMIDITY_TARGET); - if (num != nullptr) + float humidity_target; + const bool have_humidity_target = + self->try_get_number_state(NumberType::HUMIDITY_TARGET, humidity_target); + if (have_humidity_target) { - uint8_t target = static_cast(num->state); + uint8_t target = static_cast(humidity_target); ESP_LOGD(TAG_SUP_CMD, "setHumidityTarget: %u%%", target); payload = {0x01, 0x01, target}; } @@ -161,10 +163,12 @@ namespace esphome case CommandType::setTimerMinutes: { msg_type = {0x02, 0x19, 0x50}; - auto *num = self->get_number(NumberType::TIMER); - if (num != nullptr) + float timer_hours; + const bool have_timer = + self->try_get_number_state(NumberType::TIMER, timer_hours); + if (have_timer) { - uint32_t secs = static_cast(num->state * 3600); + uint32_t secs = static_cast(timer_hours * 3600); uint8_t b0 = (secs >> 0) & 0xFF; uint8_t b1 = (secs >> 8) & 0xFF; uint8_t b2 = (secs >> 16) & 0xFF; diff --git a/components/levoit/superior_status.cpp b/components/levoit/superior_status.cpp index 70ecbb5..e8c8e1a 100644 --- a/components/levoit/superior_status.cpp +++ b/components/levoit/superior_status.cpp @@ -1,9 +1,8 @@ #include "superior_status.h" +#include "esphome/core/defines.h" #include "esphome/core/log.h" #include "tlv.h" #include "types.h" -#include "fan/levoit_fan.h" -#include "sensor/levoit_sensor.h" #include "decoder_helpers.h" #include #include @@ -193,9 +192,7 @@ namespace esphome float temp_c = temp_x10 / 10.0f; ESP_LOGV(TAG_SUP, "TemperaturePrecise=%.1f°C", temp_c); // Publish as integer (x10) to sensor, or as float - auto *se = self->get_sensor(SensorType::TEMPERATURE); - if (se != nullptr) - se->publish_state(temp_c); + self->publish_sensor(SensorType::TEMPERATURE, temp_c); } break; } @@ -227,10 +224,7 @@ namespace esphome } // Apply fan state - auto *fan = self->get_fan(); - if (fan != nullptr) - { - int pwr = have_power ? (power ? 1 : 0) : -1; + int pwr = have_power ? (power ? 1 : 0) : -1; int spd = have_speed ? (int)speed : -1; // Map MCU mode values (1-based) to internal MODE_MAP values (0-based) // MCU: 1=Manual, 2=Sleep, 3=Humidity, 4=Auto @@ -253,8 +247,7 @@ namespace esphome if (have_dry_active && dry_active) mod = 6; ESP_LOGV(TAG_SUP, "Applying to fan: power=%d speed=%d mode=%d", pwr, spd, mod); - fan->apply_device_status(pwr, spd, mod); - } + self->apply_fan_status(pwr, spd, mod); } void decode_superior_timer(Levoit *self, diff --git a/components/levoit/switch/__init__.py b/components/levoit/switch/__init__.py index 8c75dea..4ed1876 100644 --- a/components/levoit/switch/__init__.py +++ b/components/levoit/switch/__init__.py @@ -31,6 +31,9 @@ ) async def to_code(config): + # Tells the levoit sources this platform is in the build - see the note + # at the top of levoit.cpp on why USE_SWITCH is not sufficient. + cg.add_define("USE_LEVOIT_SWITCH") parent = await cg.get_variable(config[CONF_LEVOIT_ID]) var = cg.new_Pvariable(config[CONF_ID]) diff --git a/components/levoit/tests/.gitignore b/components/levoit/tests/.gitignore new file mode 100644 index 0000000..d8b4157 --- /dev/null +++ b/components/levoit/tests/.gitignore @@ -0,0 +1,5 @@ +# Gitignore settings for ESPHome +# This is an example and may include too much for your use-case. +# You can modify this file to suit your needs. +/.esphome/ +/secrets.yaml diff --git a/components/levoit/tests/README.md b/components/levoit/tests/README.md new file mode 100644 index 0000000..363b269 --- /dev/null +++ b/components/levoit/tests/README.md @@ -0,0 +1,40 @@ +# Compile tests + +Self-contained ESPHome configs that build the `levoit` component without needing +a `secrets.yaml` — Wi-Fi and API credentials are inline dummies. They point at +`../..` as the external component source, so they always build the working copy. + +Run by [`.github/workflows/compile-tests.yml`](../../../.github/workflows/compile-tests.yml) +on every pull request that touches `components/`, and locally with: + +```bash +cd components/levoit/tests +esphome compile test-all-entities.yaml +``` + +## What each one is for + +| Config | Catches | +|--------|---------| +| `test-all-entities.yaml` | Codegen for **every entity type on every platform**. Anything added to a `TYPE_MAP` without working codegen fails here. | +| `test-fan-only.yaml` | Sources that include a platform header **without guarding on `USE_LEVOIT_*`**. With only a fan configured, ESPHome copies no `number/`, `sensor/`, `select/` … directories, so an unguarded include cannot resolve. | +| `test-foreign-platforms.yaml` | The subtler variant: `template` entities supply the platforms, so the **generic** `USE_SWITCH` / `USE_SELECT` / … macros are defined while no levoit platform directory is copied. Guarding on those generic macros instead of `USE_LEVOIT_*` breaks exactly here. | + +Both guard tests exist because that bug was real: before them, a levoit config +with only a fan did not build at all, and one with an unrelated `template` switch +failed the same way. + +## Why not one config per model + +`model:` is a runtime enum — every model's C++ branch compiles regardless of +which one a config selects — so per-model configs would add build time without +adding compile coverage. What actually varies at compile time is *which +platforms are present*, which is what these three configs span. + +Runtime behaviour per model is not covered here; that still needs hardware. + +## Adding a type + +Add it to the platform's `TYPE_MAP`, then add the same entry to +`test-all-entities.yaml`. The file lists types in `TYPE_MAP` order, so keeping +them aligned makes a missing one easy to spot. diff --git a/components/levoit/tests/test-all-entities.yaml b/components/levoit/tests/test-all-entities.yaml new file mode 100644 index 0000000..6a5990d --- /dev/null +++ b/components/levoit/tests/test-all-entities.yaml @@ -0,0 +1,320 @@ +# GENERATED-ish: hand-editable. See README.md in this folder. +esphome: + name: test-levoit-all + +esp32: + board: esp32dev + framework: + type: esp-idf + +logger: + level: DEBUG + +# Dummy credentials on purpose - these tests must build without a secrets.yaml. +# levoit.cpp includes esphome/components/network/util.h, so some network +# component has to be present or the build fails to find that header. +wifi: + ssid: "compile-test" + password: "compile-test" + +api: + encryption: + key: "mfEv/h5ZAzrr46UQ/Mt35PpgcbVPFlPNhlLMZzAol4w=" + +external_components: + - source: ../.. + components: [levoit] + +uart: + - id: uart_mcu + tx_pin: GPIO17 + rx_pin: GPIO16 + baud_rate: 115200 + +levoit: + id: dev + model: CORE400S + uart_id: uart_mcu + +fan: + - platform: levoit + levoit: dev + name: "Fan" + +switch: + - platform: levoit + levoit: dev + name: "display" + type: display + - platform: levoit + levoit: dev + name: "child_lock" + type: child_lock + - platform: levoit + levoit: dev + name: "light_detect" + type: light_detect + - platform: levoit + levoit: dev + name: "quick_clean" + type: quick_clean + - platform: levoit + levoit: dev + name: "white_noise" + type: white_noise + - platform: levoit + levoit: dev + name: "daytime_enabled" + type: daytime_enabled + - platform: levoit + levoit: dev + name: "led_ring" + type: led_ring + - platform: levoit + levoit: dev + name: "auto_dry_power_off" + type: auto_dry_power_off + - platform: levoit + levoit: dev + name: "auto_dry_water_empty" + type: auto_dry_water_empty + +sensor: + - platform: levoit + levoit: dev + name: "aqi" + type: aqi + - platform: levoit + levoit: dev + name: "pm25" + type: pm25 + - platform: levoit + levoit: dev + name: "timer_current" + type: timer_current + - platform: levoit + levoit: dev + name: "efficiency_counter" + type: efficiency_counter + - platform: levoit + levoit: dev + name: "current_cadr" + type: current_cadr + - platform: levoit + levoit: dev + name: "filter_life_left" + type: filter_life_left + - platform: levoit + levoit: dev + name: "pm1_0" + type: pm1_0 + - platform: levoit + levoit: dev + name: "pm10" + type: pm10 + - platform: levoit + levoit: dev + name: "fan_rpm" + type: fan_rpm + - platform: levoit + levoit: dev + name: "humidity" + type: humidity + - platform: levoit + levoit: dev + name: "temperature" + type: temperature + - platform: levoit + levoit: dev + name: "filter_life_mcu" + type: filter_life_mcu + +binary_sensor: + - platform: levoit + levoit: dev + name: "filter_low" + type: filter_low + - platform: levoit + levoit: dev + name: "cover_open" + type: cover_open + - platform: levoit + levoit: dev + name: "dark_detected" + type: dark_detected + - platform: levoit + levoit: dev + name: "water_tank_empty" + type: water_tank_empty + - platform: levoit + levoit: dev + name: "dry_active" + type: dry_active + - platform: levoit + levoit: dev + name: "humidifying" + type: humidifying + +select: + - platform: levoit + levoit: dev + name: "auto_mode" + type: auto_mode + - platform: levoit + levoit: dev + name: "sleep_mode" + type: sleep_mode + - platform: levoit + levoit: dev + name: "white_noise_fan_level" + type: white_noise_fan_level + - platform: levoit + levoit: dev + name: "sleep_mode_fan_mode_level" + type: sleep_mode_fan_mode_level + - platform: levoit + levoit: dev + name: "daytime_fan_mode" + type: daytime_fan_mode + - platform: levoit + levoit: dev + name: "nightlight" + type: nightlight + - platform: levoit + levoit: dev + name: "light_mode" + type: light_mode + - platform: levoit + levoit: dev + name: "white_noise_sound" + type: white_noise_sound + - platform: levoit + levoit: dev + name: "sleep_preference" + type: sleep_preference + - platform: levoit + levoit: dev + name: "fan_operating_mode" + type: fan_operating_mode + - platform: levoit + levoit: dev + name: "auto_profile" + type: auto_profile + - platform: levoit + levoit: dev + name: "humidity_subtype" + type: humidity_subtype + - platform: levoit + levoit: dev + name: "dry_level" + type: dry_level + +number: + - platform: levoit + levoit: dev + name: "timer" + type: timer + - platform: levoit + levoit: dev + name: "efficiency_room_size" + type: efficiency_room_size + - platform: levoit + levoit: dev + name: "quick_clean_min" + type: quick_clean_min + - platform: levoit + levoit: dev + name: "quick_clean_minutes" + type: quick_clean_minutes + - platform: levoit + levoit: dev + name: "quick_clean_fan_level" + type: quick_clean_fan_level + - platform: levoit + levoit: dev + name: "daytime_fan_level" + type: daytime_fan_level + - platform: levoit + levoit: dev + name: "white_noise_min" + type: white_noise_min + - platform: levoit + levoit: dev + name: "sleep_mode_min" + type: sleep_mode_min + - platform: levoit + levoit: dev + name: "sleep_minutes" + type: sleep_minutes + - platform: levoit + levoit: dev + name: "sleep_fan_level" + type: sleep_fan_level + - platform: levoit + levoit: dev + name: "filter_lifetime_months" + type: filter_lifetime_months + - platform: levoit + levoit: dev + name: "led_brightness_min" + type: led_brightness_min + - platform: levoit + levoit: dev + name: "led_speed" + type: led_speed + - platform: levoit + levoit: dev + name: "white_noise_volume" + type: white_noise_volume + - platform: levoit + levoit: dev + name: "aqi_scale" + type: aqi_scale + - platform: levoit + levoit: dev + name: "vent_angle" + type: vent_angle + - platform: levoit + levoit: dev + name: "auto_profile_room_size_input" + type: auto_profile_room_size_input + - platform: levoit + levoit: dev + name: "humidity_target" + type: humidity_target + +text_sensor: + - platform: levoit + levoit: dev + name: "mcu_version" + type: mcu_version + - platform: levoit + levoit: dev + name: "esp_version" + type: esp_version + - platform: levoit + levoit: dev + name: "timer_duration_initial" + type: timer_duration_initial + - platform: levoit + levoit: dev + name: "timer_duration_remaining" + type: timer_duration_remaining + - platform: levoit + levoit: dev + name: "auto_mode_room_size_high_fan" + type: auto_mode_room_size_high_fan + - platform: levoit + levoit: dev + name: "error_message" + type: error_message + - platform: levoit + levoit: dev + name: "sprout_event" + type: sprout_event + +button: + - platform: levoit + levoit: dev + name: "reset_filter_stats" + type: reset_filter_stats diff --git a/components/levoit/tests/test-fan-only.yaml b/components/levoit/tests/test-fan-only.yaml new file mode 100644 index 0000000..160c661 --- /dev/null +++ b/components/levoit/tests/test-fan-only.yaml @@ -0,0 +1,42 @@ +# GENERATED-ish: hand-editable. See README.md in this folder. +esphome: + name: test-levoit-fan-only + +esp32: + board: esp32dev + framework: + type: esp-idf + +logger: + level: DEBUG + +# Dummy credentials on purpose - these tests must build without a secrets.yaml. +# levoit.cpp includes esphome/components/network/util.h, so some network +# component has to be present or the build fails to find that header. +wifi: + ssid: "compile-test" + password: "compile-test" + +api: + encryption: + key: "mfEv/h5ZAzrr46UQ/Mt35PpgcbVPFlPNhlLMZzAol4w=" + +external_components: + - source: ../.. + components: [levoit] + +uart: + - id: uart_mcu + tx_pin: GPIO17 + rx_pin: GPIO16 + baud_rate: 115200 + +levoit: + id: dev + model: CORE200S + uart_id: uart_mcu + +fan: + - platform: levoit + levoit: dev + name: "Fan" diff --git a/components/levoit/tests/test-foreign-platforms.yaml b/components/levoit/tests/test-foreign-platforms.yaml new file mode 100644 index 0000000..975d1b7 --- /dev/null +++ b/components/levoit/tests/test-foreign-platforms.yaml @@ -0,0 +1,81 @@ +# GENERATED-ish: hand-editable. See README.md in this folder. +esphome: + name: test-levoit-foreign + +esp32: + board: esp32dev + framework: + type: esp-idf + +logger: + level: DEBUG + +# Dummy credentials on purpose - these tests must build without a secrets.yaml. +# levoit.cpp includes esphome/components/network/util.h, so some network +# component has to be present or the build fails to find that header. +wifi: + ssid: "compile-test" + password: "compile-test" + +api: + encryption: + key: "mfEv/h5ZAzrr46UQ/Mt35PpgcbVPFlPNhlLMZzAol4w=" + +external_components: + - source: ../.. + components: [levoit] + +uart: + - id: uart_mcu + tx_pin: GPIO17 + rx_pin: GPIO16 + baud_rate: 115200 + +levoit: + id: dev + model: CORE200S + uart_id: uart_mcu + +fan: + - platform: levoit + levoit: dev + name: "Fan" + +# No levoit entities on these platforms, but the platforms ARE in the build. +# That defines USE_SWITCH / USE_SELECT / USE_NUMBER / USE_SENSOR / ... while +# ESPHome copies no levoit// sources, which is exactly the mismatch +# that used to break the build before the USE_LEVOIT_* guards. +switch: + - platform: template + name: "Foreign Switch" + optimistic: true + +select: + - platform: template + name: "Foreign Select" + optimistic: true + options: ["a", "b"] + +number: + - platform: template + name: "Foreign Number" + optimistic: true + min_value: 0 + max_value: 10 + step: 1 + +sensor: + - platform: template + name: "Foreign Sensor" + +binary_sensor: + - platform: template + name: "Foreign Binary Sensor" + +text_sensor: + - platform: template + name: "Foreign Text Sensor" + +button: + - platform: template + name: "Foreign Button" diff --git a/components/levoit/text_sensor/__init__.py b/components/levoit/text_sensor/__init__.py index e0e1965..7966120 100644 --- a/components/levoit/text_sensor/__init__.py +++ b/components/levoit/text_sensor/__init__.py @@ -40,6 +40,9 @@ ) async def to_code(config): + # Tells the levoit sources this platform is in the build - see the note + # at the top of levoit.cpp on why USE_TEXT_SENSOR is not sufficient. + cg.add_define("USE_LEVOIT_TEXT_SENSOR") parent = await cg.get_variable(config[CONF_LEVOIT_ID]) sensor_type = config[CONF_TYPE] diff --git a/components/levoit/vital_commands.cpp b/components/levoit/vital_commands.cpp index 7106443..abbb9a7 100644 --- a/components/levoit/vital_commands.cpp +++ b/components/levoit/vital_commands.cpp @@ -1,10 +1,7 @@ #include "vital_commands.h" #include "levoit_message.h" #include "levoit.h" -#include "number/levoit_number.h" -#include "select/levoit_select.h" -#include "switch/levoit_switch.h" -#include "esphome/components/switch/switch.h" +#include "esphome/core/defines.h" #include "esphome/core/log.h" namespace esphome @@ -119,11 +116,10 @@ namespace esphome case CommandType::setAutoModeEfficient: { msg_type = {0x02, 0x02, 0x55}; - auto *num = self->get_number(NumberType::EFFICIENCY_ROOM_SIZE); - if (num != nullptr) + float m2; + if (self->try_get_number_state(NumberType::EFFICIENCY_ROOM_SIZE, m2)) { // Convert m² → raw MCU value: 1 m² = 10.764 sq ft, Vital MCU uses sq_ft × 1.3 - float m2 = num->state; uint32_t room_size = static_cast(m2 * 10.764f * 1.3f + 0.5f); uint8_t size_low = room_size & 0xFF; uint8_t size_high = (room_size >> 8) & 0xFF; @@ -155,10 +151,10 @@ namespace esphome case CommandType::setTimerMinutes: { msg_type = {0x02, 0x19, 0x50}; - auto *num = self->get_number(NumberType::TIMER); - if (num != nullptr) + float mins; + if (self->try_get_number_state(NumberType::TIMER, mins)) { - uint32_t secs = static_cast(num->state) * 60; + uint32_t secs = static_cast(mins) * 60; uint8_t b0 = (secs >> 0) & 0xFF; uint8_t b1 = (secs >> 8) & 0xFF; uint8_t b2 = (secs >> 16) & 0xFF; @@ -225,36 +221,18 @@ namespace esphome uint8_t dt_mode = c.dt_mode; uint8_t dt_level = c.dt_level; - if (auto *sel = self->get_select(SelectType::SLEEP_PREFERENCE)) { - if (auto idx = sel->active_index()) sleep_type = (uint8_t)idx.value(); - } - if (auto *n = self->get_number(NumberType::SLEEP_FAN_LEVEL)) { - if (n->has_state()) sleep_fan = (uint8_t)n->state; - } - if (auto *n = self->get_number(NumberType::SLEEP_MODE_MIN)) { - if (n->has_state()) sleep_min = (uint16_t)n->state; - } + { size_t i; if (self->try_get_select_index(SelectType::SLEEP_PREFERENCE, i)) sleep_type = (uint8_t)i; } + { float v; if (self->try_get_number_state(NumberType::SLEEP_FAN_LEVEL, v)) sleep_fan = (uint8_t)v; } + { float v; if (self->try_get_number_state(NumberType::SLEEP_MODE_MIN, v)) sleep_min = (uint16_t)v; } // QC cluster (Stage 4). Optimistic publish in LevoitSwitch / LevoitNumber // means the entity reflects the new value by the time we read it here. - if (auto *sw = self->get_switch(SwitchType::QUICK_CLEAN)) { - if (sw->has_state()) qc_enabled = sw->state ? 1 : 0; - } - if (auto *n = self->get_number(NumberType::QUICK_CLEAN_MIN)) { - if (n->has_state()) qc_min = (uint16_t)n->state; - } - if (auto *n = self->get_number(NumberType::QUICK_CLEAN_FAN_LEVEL)) { - if (n->has_state()) qc_fan = (uint8_t)n->state; - } + { bool b; if (self->try_get_switch_state(SwitchType::QUICK_CLEAN, b)) qc_enabled = b ? 1 : 0; } + { float v; if (self->try_get_number_state(NumberType::QUICK_CLEAN_MIN, v)) qc_min = (uint16_t)v; } + { float v; if (self->try_get_number_state(NumberType::QUICK_CLEAN_FAN_LEVEL, v)) qc_fan = (uint8_t)v; } // DT cluster (Stage 5). - if (auto *sw = self->get_switch(SwitchType::DAYTIME_ENABLED)) { - if (sw->has_state()) dt_enabled = sw->state ? 1 : 0; - } - if (auto *sel = self->get_select(SelectType::DAYTIME_FAN_MODE)) { - if (auto idx = sel->active_index()) dt_mode = (uint8_t)idx.value(); - } - if (auto *n = self->get_number(NumberType::DAYTIME_FAN_LEVEL)) { - if (n->has_state()) dt_level = (uint8_t)n->state; - } + { bool b; if (self->try_get_switch_state(SwitchType::DAYTIME_ENABLED, b)) dt_enabled = b ? 1 : 0; } + { size_t i; if (self->try_get_select_index(SelectType::DAYTIME_FAN_MODE, i)) dt_mode = (uint8_t)i; } + { float v; if (self->try_get_number_state(NumberType::DAYTIME_FAN_LEVEL, v)) dt_level = (uint8_t)v; } // WN cluster is cache-only by design — Vital 200S Pro has no WN // hardware, but the MCU still requires those 3 fields in every // bulk write or the parser silently drops the whole frame. @@ -339,11 +317,9 @@ namespace esphome // Status echoes the angle back in TLV tag 0x14. Hardware range 45–90°. msg_type = {0x02, 0x12, 0x55}; uint8_t angle = 75; // default louver position on power-up - if (auto *num = self->get_number(NumberType::VENT_ANGLE)) - { - if (num->has_state()) - angle = static_cast(num->state); - } + float v_angle; + if (self->try_get_number_state(NumberType::VENT_ANGLE, v_angle)) + angle = static_cast(v_angle); if (angle < 45) angle = 45; if (angle > 90) angle = 90; ESP_LOGD(TAG_VITAL_CMD, "setVentAngle: %u deg", angle); diff --git a/components/levoit/vital_status.cpp b/components/levoit/vital_status.cpp index cb90881..ce63b95 100644 --- a/components/levoit/vital_status.cpp +++ b/components/levoit/vital_status.cpp @@ -1,8 +1,8 @@ #include "vital_status.h" +#include "esphome/core/defines.h" #include "esphome/core/log.h" #include "tlv.h" // <- TLV extraction #include "types.h" -#include "fan/levoit_fan.h" #include "decoder_helpers.h" #include #include @@ -460,10 +460,7 @@ namespace esphome } // Apply to ESPHome fan once, with consistent values - auto *fan = (self != nullptr) ? self->get_fan() : nullptr; - if (fan != nullptr) - { - // Use sentinels: power_known, speed_known, mode_known + // Use sentinels: power_known, speed_known, mode_known // - power: pass 0/1 only if have_power, otherwise pass -1 // - speed: pass >0 only if have_speed, otherwise -1 // - mode : pass >=0 only if have_mode, otherwise -1 @@ -472,8 +469,7 @@ namespace esphome int spd = have_speed ? (int)speed : -1; int mod = have_mode ? (int)mode : -1; ESP_LOGV(TAG_VITAL, "Applying to fan: power=%d speed=%d mode=%d", pwr, spd, mod); - fan->apply_device_status(pwr, spd, mod); - } + self->apply_fan_status(pwr, spd, mod); } } // namespace levoit From dcb56f1f70e472386fc68ac4c2fa0bc2bcb85efd Mon Sep 17 00:00:00 2001 From: Tobias Stanzel Date: Sat, 26 Sep 2026 18:16:14 +0200 Subject: [PATCH 2/4] fix(levoit): preserve the -1 fan sentinels and the null-self guard Review findings on #68, all four real: - apply_fan_status() took (bool, uint8_t, uint32_t), but the Vital and Superior decoders deliberately pass -1 for "not reported in this frame, leave the field alone". Narrowing turned an unknown power into ON and an unknown speed into 255, which then clamps to maximum - so a partial status frame would switch the fan on at full speed. Takes (int, int, int) now, matching LevoitFan::apply_device_status(). - The rewrite of vital_status.cpp dropped the `self != nullptr` check at that call site, even though decode_vital_status() accepts a nullable self and guards its other entity updates. Restored. - Guarding the fan include was not enough on its own: track_cadr_usage() and calculate_current_cadr_per_hour() still dereferenced fan_ against a forward-declared type. Both are now inside USE_LEVOIT_FAN, with the latter returning 0 when there is no fan entity to derive a duty cycle from. The first two were regressions I introduced in the previous commit; only the third was pre-existing. Verified: all three test configs compile, plus Core 400S and Vital 200S. Co-Authored-By: Claude Opus 5 (1M context) --- components/levoit/levoit.cpp | 9 ++++++++- components/levoit/levoit.h | 5 ++++- components/levoit/vital_status.cpp | 3 ++- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/components/levoit/levoit.cpp b/components/levoit/levoit.cpp index 35c0347..fcd3a6d 100644 --- a/components/levoit/levoit.cpp +++ b/components/levoit/levoit.cpp @@ -343,7 +343,7 @@ namespace esphome #endif } - void Levoit::apply_fan_status(bool power, uint8_t speed, uint32_t mode) + void Levoit::apply_fan_status(int power, int speed, int mode) { #ifdef USE_LEVOIT_FAN if (this->fan_ != nullptr) @@ -843,6 +843,7 @@ namespace esphome void Levoit::track_cadr_usage() { +#ifdef USE_LEVOIT_FAN // Get fan state - check if fan is ON using .state member if (this->fan_ != nullptr && this->fan_->state) { // Fan is enabled - track usage @@ -868,10 +869,15 @@ namespace esphome pref_used_cadr_.save(&used_cadr_); pref_total_runtime_.save(&total_runtime_); } +#endif } uint32_t Levoit::calculate_current_cadr_per_hour() const { +#ifndef USE_LEVOIT_FAN + // No fan entity: nothing to derive a duty cycle from. + return 0; +#else if (this->fan_ == nullptr || !this->fan_->state) return 0; int speed = this->fan_->speed; @@ -905,6 +911,7 @@ namespace esphome } return result; +#endif } float Levoit::calculate_filter_life_left_percent() const diff --git a/components/levoit/levoit.h b/components/levoit/levoit.h index aac287e..9827468 100644 --- a/components/levoit/levoit.h +++ b/components/levoit/levoit.h @@ -111,7 +111,10 @@ class Levoit : public Component, public uart::UARTDevice { bool try_get_number_state(NumberType type, float &out) const; bool try_get_select_index(SelectType type, size_t &out) const; bool try_get_switch_state(SwitchType type, bool &out) const; - void apply_fan_status(bool power, uint8_t speed, uint32_t mode); + // power/speed/mode are the same signed values LevoitFan::apply_device_status + // takes: -1 means "not reported in this frame, leave the field alone". Do not + // narrow these - the Vital and Superior decoders rely on that sentinel. + void apply_fan_status(int power, int speed, int mode); void start_timer(){this->timer_active_ = true; this->timer_stop_pending_ = false;}; void stop_timer(){this->timer_active_ = false;}; diff --git a/components/levoit/vital_status.cpp b/components/levoit/vital_status.cpp index ce63b95..cdbf6cb 100644 --- a/components/levoit/vital_status.cpp +++ b/components/levoit/vital_status.cpp @@ -469,7 +469,8 @@ namespace esphome int spd = have_speed ? (int)speed : -1; int mod = have_mode ? (int)mode : -1; ESP_LOGV(TAG_VITAL, "Applying to fan: power=%d speed=%d mode=%d", pwr, spd, mod); - self->apply_fan_status(pwr, spd, mod); + if (self != nullptr) + self->apply_fan_status(pwr, spd, mod); } } // namespace levoit From 52ccddd13756cc29dd0ab0a98adb086fd4be0fd8 Mon Sep 17 00:00:00 2001 From: Tobias Stanzel Date: Sat, 26 Sep 2026 18:26:15 +0200 Subject: [PATCH 3/4] test(philips): add the same compile tests, and run both components in CI Mirrors the levoit suite for philips: every entity type, a fan-only config, and one where template entities supply the platforms. That last one is the regression test for a bug that was real - before the USE_PHILIPS_* guards, an AC0650 config with an unrelated template switch failed with "fatal error: switch/philips_switch.h: No such file". Nothing was guarding that fix until now. All three pass on the first run, so the philips guards were already correct. The workflow becomes a component x config matrix, so both components build on every PR - six jobs instead of three. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/compile-tests.yml | 19 +-- components/philips/tests/.gitignore | 5 + components/philips/tests/README.md | 41 +++++++ .../philips/tests/test-all-entities.yaml | 108 ++++++++++++++++++ components/philips/tests/test-fan-only.yaml | 44 +++++++ .../philips/tests/test-foreign-platforms.yaml | 79 +++++++++++++ 6 files changed, 287 insertions(+), 9 deletions(-) create mode 100644 components/philips/tests/.gitignore create mode 100644 components/philips/tests/README.md create mode 100644 components/philips/tests/test-all-entities.yaml create mode 100644 components/philips/tests/test-fan-only.yaml create mode 100644 components/philips/tests/test-foreign-platforms.yaml diff --git a/.github/workflows/compile-tests.yml b/.github/workflows/compile-tests.yml index 44b3b5c..a0bf3a6 100644 --- a/.github/workflows/compile-tests.yml +++ b/.github/workflows/compile-tests.yml @@ -20,21 +20,22 @@ concurrency: cancel-in-progress: true jobs: - levoit: - name: levoit · ${{ matrix.config }} + compile: + name: ${{ matrix.component }} · ${{ matrix.config }} runs-on: ubuntu-latest strategy: fail-fast: false matrix: + component: [levoit, philips] config: # every platform and every entity type - maximum codegen coverage - test-all-entities.yaml # only a fan: every optional platform absent. Catches sources that - # include a platform header without guarding on USE_LEVOIT_*. + # include a platform header without guarding on USE__*. - test-fan-only.yaml - # levoit fan plus template entities on the other platforms. Defines the - # generic USE_SWITCH / USE_SELECT / ... without any levoit entity of - # that kind, which is the mismatch that broke the build before. + # component fan plus template entities on the other platforms. Defines + # the generic USE_SWITCH / USE_SELECT / ... without any entity of that + # kind from this component, which is the mismatch that broke the build. - test-foreign-platforms.yaml steps: - uses: actions/checkout@v4 @@ -51,16 +52,16 @@ jobs: path: | ~/.platformio ~/.cache/pip - key: pio-${{ runner.os }}-${{ hashFiles('components/levoit/tests/*.yaml') }} + key: pio-${{ runner.os }}-${{ hashFiles('components/*/tests/*.yaml') }} restore-keys: pio-${{ runner.os }}- - name: Install ESPHome run: pip install esphome - name: esphome config - working-directory: components/levoit/tests + working-directory: components/${{ matrix.component }}/tests run: esphome config ${{ matrix.config }} - name: esphome compile - working-directory: components/levoit/tests + working-directory: components/${{ matrix.component }}/tests run: esphome compile ${{ matrix.config }} diff --git a/components/philips/tests/.gitignore b/components/philips/tests/.gitignore new file mode 100644 index 0000000..d8b4157 --- /dev/null +++ b/components/philips/tests/.gitignore @@ -0,0 +1,5 @@ +# Gitignore settings for ESPHome +# This is an example and may include too much for your use-case. +# You can modify this file to suit your needs. +/.esphome/ +/secrets.yaml diff --git a/components/philips/tests/README.md b/components/philips/tests/README.md new file mode 100644 index 0000000..70b813a --- /dev/null +++ b/components/philips/tests/README.md @@ -0,0 +1,41 @@ +# Compile tests + +Self-contained ESPHome configs that build the `philips` component without +needing a `secrets.yaml` — Wi-Fi and API credentials are inline dummies. They +point at `../..` as the external component source, so they always build the +working copy. + +Run by [`.github/workflows/compile-tests.yml`](../../../.github/workflows/compile-tests.yml) +on every pull request that touches `components/`, and locally with: + +```bash +cd components/philips/tests +esphome compile test-all-entities.yaml +``` + +## What each one is for + +| Config | Catches | +|--------|---------| +| `test-all-entities.yaml` | Codegen for **every entity type on every platform** (`AC0951`, the model with the most entities). Anything added to a `TYPE_MAP` without working codegen fails here. | +| `test-fan-only.yaml` | Sources that include a platform header **without guarding on `USE_PHILIPS_*`**. With only a fan configured, ESPHome copies no `switch/`, `select/`, `number/` … directories, so an unguarded include cannot resolve. | +| `test-foreign-platforms.yaml` | The subtler variant: `template` entities supply the platforms, so the **generic** `USE_SWITCH` / `USE_SELECT` / … macros are defined while no philips platform directory is copied. Guarding on those generic macros instead of `USE_PHILIPS_*` breaks exactly here. | + +The last one is the regression test for a bug that was real: before the +`USE_PHILIPS_*` guards, an `AC0650` config with an unrelated `template` switch +failed to compile with `fatal error: switch/philips_switch.h: No such file`. + +## Why not one config per model + +`model:` is a runtime enum — every model's C++ branch compiles regardless of +which one a config selects — so per-model configs would add build time without +adding compile coverage. What varies at compile time is *which platforms are +present*, which is what these three configs span. + +Runtime behaviour per model is not covered here; that still needs hardware, and +the `AC0950` in particular has never been tested on a device. + +## Adding a type + +Add it to the platform's `TYPE_MAP`, then add the same entry to +`test-all-entities.yaml`. diff --git a/components/philips/tests/test-all-entities.yaml b/components/philips/tests/test-all-entities.yaml new file mode 100644 index 0000000..71ee954 --- /dev/null +++ b/components/philips/tests/test-all-entities.yaml @@ -0,0 +1,108 @@ +# Self-contained compile test - see README.md in this folder. +esphome: + name: test-philips-all + +esp32: + board: seeed_xiao_esp32c3 + framework: + type: esp-idf + +logger: + # GPIO20/21 are the C3's UART0 pins and GPIO20 carries the MCU link, so the + # logger has to go somewhere else. + hardware_uart: USB_SERIAL_JTAG + level: DEBUG + +# Dummy credentials on purpose - these tests must build without a secrets.yaml. +# philips.cpp pulls in network headers, so a network component must be present. +wifi: + ssid: "compile-test" + password: "compile-test" + +api: + encryption: + key: "mfEv/h5ZAzrr46UQ/Mt35PpgcbVPFlPNhlLMZzAol4w=" + +external_components: + - source: ../.. + components: [philips] + +uart: + - id: uart_mcu + rx_pin: GPIO20 + tx_pin: GPIO10 + baud_rate: 115200 + +philips: + id: purifier + model: AC0951 + uart_id: uart_mcu + +fan: + - platform: philips + philips: purifier + name: "Fan" + +switch: + - platform: philips + philips: purifier + name: "standby_sensor" + type: standby_sensor + - platform: philips + philips: purifier + name: "child_lock" + type: child_lock + - platform: philips + philips: purifier + name: "beep" + type: beep + +sensor: + - platform: philips + philips: purifier + name: "filter_clean" + type: filter_clean + - platform: philips + philips: purifier + name: "filter_lifetime" + type: filter_lifetime + - platform: philips + philips: purifier + name: "pm2_5" + type: pm2_5 + - platform: philips + philips: purifier + name: "allergen_index" + type: allergen_index + - platform: philips + philips: purifier + name: "timer_remaining" + type: timer_remaining + +select: + - platform: philips + philips: purifier + name: "display_brightness" + type: display_brightness + +number: + - platform: philips + philips: purifier + name: "timer" + type: timer + +text_sensor: + - platform: philips + philips: purifier + name: "mcu_version" + type: mcu_version + +button: + - platform: philips + philips: purifier + name: "reset_prefilter" + type: reset_prefilter + - platform: philips + philips: purifier + name: "reset_hepa" + type: reset_hepa diff --git a/components/philips/tests/test-fan-only.yaml b/components/philips/tests/test-fan-only.yaml new file mode 100644 index 0000000..6723bdf --- /dev/null +++ b/components/philips/tests/test-fan-only.yaml @@ -0,0 +1,44 @@ +# Self-contained compile test - see README.md in this folder. +esphome: + name: test-philips-fan-only + +esp32: + board: seeed_xiao_esp32c3 + framework: + type: esp-idf + +logger: + # GPIO20/21 are the C3's UART0 pins and GPIO20 carries the MCU link, so the + # logger has to go somewhere else. + hardware_uart: USB_SERIAL_JTAG + level: DEBUG + +# Dummy credentials on purpose - these tests must build without a secrets.yaml. +# philips.cpp pulls in network headers, so a network component must be present. +wifi: + ssid: "compile-test" + password: "compile-test" + +api: + encryption: + key: "mfEv/h5ZAzrr46UQ/Mt35PpgcbVPFlPNhlLMZzAol4w=" + +external_components: + - source: ../.. + components: [philips] + +uart: + - id: uart_mcu + rx_pin: GPIO20 + tx_pin: GPIO10 + baud_rate: 115200 + +philips: + id: purifier + model: AC0650 + uart_id: uart_mcu + +fan: + - platform: philips + philips: purifier + name: "Fan" diff --git a/components/philips/tests/test-foreign-platforms.yaml b/components/philips/tests/test-foreign-platforms.yaml new file mode 100644 index 0000000..a90a429 --- /dev/null +++ b/components/philips/tests/test-foreign-platforms.yaml @@ -0,0 +1,79 @@ +# Self-contained compile test - see README.md in this folder. +esphome: + name: test-philips-foreign + +esp32: + board: seeed_xiao_esp32c3 + framework: + type: esp-idf + +logger: + # GPIO20/21 are the C3's UART0 pins and GPIO20 carries the MCU link, so the + # logger has to go somewhere else. + hardware_uart: USB_SERIAL_JTAG + level: DEBUG + +# Dummy credentials on purpose - these tests must build without a secrets.yaml. +# philips.cpp pulls in network headers, so a network component must be present. +wifi: + ssid: "compile-test" + password: "compile-test" + +api: + encryption: + key: "mfEv/h5ZAzrr46UQ/Mt35PpgcbVPFlPNhlLMZzAol4w=" + +external_components: + - source: ../.. + components: [philips] + +uart: + - id: uart_mcu + rx_pin: GPIO20 + tx_pin: GPIO10 + baud_rate: 115200 + +philips: + id: purifier + model: AC0650 + uart_id: uart_mcu + +fan: + - platform: philips + philips: purifier + name: "Fan" + +# No philips entities on these platforms, but the platforms ARE in the build. +# That defines USE_SWITCH / USE_SELECT / USE_NUMBER / ... while ESPHome copies +# no philips// sources - the mismatch the USE_PHILIPS_* guards exist +# to survive. +switch: + - platform: template + name: "Foreign Switch" + optimistic: true + +select: + - platform: template + name: "Foreign Select" + optimistic: true + options: ["a", "b"] + +number: + - platform: template + name: "Foreign Number" + optimistic: true + min_value: 0 + max_value: 10 + step: 1 + +sensor: + - platform: template + name: "Foreign Sensor" + +text_sensor: + - platform: template + name: "Foreign Text Sensor" + +button: + - platform: template + name: "Foreign Button" From 5104f8819db1ed76de52f165d44e8e10a8a4b495 Mon Sep 17 00:00:00 2001 From: Tobias Stanzel Date: Sat, 26 Sep 2026 18:29:42 +0200 Subject: [PATCH 4/4] test(levoit): cover the light platform, and re-apply the changelog correction Review finding: test-all-entities.yaml omitted the light platform, so USE_LEVOIT_LIGHT and the Sprout LED-ring codegen went untested. The generator built the entity list from each platform's TYPE_MAP and light has none - it is a single light_schema entity - so it was skipped silently. Listed explicitly now, with a comment saying why. All nine platforms that define a USE_LEVOIT_* macro are exercised. The changelog correction in this branch targeted the root README, but #69 moved the levoit change log to components/levoit/README.md in the meantime, so the rebase dropped it and the inaccurate claim survived at its new location. Re-applied there. Co-Authored-By: Claude Opus 5 (1M context) --- components/levoit/README.md | 18 ++++++++++++++---- components/levoit/tests/test-all-entities.yaml | 10 ++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/components/levoit/README.md b/components/levoit/README.md index f509042..fa0f37c 100644 --- a/components/levoit/README.md +++ b/components/levoit/README.md @@ -536,10 +536,20 @@ Auto mode options per model: the purifiers use minutes * **`dry_level` does not act on its own** — the select records Low/High and the value is applied when Dry is picked on the fan entity -* Fix a latent build bug: platform includes were guarded on the generic - `USE_SWITCH` / `USE_SENSOR` / … macros, which any component defines, so a - config with (say) a `template` switch but no `levoit` switch failed to - compile. Each platform now defines its own `USE_LEVOIT_*` guard +* **Fix two build failures around optional platforms.** A levoit config with + only a fan did not build at all, and one with an unrelated `template` switch + (or select, number, …) failed the same way: several sources included + `number/levoit_number.h` and friends either unguarded or guarded on the + *generic* `USE_SWITCH` / `USE_NUMBER` macros, which any component defines, + while ESPHome only copies `levoit//` when a **levoit** entity of + that platform is configured + * Each platform now defines its own `USE_LEVOIT_*` macro, and the entity + classes are confined to `levoit.cpp` behind new value accessors + (`try_get_number_state()`, `try_get_select_index()`, + `try_get_switch_state()`, `apply_fan_status()`), so the other sources no + longer need those headers at all + * Added compile tests in [`tests/`](./tests) covering every entity type and + both of the above cases, run by GitHub Actions on every PR * `publish_sensor` widened `uint32_t` → `float` so the humidifier can report fractional temperature and humidity (it already converted internally) * ⚠️ Superior 6000S not verified on hardware — see diff --git a/components/levoit/tests/test-all-entities.yaml b/components/levoit/tests/test-all-entities.yaml index 6a5990d..d738038 100644 --- a/components/levoit/tests/test-all-entities.yaml +++ b/components/levoit/tests/test-all-entities.yaml @@ -41,6 +41,16 @@ fan: levoit: dev name: "Fan" +# The light platform has no TYPE_MAP - it is the single Sprout LED ring - so it +# needs listing explicitly, otherwise USE_LEVOIT_LIGHT and its codegen go +# untested. +light: + - platform: levoit + levoit: dev + name: "LED Ring" + gamma_correct: 0 + default_transition_length: 0s + switch: - platform: levoit levoit: dev