FIX: Unnecessary flash writes by ModbusSwitch component (#3648)

This commit is contained in:
Javier Peletier 2022-11-29 22:05:40 +01:00 committed by GitHub
parent a59ce7bfa2
commit c55e01ff3f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 114 additions and 144 deletions

View file

@ -2,20 +2,10 @@ import esphome.codegen as cg
import esphome.config_validation as cv
from esphome import pins
from esphome.components import switch
from esphome.const import CONF_INTERLOCK, CONF_PIN, CONF_RESTORE_MODE
from esphome.const import CONF_INTERLOCK, CONF_PIN
from .. import gpio_ns
GPIOSwitch = gpio_ns.class_("GPIOSwitch", switch.Switch, cg.Component)
GPIOSwitchRestoreMode = gpio_ns.enum("GPIOSwitchRestoreMode")
RESTORE_MODES = {
"RESTORE_DEFAULT_OFF": GPIOSwitchRestoreMode.GPIO_SWITCH_RESTORE_DEFAULT_OFF,
"RESTORE_DEFAULT_ON": GPIOSwitchRestoreMode.GPIO_SWITCH_RESTORE_DEFAULT_ON,
"ALWAYS_OFF": GPIOSwitchRestoreMode.GPIO_SWITCH_ALWAYS_OFF,
"ALWAYS_ON": GPIOSwitchRestoreMode.GPIO_SWITCH_ALWAYS_ON,
"RESTORE_INVERTED_DEFAULT_OFF": GPIOSwitchRestoreMode.GPIO_SWITCH_RESTORE_INVERTED_DEFAULT_OFF,
"RESTORE_INVERTED_DEFAULT_ON": GPIOSwitchRestoreMode.GPIO_SWITCH_RESTORE_INVERTED_DEFAULT_ON,
}
CONF_INTERLOCK_WAIT_TIME = "interlock_wait_time"
CONFIG_SCHEMA = (
@ -23,9 +13,6 @@ CONFIG_SCHEMA = (
.extend(
{
cv.Required(CONF_PIN): pins.gpio_output_pin_schema,
cv.Optional(CONF_RESTORE_MODE, default="RESTORE_DEFAULT_OFF"): cv.enum(
RESTORE_MODES, upper=True, space="_"
),
cv.Optional(CONF_INTERLOCK): cv.ensure_list(cv.use_id(switch.Switch)),
cv.Optional(
CONF_INTERLOCK_WAIT_TIME, default="0ms"
@ -43,8 +30,6 @@ async def to_code(config):
pin = await cg.gpio_pin_expression(config[CONF_PIN])
cg.add(var.set_pin(pin))
cg.add(var.set_restore_mode(config[CONF_RESTORE_MODE]))
if CONF_INTERLOCK in config:
interlock = []
for it in config[CONF_INTERLOCK]:

View file

@ -10,27 +10,7 @@ float GPIOSwitch::get_setup_priority() const { return setup_priority::HARDWARE;
void GPIOSwitch::setup() {
ESP_LOGCONFIG(TAG, "Setting up GPIO Switch '%s'...", this->name_.c_str());
bool initial_state = false;
switch (this->restore_mode_) {
case GPIO_SWITCH_RESTORE_DEFAULT_OFF:
initial_state = this->get_initial_state().value_or(false);
break;
case GPIO_SWITCH_RESTORE_DEFAULT_ON:
initial_state = this->get_initial_state().value_or(true);
break;
case GPIO_SWITCH_RESTORE_INVERTED_DEFAULT_OFF:
initial_state = !this->get_initial_state().value_or(true);
break;
case GPIO_SWITCH_RESTORE_INVERTED_DEFAULT_ON:
initial_state = !this->get_initial_state().value_or(false);
break;
case GPIO_SWITCH_ALWAYS_OFF:
initial_state = false;
break;
case GPIO_SWITCH_ALWAYS_ON:
initial_state = true;
break;
}
bool initial_state = Switch::get_initial_state_with_restore_mode();
// write state before setup
if (initial_state) {
@ -49,28 +29,6 @@ void GPIOSwitch::setup() {
void GPIOSwitch::dump_config() {
LOG_SWITCH("", "GPIO Switch", this);
LOG_PIN(" Pin: ", this->pin_);
const LogString *restore_mode = LOG_STR("");
switch (this->restore_mode_) {
case GPIO_SWITCH_RESTORE_DEFAULT_OFF:
restore_mode = LOG_STR("Restore (Defaults to OFF)");
break;
case GPIO_SWITCH_RESTORE_DEFAULT_ON:
restore_mode = LOG_STR("Restore (Defaults to ON)");
break;
case GPIO_SWITCH_RESTORE_INVERTED_DEFAULT_ON:
restore_mode = LOG_STR("Restore inverted (Defaults to ON)");
break;
case GPIO_SWITCH_RESTORE_INVERTED_DEFAULT_OFF:
restore_mode = LOG_STR("Restore inverted (Defaults to OFF)");
break;
case GPIO_SWITCH_ALWAYS_OFF:
restore_mode = LOG_STR("Always OFF");
break;
case GPIO_SWITCH_ALWAYS_ON:
restore_mode = LOG_STR("Always ON");
break;
}
ESP_LOGCONFIG(TAG, " Restore Mode: %s", LOG_STR_ARG(restore_mode));
if (!this->interlock_.empty()) {
ESP_LOGCONFIG(TAG, " Interlocks:");
for (auto *lock : this->interlock_) {
@ -111,7 +69,6 @@ void GPIOSwitch::write_state(bool state) {
this->pin_->digital_write(state);
this->publish_state(state);
}
void GPIOSwitch::set_restore_mode(GPIOSwitchRestoreMode restore_mode) { this->restore_mode_ = restore_mode; }
void GPIOSwitch::set_interlock(const std::vector<Switch *> &interlock) { this->interlock_ = interlock; }
} // namespace gpio

View file

@ -9,21 +9,10 @@
namespace esphome {
namespace gpio {
enum GPIOSwitchRestoreMode {
GPIO_SWITCH_RESTORE_DEFAULT_OFF,
GPIO_SWITCH_RESTORE_DEFAULT_ON,
GPIO_SWITCH_ALWAYS_OFF,
GPIO_SWITCH_ALWAYS_ON,
GPIO_SWITCH_RESTORE_INVERTED_DEFAULT_OFF,
GPIO_SWITCH_RESTORE_INVERTED_DEFAULT_ON,
};
class GPIOSwitch : public switch_::Switch, public Component {
public:
void set_pin(GPIOPin *pin) { pin_ = pin; }
void set_restore_mode(GPIOSwitchRestoreMode restore_mode);
// ========== INTERNAL METHODS ==========
// (In most use cases you won't need these)
float get_setup_priority() const override;
@ -37,7 +26,6 @@ class GPIOSwitch : public switch_::Switch, public Component {
void write_state(bool state) override;
GPIOPin *pin_;
GPIOSwitchRestoreMode restore_mode_{GPIO_SWITCH_RESTORE_DEFAULT_OFF};
std::vector<Switch *> interlock_;
uint32_t interlock_wait_time_{0};
};

View file

@ -32,7 +32,7 @@ ModbusSwitch = modbus_controller_ns.class_(
)
CONFIG_SCHEMA = cv.All(
switch.switch_schema(ModbusSwitch)
switch.switch_schema(ModbusSwitch, default_restore_mode="DISABLED")
.extend(cv.COMPONENT_SCHEMA)
.extend(ModbusItemBaseSchema)
.extend(

View file

@ -6,11 +6,7 @@ namespace modbus_controller {
static const char *const TAG = "modbus_controller.switch";
void ModbusSwitch::setup() {
// value isn't required
// without it we crash on save
this->get_initial_state();
}
void ModbusSwitch::setup() {}
void ModbusSwitch::dump_config() { LOG_SWITCH(TAG, "Modbus Controller Switch", this); }
void ModbusSwitch::parse_and_publish(const std::vector<uint8_t> &data) {

View file

@ -1,29 +1,19 @@
import esphome.codegen as cg
import esphome.config_validation as cv
from esphome.components import output, switch
from esphome.const import CONF_OUTPUT, CONF_RESTORE_MODE
from esphome.const import CONF_OUTPUT
from .. import output_ns
OutputSwitch = output_ns.class_("OutputSwitch", switch.Switch, cg.Component)
OutputSwitchRestoreMode = output_ns.enum("OutputSwitchRestoreMode")
RESTORE_MODES = {
"RESTORE_DEFAULT_OFF": OutputSwitchRestoreMode.OUTPUT_SWITCH_RESTORE_DEFAULT_OFF,
"RESTORE_DEFAULT_ON": OutputSwitchRestoreMode.OUTPUT_SWITCH_RESTORE_DEFAULT_ON,
"ALWAYS_OFF": OutputSwitchRestoreMode.OUTPUT_SWITCH_ALWAYS_OFF,
"ALWAYS_ON": OutputSwitchRestoreMode.OUTPUT_SWITCH_ALWAYS_ON,
"RESTORE_INVERTED_DEFAULT_OFF": OutputSwitchRestoreMode.OUTPUT_SWITCH_RESTORE_INVERTED_DEFAULT_OFF,
"RESTORE_INVERTED_DEFAULT_ON": OutputSwitchRestoreMode.OUTPUT_SWITCH_RESTORE_INVERTED_DEFAULT_ON,
}
CONFIG_SCHEMA = (
switch.switch_schema(OutputSwitch)
.extend(
{
cv.Required(CONF_OUTPUT): cv.use_id(output.BinaryOutput),
cv.Optional(CONF_RESTORE_MODE, default="RESTORE_DEFAULT_OFF"): cv.enum(
RESTORE_MODES, upper=True, space="_"
),
}
)
.extend(cv.COMPONENT_SCHEMA)
@ -36,5 +26,3 @@ async def to_code(config):
output_ = await cg.get_variable(config[CONF_OUTPUT])
cg.add(var.set_output(output_))
cg.add(var.set_restore_mode(config[CONF_RESTORE_MODE]))

View file

@ -8,27 +8,9 @@ static const char *const TAG = "output.switch";
void OutputSwitch::dump_config() { LOG_SWITCH("", "Output Switch", this); }
void OutputSwitch::setup() {
bool initial_state = false;
switch (this->restore_mode_) {
case OUTPUT_SWITCH_RESTORE_DEFAULT_OFF:
initial_state = this->get_initial_state().value_or(false);
break;
case OUTPUT_SWITCH_RESTORE_DEFAULT_ON:
initial_state = this->get_initial_state().value_or(true);
break;
case OUTPUT_SWITCH_RESTORE_INVERTED_DEFAULT_OFF:
initial_state = !this->get_initial_state().value_or(true);
break;
case OUTPUT_SWITCH_RESTORE_INVERTED_DEFAULT_ON:
initial_state = !this->get_initial_state().value_or(false);
break;
case OUTPUT_SWITCH_ALWAYS_OFF:
initial_state = false;
break;
case OUTPUT_SWITCH_ALWAYS_ON:
initial_state = true;
break;
}
ESP_LOGCONFIG(TAG, "Setting up Output Switch '%s'...", this->name_.c_str());
bool initial_state = Switch::get_initial_state_with_restore_mode();
if (initial_state) {
this->turn_on();

View file

@ -7,21 +7,10 @@
namespace esphome {
namespace output {
enum OutputSwitchRestoreMode {
OUTPUT_SWITCH_RESTORE_DEFAULT_OFF,
OUTPUT_SWITCH_RESTORE_DEFAULT_ON,
OUTPUT_SWITCH_ALWAYS_OFF,
OUTPUT_SWITCH_ALWAYS_ON,
OUTPUT_SWITCH_RESTORE_INVERTED_DEFAULT_OFF,
OUTPUT_SWITCH_RESTORE_INVERTED_DEFAULT_ON,
};
class OutputSwitch : public switch_::Switch, public Component {
public:
void set_output(BinaryOutput *output) { output_ = output; }
void set_restore_mode(OutputSwitchRestoreMode restore_mode) { restore_mode_ = restore_mode; }
void setup() override;
float get_setup_priority() const override { return setup_priority::HARDWARE - 1.0f; }
void dump_config() override;
@ -30,7 +19,6 @@ class OutputSwitch : public switch_::Switch, public Component {
void write_state(bool state) override;
output::BinaryOutput *output_;
OutputSwitchRestoreMode restore_mode_;
};
} // namespace output

View file

@ -12,6 +12,7 @@ from esphome.const import (
CONF_MQTT_ID,
CONF_ON_TURN_OFF,
CONF_ON_TURN_ON,
CONF_RESTORE_MODE,
CONF_TRIGGER_ID,
DEVICE_CLASS_EMPTY,
DEVICE_CLASS_OUTLET,
@ -33,6 +34,19 @@ switch_ns = cg.esphome_ns.namespace("switch_")
Switch = switch_ns.class_("Switch", cg.EntityBase)
SwitchPtr = Switch.operator("ptr")
SwitchRestoreMode = switch_ns.enum("SwitchRestoreMode")
RESTORE_MODES = {
"RESTORE_DEFAULT_OFF": SwitchRestoreMode.SWITCH_RESTORE_DEFAULT_OFF,
"RESTORE_DEFAULT_ON": SwitchRestoreMode.SWITCH_RESTORE_DEFAULT_ON,
"ALWAYS_OFF": SwitchRestoreMode.SWITCH_ALWAYS_OFF,
"ALWAYS_ON": SwitchRestoreMode.SWITCH_ALWAYS_ON,
"RESTORE_INVERTED_DEFAULT_OFF": SwitchRestoreMode.SWITCH_RESTORE_INVERTED_DEFAULT_OFF,
"RESTORE_INVERTED_DEFAULT_ON": SwitchRestoreMode.SWITCH_RESTORE_INVERTED_DEFAULT_ON,
"DISABLED": SwitchRestoreMode.SWITCH_RESTORE_DISABLED,
}
ToggleAction = switch_ns.class_("ToggleAction", automation.Action)
TurnOffAction = switch_ns.class_("TurnOffAction", automation.Action)
TurnOnAction = switch_ns.class_("TurnOnAction", automation.Action)
@ -50,7 +64,7 @@ SwitchTurnOffTrigger = switch_ns.class_(
validate_device_class = cv.one_of(*DEVICE_CLASSES, lower=True)
SWITCH_SCHEMA = cv.ENTITY_BASE_SCHEMA.extend(cv.MQTT_COMMAND_COMPONENT_SCHEMA).extend(
_SWITCH_SCHEMA = cv.ENTITY_BASE_SCHEMA.extend(cv.MQTT_COMMAND_COMPONENT_SCHEMA).extend(
{
cv.OnlyWith(CONF_MQTT_ID, "mqtt"): cv.declare_id(mqtt.MQTTSwitchComponent),
cv.Optional(CONF_INVERTED): cv.boolean,
@ -78,8 +92,15 @@ def switch_schema(
device_class: str = _UNDEF,
icon: str = _UNDEF,
block_inverted: bool = False,
default_restore_mode: str = "RESTORE_DEFAULT_OFF",
):
schema = SWITCH_SCHEMA
schema = _SWITCH_SCHEMA.extend(
{
cv.Optional(CONF_RESTORE_MODE, default=default_restore_mode): cv.enum(
RESTORE_MODES, upper=True, space="_"
),
}
)
if class_ is not _UNDEF:
schema = schema.extend({cv.GenerateID(): cv.declare_id(class_)})
if entity_category is not _UNDEF:
@ -111,6 +132,9 @@ def switch_schema(
return schema
SWITCH_SCHEMA = switch_schema() # for compatibility
async def setup_switch_core_(var, config):
await setup_entity(var, config)
@ -130,6 +154,8 @@ async def setup_switch_core_(var, config):
if CONF_DEVICE_CLASS in config:
cg.add(var.set_device_class(config[CONF_DEVICE_CLASS]))
cg.add(var.set_restore_mode(config[CONF_RESTORE_MODE]))
async def register_switch(var, config):
if not CORE.has_id(config[CONF_ID]):

View file

@ -22,18 +22,36 @@ void Switch::toggle() {
this->write_state(this->inverted_ == this->state);
}
optional<bool> Switch::get_initial_state() {
if (!(restore_mode & RESTORE_MODE_PERSISTENT_MASK))
return {};
this->rtc_ = global_preferences->make_preference<bool>(this->get_object_id_hash());
bool initial_state;
if (!this->rtc_.load(&initial_state))
return {};
return initial_state;
}
optional<bool> Switch::get_initial_state_with_restore_mode() {
if (restore_mode & RESTORE_MODE_DISABLED_MASK) {
return {};
}
bool initial_state = restore_mode & RESTORE_MODE_ON_MASK;
if (restore_mode & RESTORE_MODE_INVERTED_MASK)
initial_state = !initial_state;
if (restore_mode & RESTORE_MODE_PERSISTENT_MASK) {
initial_state = this->get_initial_state().value_or(initial_state);
}
return initial_state;
}
void Switch::publish_state(bool state) {
if (!this->publish_dedup_.next(state))
return;
this->state = state != this->inverted_;
this->rtc_.save(&this->state);
if (restore_mode & RESTORE_MODE_PERSISTENT_MASK)
this->rtc_.save(&this->state);
ESP_LOGD(TAG, "'%s': Sending state %s", this->name_.c_str(), ONOFF(this->state));
this->state_callback_.call(this->state);
}
@ -52,5 +70,30 @@ std::string Switch::get_device_class() {
}
void Switch::set_device_class(const std::string &device_class) { this->device_class_ = device_class; }
void log_switch(const char *tag, const char *prefix, const char *type, Switch *obj) {
if (obj != nullptr) {
ESP_LOGCONFIG(tag, "%s%s '%s'", prefix, type, obj->get_name().c_str());
if (!obj->get_icon().empty()) {
ESP_LOGCONFIG(tag, "%s Icon: '%s'", prefix, obj->get_icon().c_str());
}
if (obj->assumed_state()) {
ESP_LOGCONFIG(tag, "%s Assumed State: YES", prefix);
}
if (obj->is_inverted()) {
ESP_LOGCONFIG(tag, "%s Inverted: YES", prefix);
}
if (!obj->get_device_class().empty()) {
ESP_LOGCONFIG(tag, "%s Device Class: '%s'", prefix, obj->get_device_class().c_str());
}
const LogString *onoff = obj->restore_mode & RESTORE_MODE_ON_MASK ? LOG_STR("ON") : LOG_STR("OFF");
const LogString *inverted = obj->restore_mode & RESTORE_MODE_INVERTED_MASK ? LOG_STR("inverted ") : LOG_STR("");
const LogString *restore =
obj->restore_mode & RESTORE_MODE_PERSISTENT_MASK ? LOG_STR("restore defaults to") : LOG_STR("always");
ESP_LOGCONFIG(tag, "%s Restore Mode: %s%s %s", prefix, LOG_STR_ARG(inverted), LOG_STR_ARG(restore),
LOG_STR_ARG(onoff));
}
}
} // namespace switch_
} // namespace esphome

View file

@ -8,22 +8,21 @@
namespace esphome {
namespace switch_ {
#define LOG_SWITCH(prefix, type, obj) \
if ((obj) != nullptr) { \
ESP_LOGCONFIG(TAG, "%s%s '%s'", prefix, LOG_STR_LITERAL(type), (obj)->get_name().c_str()); \
if (!(obj)->get_icon().empty()) { \
ESP_LOGCONFIG(TAG, "%s Icon: '%s'", prefix, (obj)->get_icon().c_str()); \
} \
if ((obj)->assumed_state()) { \
ESP_LOGCONFIG(TAG, "%s Assumed State: YES", prefix); \
} \
if ((obj)->is_inverted()) { \
ESP_LOGCONFIG(TAG, "%s Inverted: YES", prefix); \
} \
if (!(obj)->get_device_class().empty()) { \
ESP_LOGCONFIG(TAG, "%s Device Class: '%s'", prefix, (obj)->get_device_class().c_str()); \
} \
}
// bit0: on/off. bit1: persistent. bit2: inverted. bit3: disabled
const int RESTORE_MODE_ON_MASK = 0x01;
const int RESTORE_MODE_PERSISTENT_MASK = 0x02;
const int RESTORE_MODE_INVERTED_MASK = 0x04;
const int RESTORE_MODE_DISABLED_MASK = 0x08;
enum SwitchRestoreMode {
SWITCH_ALWAYS_OFF = !RESTORE_MODE_ON_MASK,
SWITCH_ALWAYS_ON = RESTORE_MODE_ON_MASK,
SWITCH_RESTORE_DEFAULT_OFF = RESTORE_MODE_PERSISTENT_MASK,
SWITCH_RESTORE_DEFAULT_ON = RESTORE_MODE_PERSISTENT_MASK | RESTORE_MODE_ON_MASK,
SWITCH_RESTORE_INVERTED_DEFAULT_OFF = RESTORE_MODE_PERSISTENT_MASK | RESTORE_MODE_INVERTED_MASK,
SWITCH_RESTORE_INVERTED_DEFAULT_ON = RESTORE_MODE_PERSISTENT_MASK | RESTORE_MODE_INVERTED_MASK | RESTORE_MODE_ON_MASK,
SWITCH_RESTORE_DISABLED = RESTORE_MODE_DISABLED_MASK,
};
/** Base class for all switches.
*
@ -47,6 +46,9 @@ class Switch : public EntityBase {
/// The current reported state of the binary sensor.
bool state;
/// Indicates whether or not state is to be retrieved from flash and how
SwitchRestoreMode restore_mode{SWITCH_RESTORE_DEFAULT_OFF};
/** Turn this switch on. This is called by the front-end.
*
* For implementing switches, please override write_state.
@ -80,8 +82,19 @@ class Switch : public EntityBase {
*/
void add_on_state_callback(std::function<void(bool)> &&callback);
/** Returns the initial state of the switch, as persisted previously,
or empty if never persisted.
*/
optional<bool> get_initial_state();
/** Returns the initial state of the switch, after applying restore mode rules.
* If restore mode is disabled, this function will return an optional with no value
* (.has_value() is false), leaving it up to the component to decide the state.
* For example, the component could read the state from hardware and determine the current
* state.
*/
optional<bool> get_initial_state_with_restore_mode();
/** Return whether this switch uses an assumed state - i.e. if both the ON/OFF actions should be displayed in Home
* Assistant because the real state is unknown.
*
@ -95,6 +108,7 @@ class Switch : public EntityBase {
std::string get_device_class();
/// Set the Home Assistant device class for this switch.
void set_device_class(const std::string &device_class);
void set_restore_mode(SwitchRestoreMode restore_mode) { this->restore_mode = restore_mode; }
protected:
/** Write the given state to hardware. You should implement this
@ -114,5 +128,8 @@ class Switch : public EntityBase {
optional<std::string> device_class_;
};
#define LOG_SWITCH(prefix, type, obj) log_switch((TAG), (prefix), LOG_STR_LITERAL(type), (obj))
void log_switch(const char *tag, const char *prefix, const char *type, Switch *obj);
} // namespace switch_
} // namespace esphome