Skip to content

[actuator] Add TimeBasedActuatorBase, migrate TimeBasedCover (cover/valve refactor 3/5)#5

Closed
exciton wants to merge 20 commits into
cover_valve_refactorfrom
pr3/time-based-actuator-base
Closed

[actuator] Add TimeBasedActuatorBase, migrate TimeBasedCover (cover/valve refactor 3/5)#5
exciton wants to merge 20 commits into
cover_valve_refactorfrom
pr3/time-based-actuator-base

Conversation

@exciton

@exciton exciton commented May 24, 2026

Copy link
Copy Markdown
Owner

Cover/Valve Actuator Refactor — PR Series

Step PR Title Depends on
1/5 #3 [actuator] Add shared actuator base component
2/5 #4 [actuator] Add EndstopActuatorBase, migrate EndstopCover Step 1 (#3)
3/5 #5 [actuator] Add TimeBasedActuatorBase, migrate TimeBasedCover Step 1 (#3)
4/5 #6 [valve] Add endstop_valve platform Step 2 (#4)
5/5 #7 [valve] Add time_based_valve platform Step 3 (#5)

Merge order: #3#4 and #5 (parallel) → #6 and #7 (parallel)

This PR: Step 3/5


What does this implement/fix?

Extracts the time-based cover logic into a new reusable TimeBasedActuatorBase in the actuator component, then migrates TimeBasedCover to use it. This enables the time_based_valve platform (step 5/5) to reuse the same logic without duplication.

New actuator::TimeBasedActuatorBase (Component):

  • Full time-based logic: loop(), control(), position recompute, open/close/stop triggers
  • Supports has_built_in_endstop, manual_control, and assumed_state configuration
  • Holds an IActuator* pointer — works with any entity implementing IActuator (cover or valve)
  • No diamond inheritance: inherits only Component

TimeBasedCover migrated:

  • Now inherits TimeBasedActuatorBase + Cover (thin wrapper, constructor calls set_actuator(this))
  • get_traits() uses get_assumed_state() from the base
  • control() bridges to TimeBasedActuatorBase::control() via safe static_cast
  • Deprecation pragma suppressions removed (deprecated names were already eliminated in step 1)

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Other

Related issue or feature (if applicable): fixes

Pull request in esphome-docs with documentation (if applicable): N/A — no user-facing changes

Test Environment

  • ESP32
  • ESP32 IDF
  • ESP8266
  • RP2040
  • BK72xx
  • RTL87xx

Example entry for config.yaml:

No new YAML keys. Existing time_based cover configs continue to work unchanged.

cover:
  - platform: time_based
    name: "My Cover"
    open_action:
      - switch.turn_on: open_switch
    close_action:
      - switch.turn_on: close_switch
    stop_action:
      - switch.turn_off: open_switch
    open_duration: 25s
    close_duration: 23s

Checklist:

  • The code change is tested and works locally.
  • Tests have been added to verify that the new code works (under tests/ folder).

If user exposed functionality or configuration variables are added/changed:


Generated by Claude Code

claude added 2 commits May 24, 2026 17:43
- Phase 8: Add unit tests for TimeBasedActuatorBase (test_time_based_actuator.py)
- Phase 9: Fully implement TimeBasedActuatorBase in time_based_actuator.cpp,
  porting logic from TimeBasedCover using IActuator* interface; export
  TimeBasedActuatorBase from actuator/__init__.py; mark setup() inline empty
- Phase 10: Replace TimeBasedCover with thin wrapper inheriting
  TimeBasedActuatorBase + Cover; constructor calls set_actuator(this);
  setup() calls restore_state_(); get_traits() reads assumed_state via
  get_assumed_state(); dump_config() unchanged; update __init__.py class
  declaration to reference TimeBasedActuatorBase

https://claude.ai/code/session_01Q3vPAvxZLoVtM7PRo6CdgE
get_has_built_in_endstop() and get_last_operation() have no callers
anywhere in the codebase; derived classes access the fields directly
via this-> since they are protected. Confirmed no Python codegen
references either.

https://claude.ai/code/session_01Q3vPAvxZLoVtM7PRo6CdgE
claude added 7 commits May 24, 2026 19:14
…latforms

Introduces TIME_BASED_ACTUATOR_SCHEMA and apply_time_based_actuator_config in
actuator/__init__.py, and moves CONF_HAS_BUILT_IN_ENDSTOP / CONF_MANUAL_CONTROL
constants there. Simplifies time_based/cover/__init__.py from ~60 lines to ~20 lines.
… component

- Move time_based_actuator.h/.cpp from actuator/ to time_based/
- Change namespace from esphome::actuator to esphome::time_based
- Move TIME_BASED_ACTUATOR_SCHEMA, apply_time_based_actuator_config,
  CONF_HAS_BUILT_IN_ENDSTOP, CONF_MANUAL_CONTROL from actuator/__init__.py
  to time_based/__init__.py
- Update time_based/cover/__init__.py to import from local package
- Update time_based/cover/time_based_cover.h to include local header
  and use unqualified TimeBasedActuatorBase (same namespace)
- Strip time-based helpers from actuator/__init__.py; actuator now only
  contains shared base infrastructure (ActuatorBase, ActuatorCallBase,
  IActuator, ActuatorOperation, build_apply_lambda_action)

