Code cleanup fixes for the select component (#3457)

Co-authored-by: Maurice Makaay <mmakaay1@xs4all.net>

esphome/components/select/select.cpp

@@ -23,6 +23,10 @@ void Select::add_on_state_callback(std::function<void(std::string, size_t)> &&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<size_t> Select::active_index() const {
 }
 
 optional<std::string> Select::at(size_t index) const {
+  if (this->has_index(index)) {
     auto options = traits.get_options();
-  if (index >= options.size()) {
+    return options.at(index);
+  } else {
     return {};
   }
-  return options.at(index);
 }
 
 uint32_t Select::hash_base() { return 2812997003UL; }

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<size_t> index_of(const std::string &option) const;
+
+  /// Return the (optional) index offset of the currently active option.
   optional<size_t> active_index() const;
+
+  /// Return the (optional) option value at the provided index offset.
   optional<std::string> at(size_t index) const;
 
   void add_on_state_callback(std::function<void(std::string, size_t)> &&callback);

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<std::string> &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); }

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<std::string> &get_option() const;
 
   SelectCall &select_next(bool cycle);
   SelectCall &select_previous(bool cycle);

esphome/components/template/select/template_select.cpp

@@ -20,9 +20,12 @@ void TemplateSelect::setup() {
     this->pref_ = global_preferences->make_preference<size_t>(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 (!this->has_option(*val)) {
-  if (std::find(options.begin(), options.end(), *val) == options.end()) {
+    ESP_LOGE(TAG, "Lambda returned an invalid option: %s", (*val).c_str());
-    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();
+    auto index = this->index_of(value);
-    size_t index = std::find(options.begin(), options.end(), value) - options.begin();
+    this->pref_.save(&index.value());
-
-    this->pref_.save(&index);
   }
 }
+
 void TemplateSelect::dump_config() {
   LOG_SELECT("", "Template Select", this);
   LOG_UPDATE_INTERVAL(this);