From 62f9e181e03e6f3ae1aea18833b686b069fb0680 Mon Sep 17 00:00:00 2001 From: Maurice Makaay Date: Wed, 11 May 2022 00:58:28 +0200 Subject: [PATCH] Code cleanup fixes for the select component (#3457) Co-authored-by: Maurice Makaay --- esphome/components/select/select.cpp | 11 ++++++++--- esphome/components/select/select.h | 18 +++++++++++++++--- esphome/components/select/select_call.cpp | 2 -- esphome/components/select/select_call.h | 1 - .../template/select/template_select.cpp | 19 ++++++++++--------- 5 files changed, 33 insertions(+), 18 deletions(-) diff --git a/esphome/components/select/select.cpp b/esphome/components/select/select.cpp index 75edb5c8ba..35f1cfba46 100644 --- a/esphome/components/select/select.cpp +++ b/esphome/components/select/select.cpp @@ -23,6 +23,10 @@ void Select::add_on_state_callback(std::function &&ca this->state_callback_.add(std::move(callback)); } +bool Select::has_option(const std::string &option) const { return this->index_of(option).has_value(); } + +bool Select::has_index(size_t index) const { return index < this->size(); } + size_t Select::size() const { auto options = traits.get_options(); return options.size(); @@ -46,11 +50,12 @@ optional Select::active_index() const { } optional Select::at(size_t index) const { - auto options = traits.get_options(); - if (index >= options.size()) { + if (this->has_index(index)) { + auto options = traits.get_options(); + return options.at(index); + } else { return {}; } - return options.at(index); } uint32_t Select::hash_base() { return 2812997003UL; } diff --git a/esphome/components/select/select.h b/esphome/components/select/select.h index 64870fc9a3..b11c6404a0 100644 --- a/esphome/components/select/select.h +++ b/esphome/components/select/select.h @@ -28,16 +28,28 @@ class Select : public EntityBase { void publish_state(const std::string &state); - /// Return whether this select has gotten a full state yet. + /// Return whether this select component has gotten a full state yet. bool has_state() const { return has_state_; } + /// Instantiate a SelectCall object to modify this select component's state. SelectCall make_call() { return SelectCall(this); } - void set(const std::string &value) { make_call().set_option(value).perform(); } - // Methods that provide an API to index-based access. + /// Return whether this select component contains the provided option. + bool has_option(const std::string &option) const; + + /// Return whether this select component contains the provided index offset. + bool has_index(size_t index) const; + + /// Return the number of options in this select component. size_t size() const; + + /// Find the (optional) index offset of the provided option value. optional index_of(const std::string &option) const; + + /// Return the (optional) index offset of the currently active option. optional active_index() const; + + /// Return the (optional) option value at the provided index offset. optional at(size_t index) const; void add_on_state_callback(std::function &&callback); diff --git a/esphome/components/select/select_call.cpp b/esphome/components/select/select_call.cpp index 9442598740..6ee41b1029 100644 --- a/esphome/components/select/select_call.cpp +++ b/esphome/components/select/select_call.cpp @@ -13,8 +13,6 @@ SelectCall &SelectCall::set_option(const std::string &option) { SelectCall &SelectCall::set_index(size_t index) { return with_operation(SELECT_OP_SET_INDEX).with_index(index); } -const optional &SelectCall::get_option() const { return option_; } - SelectCall &SelectCall::select_next(bool cycle) { return with_operation(SELECT_OP_NEXT).with_cycle(cycle); } SelectCall &SelectCall::select_previous(bool cycle) { return with_operation(SELECT_OP_PREVIOUS).with_cycle(cycle); } diff --git a/esphome/components/select/select_call.h b/esphome/components/select/select_call.h index ea4d34ab5f..efc9a982ec 100644 --- a/esphome/components/select/select_call.h +++ b/esphome/components/select/select_call.h @@ -24,7 +24,6 @@ class SelectCall { SelectCall &set_option(const std::string &option); SelectCall &set_index(size_t index); - const optional &get_option() const; SelectCall &select_next(bool cycle); SelectCall &select_previous(bool cycle); diff --git a/esphome/components/template/select/template_select.cpp b/esphome/components/template/select/template_select.cpp index 219c341ec9..88a0d66ed6 100644 --- a/esphome/components/template/select/template_select.cpp +++ b/esphome/components/template/select/template_select.cpp @@ -20,9 +20,12 @@ void TemplateSelect::setup() { this->pref_ = global_preferences->make_preference(this->get_object_id_hash()); if (!this->pref_.load(&index)) { value = this->initial_option_; - ESP_LOGD(TAG, "State from initial (could not load): %s", value.c_str()); + ESP_LOGD(TAG, "State from initial (could not load stored index): %s", value.c_str()); + } else if (!this->has_index(index)) { + value = this->initial_option_; + ESP_LOGD(TAG, "State from initial (restored index %d out of bounds): %s", index, value.c_str()); } else { - value = this->traits.get_options().at(index); + value = this->at(index).value(); ESP_LOGD(TAG, "State from restore: %s", value.c_str()); } } @@ -38,9 +41,8 @@ void TemplateSelect::update() { if (!val.has_value()) return; - auto options = this->traits.get_options(); - if (std::find(options.begin(), options.end(), *val) == options.end()) { - ESP_LOGE(TAG, "lambda returned an invalid option %s", (*val).c_str()); + if (!this->has_option(*val)) { + ESP_LOGE(TAG, "Lambda returned an invalid option: %s", (*val).c_str()); return; } @@ -54,12 +56,11 @@ void TemplateSelect::control(const std::string &value) { this->publish_state(value); if (this->restore_value_) { - auto options = this->traits.get_options(); - size_t index = std::find(options.begin(), options.end(), value) - options.begin(); - - this->pref_.save(&index); + auto index = this->index_of(value); + this->pref_.save(&index.value()); } } + void TemplateSelect::dump_config() { LOG_SELECT("", "Template Select", this); LOG_UPDATE_INTERVAL(this);