1
0
mirror of https://github.com/esphome/esphome.git synced 2026-02-08 00:31:58 +00:00

[fan] Return StringRef from get_preset_mode() for safety and modern API (#13092)

This commit is contained in:
J. Nick Koston
2026-01-11 17:40:09 -10:00
committed by GitHub
parent 45c0796e40
commit eeeae53f76
11 changed files with 113 additions and 32 deletions

View File

@@ -443,7 +443,7 @@ uint16_t APIConnection::try_send_fan_state(EntityBase *entity, APIConnection *co
if (traits.supports_direction()) if (traits.supports_direction())
msg.direction = static_cast<enums::FanDirection>(fan->direction); msg.direction = static_cast<enums::FanDirection>(fan->direction);
if (traits.supports_preset_modes() && fan->has_preset_mode()) if (traits.supports_preset_modes() && fan->has_preset_mode())
msg.preset_mode = StringRef(fan->get_preset_mode()); msg.preset_mode = fan->get_preset_mode();
return fill_and_encode_entity_state(fan, msg, FanStateResponse::MESSAGE_TYPE, conn, remaining_size, is_single); return fill_and_encode_entity_state(fan, msg, FanStateResponse::MESSAGE_TYPE, conn, remaining_size, is_single);
} }
uint16_t APIConnection::try_send_fan_info(EntityBase *entity, APIConnection *conn, uint32_t remaining_size, uint16_t APIConnection::try_send_fan_info(EntityBase *entity, APIConnection *conn, uint32_t remaining_size,

View File

@@ -8,20 +8,24 @@ static const char *const TAG = "copy.fan";
void CopyFan::setup() { void CopyFan::setup() {
source_->add_on_state_callback([this]() { source_->add_on_state_callback([this]() {
this->state = source_->state; this->copy_state_from_source_();
this->oscillating = source_->oscillating;
this->speed = source_->speed;
this->direction = source_->direction;
this->set_preset_mode_(source_->get_preset_mode());
this->publish_state(); this->publish_state();
}); });
this->copy_state_from_source_();
this->publish_state();
}
void CopyFan::copy_state_from_source_() {
this->state = source_->state; this->state = source_->state;
this->oscillating = source_->oscillating; this->oscillating = source_->oscillating;
this->speed = source_->speed; this->speed = source_->speed;
this->direction = source_->direction; this->direction = source_->direction;
this->set_preset_mode_(source_->get_preset_mode()); if (source_->has_preset_mode()) {
this->publish_state(); this->set_preset_mode_(source_->get_preset_mode());
} else {
this->clear_preset_mode_();
}
} }
void CopyFan::dump_config() { LOG_FAN("", "Copy Fan", this); } void CopyFan::dump_config() { LOG_FAN("", "Copy Fan", this); }

View File

@@ -16,7 +16,7 @@ class CopyFan : public fan::Fan, public Component {
protected: protected:
void control(const fan::FanCall &call) override; void control(const fan::FanCall &call) override;
; void copy_state_from_source_();
fan::Fan *source_; fan::Fan *source_;
}; };

View File

@@ -212,19 +212,18 @@ class FanPresetSetTrigger : public Trigger<std::string> {
public: public:
FanPresetSetTrigger(Fan *state) { FanPresetSetTrigger(Fan *state) {
state->add_on_state_callback([this, state]() { state->add_on_state_callback([this, state]() {
const auto *preset_mode = state->get_preset_mode(); auto preset_mode = state->get_preset_mode();
auto should_trigger = preset_mode != this->last_preset_mode_; auto should_trigger = preset_mode != this->last_preset_mode_;
this->last_preset_mode_ = preset_mode; this->last_preset_mode_ = preset_mode;
if (should_trigger) { if (should_trigger) {
// Trigger with empty string when nullptr to maintain backward compatibility this->trigger(std::string(preset_mode));
this->trigger(preset_mode != nullptr ? preset_mode : "");
} }
}); });
this->last_preset_mode_ = state->get_preset_mode(); this->last_preset_mode_ = state->get_preset_mode();
} }
protected: protected:
const char *last_preset_mode_{nullptr}; StringRef last_preset_mode_{};
}; };
} // namespace fan } // namespace fan

View File

@@ -61,7 +61,7 @@ void FanCall::perform() {
if (this->direction_.has_value()) { if (this->direction_.has_value()) {
ESP_LOGD(TAG, " Direction: %s", LOG_STR_ARG(fan_direction_to_string(*this->direction_))); ESP_LOGD(TAG, " Direction: %s", LOG_STR_ARG(fan_direction_to_string(*this->direction_)));
} }
if (this->has_preset_mode()) { if (this->preset_mode_ != nullptr) {
ESP_LOGD(TAG, " Preset Mode: %s", this->preset_mode_); ESP_LOGD(TAG, " Preset Mode: %s", this->preset_mode_);
} }
this->parent_.control(*this); this->parent_.control(*this);
@@ -83,7 +83,7 @@ void FanCall::validate_() {
*this->binary_state_ *this->binary_state_
// ..,and no preset mode will be active... // ..,and no preset mode will be active...
&& !this->has_preset_mode() && && !this->has_preset_mode() &&
this->parent_.get_preset_mode() == nullptr !this->parent_.has_preset_mode()
// ...and neither current nor new speed is available... // ...and neither current nor new speed is available...
&& traits.supports_speed() && this->parent_.speed == 0 && !this->speed_.has_value()) { && traits.supports_speed() && this->parent_.speed == 0 && !this->speed_.has_value()) {
// ...set speed to 100% // ...set speed to 100%
@@ -154,16 +154,16 @@ const char *Fan::find_preset_mode_(const char *preset_mode, size_t len) {
return this->get_traits().find_preset_mode(preset_mode, len); return this->get_traits().find_preset_mode(preset_mode, len);
} }
bool Fan::set_preset_mode_(const char *preset_mode) { bool Fan::set_preset_mode_(const char *preset_mode, size_t len) {
if (preset_mode == nullptr) { if (preset_mode == nullptr || len == 0) {
// Treat nullptr as clearing the preset mode // Treat nullptr or empty string as clearing the preset mode (no valid preset is "")
if (this->preset_mode_ == nullptr) { if (this->preset_mode_ == nullptr) {
return false; // No change return false; // No change
} }
this->clear_preset_mode_(); this->clear_preset_mode_();
return true; return true;
} }
const char *validated = this->find_preset_mode_(preset_mode); const char *validated = this->find_preset_mode_(preset_mode, len);
if (validated == nullptr || this->preset_mode_ == validated) { if (validated == nullptr || this->preset_mode_ == validated) {
return false; // Preset mode not supported or no change return false; // Preset mode not supported or no change
} }
@@ -171,10 +171,31 @@ bool Fan::set_preset_mode_(const char *preset_mode) {
return true; return true;
} }
bool Fan::set_preset_mode_(const std::string &preset_mode) { return this->set_preset_mode_(preset_mode.c_str()); } bool Fan::set_preset_mode_(const char *preset_mode) {
return this->set_preset_mode_(preset_mode, preset_mode ? strlen(preset_mode) : 0);
}
bool Fan::set_preset_mode_(const std::string &preset_mode) {
return this->set_preset_mode_(preset_mode.data(), preset_mode.size());
}
bool Fan::set_preset_mode_(StringRef preset_mode) {
// Safe: find_preset_mode_ only uses the input for comparison and returns
// a pointer from traits, so the input StringRef's lifetime doesn't matter.
return this->set_preset_mode_(preset_mode.c_str(), preset_mode.size());
}
void Fan::clear_preset_mode_() { this->preset_mode_ = nullptr; } void Fan::clear_preset_mode_() { this->preset_mode_ = nullptr; }
void Fan::apply_preset_mode_(const FanCall &call) {
if (call.has_preset_mode()) {
this->set_preset_mode_(call.get_preset_mode());
} else if (call.get_speed().has_value()) {
// Manually setting speed clears preset (per Home Assistant convention)
this->clear_preset_mode_();
}
}
void Fan::add_on_state_callback(std::function<void()> &&callback) { this->state_callback_.add(std::move(callback)); } void Fan::add_on_state_callback(std::function<void()> &&callback) { this->state_callback_.add(std::move(callback)); }
void Fan::publish_state() { void Fan::publish_state() {
auto traits = this->get_traits(); auto traits = this->get_traits();
@@ -192,9 +213,8 @@ void Fan::publish_state() {
if (traits.supports_direction()) { if (traits.supports_direction()) {
ESP_LOGD(TAG, " Direction: %s", LOG_STR_ARG(fan_direction_to_string(this->direction))); ESP_LOGD(TAG, " Direction: %s", LOG_STR_ARG(fan_direction_to_string(this->direction)));
} }
const char *preset = this->get_preset_mode(); if (this->preset_mode_ != nullptr) {
if (preset != nullptr) { ESP_LOGD(TAG, " Preset Mode: %s", this->preset_mode_);
ESP_LOGD(TAG, " Preset Mode: %s", preset);
} }
this->state_callback_.call(); this->state_callback_.call();
#if defined(USE_FAN) && defined(USE_CONTROLLER_REGISTRY) #if defined(USE_FAN) && defined(USE_CONTROLLER_REGISTRY)
@@ -249,12 +269,11 @@ void Fan::save_state_() {
state.speed = this->speed; state.speed = this->speed;
state.direction = this->direction; state.direction = this->direction;
const char *preset = this->get_preset_mode(); if (this->has_preset_mode()) {
if (preset != nullptr) {
const auto &preset_modes = traits.supported_preset_modes(); const auto &preset_modes = traits.supported_preset_modes();
// Find index of current preset mode (pointer comparison is safe since preset is from traits) // Find index of current preset mode (pointer comparison is safe since preset is from traits)
for (size_t i = 0; i < preset_modes.size(); i++) { for (size_t i = 0; i < preset_modes.size(); i++) {
if (preset_modes[i] == preset) { if (preset_modes[i] == this->preset_mode_) {
state.preset_mode = i; state.preset_mode = i;
break; break;
} }

View File

@@ -5,6 +5,7 @@
#include "esphome/core/log.h" #include "esphome/core/log.h"
#include "esphome/core/optional.h" #include "esphome/core/optional.h"
#include "esphome/core/preferences.h" #include "esphome/core/preferences.h"
#include "esphome/core/string_ref.h"
#include "fan_traits.h" #include "fan_traits.h"
namespace esphome { namespace esphome {
@@ -128,8 +129,11 @@ class Fan : public EntityBase {
/// Set the restore mode of this fan. /// Set the restore mode of this fan.
void set_restore_mode(FanRestoreMode restore_mode) { this->restore_mode_ = restore_mode; } void set_restore_mode(FanRestoreMode restore_mode) { this->restore_mode_ = restore_mode; }
/// Get the current preset mode (returns pointer to string stored in traits, or nullptr if not set) /// Get the current preset mode.
const char *get_preset_mode() const { return this->preset_mode_; } /// Returns a StringRef of the string stored in traits, or empty ref if not set.
/// The returned ref points to string literals from codegen (static storage).
/// Traits are set once at startup and valid for the lifetime of the program.
StringRef get_preset_mode() const { return StringRef::from_maybe_nullptr(this->preset_mode_); }
/// Check if a preset mode is currently active /// Check if a preset mode is currently active
bool has_preset_mode() const { return this->preset_mode_ != nullptr; } bool has_preset_mode() const { return this->preset_mode_ != nullptr; }
@@ -146,11 +150,15 @@ class Fan : public EntityBase {
void dump_traits_(const char *tag, const char *prefix); void dump_traits_(const char *tag, const char *prefix);
/// Set the preset mode (finds and stores pointer from traits). Returns true if changed. /// Set the preset mode (finds and stores pointer from traits). Returns true if changed.
/// Passing nullptr or empty string clears the preset mode.
bool set_preset_mode_(const char *preset_mode, size_t len);
bool set_preset_mode_(const char *preset_mode); bool set_preset_mode_(const char *preset_mode);
/// Set the preset mode (finds and stores pointer from traits). Returns true if changed.
bool set_preset_mode_(const std::string &preset_mode); bool set_preset_mode_(const std::string &preset_mode);
bool set_preset_mode_(StringRef preset_mode);
/// Clear the preset mode /// Clear the preset mode
void clear_preset_mode_(); void clear_preset_mode_();
/// Apply preset mode from a FanCall (handles speed-clears-preset convention)
void apply_preset_mode_(const FanCall &call);
/// Find and return the matching preset mode pointer from traits, or nullptr if not found. /// Find and return the matching preset mode pointer from traits, or nullptr if not found.
const char *find_preset_mode_(const char *preset_mode); const char *find_preset_mode_(const char *preset_mode);
const char *find_preset_mode_(const char *preset_mode, size_t len); const char *find_preset_mode_(const char *preset_mode, size_t len);

View File

@@ -57,7 +57,7 @@ void HBridgeFan::control(const fan::FanCall &call) {
this->oscillating = *call.get_oscillating(); this->oscillating = *call.get_oscillating();
if (call.get_direction().has_value()) if (call.get_direction().has_value())
this->direction = *call.get_direction(); this->direction = *call.get_direction();
this->set_preset_mode_(call.get_preset_mode()); this->apply_preset_mode_(call);
this->write_state_(); this->write_state_();
this->publish_state(); this->publish_state();

View File

@@ -29,7 +29,7 @@ void SpeedFan::control(const fan::FanCall &call) {
this->oscillating = *call.get_oscillating(); this->oscillating = *call.get_oscillating();
if (call.get_direction().has_value()) if (call.get_direction().has_value())
this->direction = *call.get_direction(); this->direction = *call.get_direction();
this->set_preset_mode_(call.get_preset_mode()); this->apply_preset_mode_(call);
this->write_state_(); this->write_state_();
this->publish_state(); this->publish_state();

View File

@@ -28,7 +28,7 @@ void TemplateFan::control(const fan::FanCall &call) {
this->oscillating = *call.get_oscillating(); this->oscillating = *call.get_oscillating();
if (call.get_direction().has_value() && this->has_direction_) if (call.get_direction().has_value() && this->has_direction_)
this->direction = *call.get_direction(); this->direction = *call.get_direction();
this->set_preset_mode_(call.get_preset_mode()); this->apply_preset_mode_(call);
this->publish_state(); this->publish_state();
} }

View File

@@ -7,6 +7,9 @@ fan:
- platform: speed - platform: speed
id: fan_speed id: fan_speed
output: fan_output_1 output: fan_output_1
preset_modes:
- Eco
- Turbo
- platform: copy - platform: copy
source_id: fan_speed source_id: fan_speed
name: Fan Speed Copy name: Fan Speed Copy

View File

@@ -9,3 +9,51 @@ fan:
has_oscillating: true has_oscillating: true
has_direction: true has_direction: true
speed_count: 3 speed_count: 3
# Test lambdas using get_preset_mode() which returns StringRef
# These examples match the migration guide in the PR description
binary_sensor:
- platform: template
id: fan_has_preset
name: "Fan Has Preset"
lambda: |-
// Migration guide: Checking if preset mode is set
// Use empty() or has_preset_mode()
if (!id(test_fan).get_preset_mode().empty()) {
// preset is set
}
if (id(test_fan).has_preset_mode()) {
// preset is set
}
// Migration guide: Comparing preset mode
// Use == operator directly (safe, works even when empty)
if (id(test_fan).get_preset_mode() == "Eco") {
return true;
}
// Migration guide: Checking for no preset
if (id(test_fan).get_preset_mode().empty()) {
// no preset
}
if (!id(test_fan).has_preset_mode()) {
// no preset
}
// Migration guide: Getting as std::string
std::string preset = std::string(id(test_fan).get_preset_mode());
// Migration guide: Logging option 1
// Use .c_str() - works because StringRef points to null-terminated string in traits
ESP_LOGD("test", "Preset: %s", id(test_fan).get_preset_mode().c_str());
// Migration guide: Logging option 2
// Use %.*s format (safer, no null-termination assumption)
auto preset_ref = id(test_fan).get_preset_mode();
ESP_LOGD("test", "Preset: %.*s", (int)preset_ref.size(), preset_ref.c_str());
// Test != comparison
if (id(test_fan).get_preset_mode() != "Sleep") {
return true;
}
return false;