https://claude.ai/code/session_01Q3vPAvxZLoVtM7PRo6CdgE
IActuator::do_publish_state takes bool save with no default parameter;
callers must be explicit.

https://claude.ai/code/session_01Q3vPAvxZLoVtM7PRo6CdgE
Move cover/__init__.py → cover.py and cover/time_based_cover.{h,cpp} to
the time_based/ root, matching the flat layout used by the endstop component.

https://claude.ai/code/session_01Q3vPAvxZLoVtM7PRo6CdgE
@github-actions

github-actions Bot commented May 25, 2026

Copy link
Copy Markdown

Memory Impact Analysis

Components: time_based
Platform: esp8266-ard

Metric Target Branch This PR Change
RAM 28,760 bytes 28,760 bytes ➡️ +0 bytes (0.00%)
Flash 268,643 bytes 268,723 bytes 📈 🔸 +80 bytes (+0.03%)
📊 Component Memory Breakdown
Component Target Flash PR Flash Change
[esphome]time_based 1,479 bytes 1,579 bytes 📈 🚨 +100 bytes (+6.76%)
[esphome]core 7,284 bytes 7,276 bytes 📉 ✅ -8 bytes (-0.11%)
🔍 Symbol-Level Changes (click to expand)

Changed Symbols

Symbol Target Size PR Size Change
esphome::time_based::TimeBasedCover::control(esphome::cover::CoverCall const&) 363 bytes 6 bytes 📉 -357 bytes (-98.35%)
setup 469 bytes 461 bytes 📉 -8 bytes (-1.71%)
time_based__time_based_cover__pstorage 92 bytes 96 bytes 📈 +4 bytes (+4.35%)
vtable for esphome::time_based::TimeBasedCover 104 bytes 100 bytes 📉 -4 bytes (-3.85%)

New Symbols (top 15)

Symbol Size
esphome::time_based::TimeBasedActuatorBase::control_(esphome::actuator::ActuatorCallBase const&) 440 bytes
esphome::time_based::TimeBasedActuatorBase::recompute_position_() 207 bytes
esphome::time_based::TimeBasedActuatorBase::loop() 143 bytes
esphome::time_based::TimeBasedActuatorBase::start_direction_(esphome::actuator::ActuatorOperation) 125 bytes
esphome::time_based::TimeBasedActuatorBase::is_at_target_() const 95 bytes
esphome::time_based::TimeBasedActuatorBase::setup() 69 bytes
vtable for esphome::time_based::TimeBasedActuatorBase 48 bytes
esphome::time_based::TimeBasedActuatorBase::stop_prev_trigger_() 41 bytes
non-virtual thunk to esphome::time_based::TimeBasedCover::get_traits() 15 bytes
non-virtual thunk to esphome::time_based::TimeBasedCover::control(esphome::cover::CoverCall const&) 9 bytes

Removed Symbols (top 15)

Symbol Size
esphome::time_based::TimeBasedCover::setup() 196 bytes
esphome::time_based::TimeBasedCover::recompute_position_() 154 bytes
esphome::time_based::TimeBasedCover::loop() 128 bytes
esphome::time_based::TimeBasedCover::start_direction_(esphome::actuator::ActuatorOperation) 113 bytes
esphome::time_based::TimeBasedCover::is_at_target_() const 70 bytes
esphome::time_based::TimeBasedCover::stop_prev_trigger_() 43 bytes
non-virtual thunk to esphome::time_based::TimeBasedCover::loop() 9 bytes
non-virtual thunk to esphome::time_based::TimeBasedCover::dump_config() 9 bytes
non-virtual thunk to esphome::time_based::TimeBasedCover::setup() 9 bytes

Note: This analysis measures static RAM and Flash usage only (compile-time allocation).
Dynamic memory (heap) cannot be measured automatically.
⚠️ You must test this PR on a real device to measure free heap and ensure no runtime memory issues.

This analysis runs automatically when components change. Memory usage is measured from a representative test configuration.

exciton and others added 11 commits May 26, 2026 14:25
…rface

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Matches the standard ESPHome component layout where each platform lives
in its own subdirectory. Renames:
  cover.py             -> cover/__init__.py
  time_based_cover.h   -> cover/time_based_cover.h
  time_based_cover.cpp -> cover/time_based_cover.cpp

Updates the relative include in time_based_cover.h to reach the
parent-directory header, and the relative Python import in the moved
__init__.py to reach the parent package (from . -> from ..).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
After moving the cover platform into time_based/cover/, the parent
time_based/ directory's shared source files (time_based_actuator.cpp/.h)
were no longer being picked up by the build because the cover
subpackage's resource scan is limited to its own directory.

Adding AUTO_LOAD = ["time_based"] to the platform __init__.py registers
the parent component so its resources flow into the build, fixing
the missing-symbol link errors.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Move restore-state logic from TimeBasedCover::setup() into
  TimeBasedActuatorBase::setup() via do_restore_state(), matching the
  endstop pattern so TimeBasedValve gets setup for free
- Remove now-redundant TimeBasedCover::setup()
- Expand unit tests to match the endstop suite (class groups, Component
  transitively reachable test, is-class-like-object check)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@exciton exciton closed this Jun 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants