Refactor select entity (#32)

Fixes: #31

components/jnge_mppt_controller/jnge_mppt_controller.cpp

@@ -43,7 +43,7 @@ static const char *const OPERATION_MODES[OPERATION_MODES_SIZE] = {
 
 static const uint8_t BATTERY_TYPES_SIZE = 5;
 static const char *const BATTERY_TYPES[BATTERY_TYPES_SIZE] = {
-    "Lead-Acid",        // 0x00
+    "Lead Acid",        // 0x00
     "Gel",              // 0x01
     "Ternary Lithium",  // 0x02
     "LiFePo4",          // 0x03
@@ -256,7 +256,7 @@ void JngeMpptController::on_status_data_(const std::vector<uint8_t> &data) {
   // 0x1017: Number of battery strings  2 bytes    1
   this->publish_state_(this->battery_strings_sensor_, (float) jnge_get_16bit(46));
 
-  // 0x1018: Battery type               2 bytes                  0 (Lead-Acid), 1 (Gel), 2 (Ternary Lithium),
+  // 0x1018: Battery type               2 bytes                  0 (Lead Acid), 1 (Gel), 2 (Ternary Lithium),
   //                                                             3 (LiFePo4), 4 (Custom)
   uint16_t raw_battery_type = jnge_get_16bit(48);
   if (raw_battery_type < BATTERY_TYPES_SIZE) {
@@ -265,7 +265,11 @@ void JngeMpptController::on_status_data_(const std::vector<uint8_t> &data) {
     this->publish_state_(this->battery_type_text_sensor_, "Unknown");
   }
   if (this->battery_type_select_ != nullptr) {
-    this->battery_type_select_->map_and_publish(raw_battery_type);
+    for (auto &listener : this->select_listeners_) {
+      if (listener.holding_register == 0x103B) {
+        listener.on_value(raw_battery_type);
+      }
+    }
   }
 
   // 0x1019: Charging switch status     2 bytes                  0 (Charging off), 1 (Charging on)
@@ -424,7 +428,7 @@ void JngeMpptController::on_configuration_data_(const std::vector<uint8_t> &data
   // 0x103A: Number of battery strings        2 bytes
   this->publish_state_(this->battery_strings_number_, (float) jnge_get_16bit(44));
 
-  // 0x103B: Battery Type                     2 bytes                     0 (Lead-acid), 1 (gel), 2 (ternary lithium),
+  // 0x103B: Battery Type                     2 bytes                     0 (Lead Acid), 1 (Gel), 2 (Ternary Lithium),
   //                                                                      3 (LiFePo4), 4 (Custom)
   // this->publish_state_(this->battery_type_control_sensor_, (float) jnge_get_16bit(46));
 
@@ -485,6 +489,15 @@ void JngeMpptController::update() {
   }
 }
 
+void JngeMpptController::register_select_listener(uint16_t holding_register,
+                                                  const std::function<void(uint16_t)> &func) {
+  auto select_listener = JngeSelectListener{
+      .holding_register = holding_register,
+      .on_value = func,
+  };
+  this->select_listeners_.push_back(select_listener);
+}
+
 void JngeMpptController::publish_state_(binary_sensor::BinarySensor *binary_sensor, const bool &state) {
   if (binary_sensor == nullptr)
     return;

components/jnge_mppt_controller/jnge_mppt_controller.h

@@ -5,18 +5,20 @@
 #include "esphome/components/number/number.h"
 #include "esphome/components/sensor/sensor.h"
 #include "esphome/components/select/select.h"
-#include "esphome/components/jnge_mppt_controller/select/jnge_select.h"
 #include "esphome/components/switch/switch.h"
 #include "esphome/components/text_sensor/text_sensor.h"
 #include "esphome/components/jnge_modbus/jnge_modbus.h"
 
 namespace esphome {
 namespace jnge_mppt_controller {
 
-class JngeSelect;
-
 static const uint8_t NO_RESPONSE_THRESHOLD = 15;
 
+struct JngeSelectListener {
+  uint16_t holding_register;
+  std::function<void(uint16_t)> on_value;
+};
+
 class JngeMpptController : public PollingComponent, public jnge_modbus::JngeModbusDevice {
  public:
   void set_load_detected_binary_sensor(binary_sensor::BinarySensor *load_detected_binary_sensor) {
@@ -155,9 +157,7 @@ class JngeMpptController : public PollingComponent, public jnge_modbus::JngeModb
     load_turn_off_time_sensor_ = load_turn_off_time_sensor;
   }
 
-  void set_battery_type_select(jnge_mppt_controller::JngeSelect *battery_type_select) {
-    battery_type_select_ = battery_type_select;
-  }
+  void set_battery_type_select(select::Select *battery_type_select) { battery_type_select_ = battery_type_select; }
 
   void set_battery_overvoltage_number(number::Number *battery_overvoltage_number) {
     battery_overvoltage_number_ = battery_overvoltage_number;
@@ -240,6 +240,7 @@ class JngeMpptController : public PollingComponent, public jnge_modbus::JngeModb
   }
 
   void set_enable_fake_traffic(bool enable_fake_traffic) { enable_fake_traffic_ = enable_fake_traffic; }
+  void register_select_listener(uint16_t holding_register, const std::function<void(uint16_t)> &func);
 
   void dump_config() override;
 
@@ -300,7 +301,7 @@ class JngeMpptController : public PollingComponent, public jnge_modbus::JngeModb
   sensor::Sensor *light_control_on_period_2_sensor_;
   sensor::Sensor *load_turn_off_time_sensor_;
 
-  jnge_mppt_controller::JngeSelect *battery_type_select_;
+  select::Select *battery_type_select_;
 
   number::Number *battery_overvoltage_number_;
   number::Number *charging_limit_voltage_number_;
@@ -336,6 +337,8 @@ class JngeMpptController : public PollingComponent, public jnge_modbus::JngeModb
   bool suppress_battery_temperature_errors_;
   uint8_t no_response_count_ = 0;
 
+  std::vector<JngeSelectListener> select_listeners_;
+
   void on_status_data_(const std::vector<uint8_t> &data);
   void on_configuration_data_(const std::vector<uint8_t> &data);
   void on_write_data_(const std::vector<uint8_t> &data);

components/jnge_mppt_controller/select/__init__.py

@@ -19,22 +19,19 @@
 JngeSelect = jnge_mppt_controller_ns.class_("JngeSelect", select.Select, cg.Component)
 
 
-def ensure_option_map():
-    def validator(value):
-        cv.check_not_templatable(value)
-        option = cv.All(cv.string_strict)
-        mapping = cv.All(cv.int_range(0, 65535))
-        options_map_schema = cv.Schema({option: mapping})
-        value = options_map_schema(value)
+def ensure_option_map(value):
+    cv.check_not_templatable(value)
+    option = cv.All(cv.int_range(0, 2**16 - 1))
+    mapping = cv.All(cv.string_strict)
+    options_map_schema = cv.Schema({option: mapping})
+    value = options_map_schema(value)
 
-        all_values = list(value.values())
-        unique_values = set(value.values())
-        if len(all_values) != len(unique_values):
-            raise cv.Invalid("Mapping values must be unique.")
+    all_values = list(value.keys())
+    unique_values = set(value.keys())
+    if len(all_values) != len(unique_values):
+        raise cv.Invalid("Mapping values must be unique.")
 
-        return value
-
-    return validator
+    return value
 
 
 SELECTS = {
@@ -44,7 +41,7 @@ def validator(value):
 JNGESELECT_SCHEMA = select.SELECT_SCHEMA.extend(
     {
         cv.GenerateID(): cv.declare_id(JngeSelect),
-        cv.Optional(CONF_OPTIONSMAP): ensure_option_map(),
+        cv.Optional(CONF_OPTIONSMAP): ensure_option_map,
     }
 ).extend(cv.COMPONENT_SCHEMA)
 
@@ -63,9 +60,9 @@ async def to_code(config):
             options_map = conf[CONF_OPTIONSMAP]
             var = cg.new_Pvariable(conf[CONF_ID])
             await cg.register_component(var, conf)
-            await select.register_select(var, conf, options=list(options_map.keys()))
+            await select.register_select(var, conf, options=list(options_map.values()))
+            cg.add(var.set_select_mappings(list(options_map.keys())))
+
             cg.add(getattr(hub, f"set_{key}_select")(var))
             cg.add(var.set_parent(hub))
             cg.add(var.set_holding_register(address))
-            for mappingkey in options_map.keys():
-                cg.add(var.add_mapping(mappingkey, options_map[mappingkey]))

components/jnge_mppt_controller/select/jnge_select.cpp

@@ -6,37 +6,41 @@ namespace jnge_mppt_controller {
 
 static const char *const TAG = "jnge_mppt_controller.select";
 
+void JngeSelect::setup() {
+  this->parent_->register_select_listener(this->holding_register_, [this](const uint16_t &enum_value) {
+    ESP_LOGV(TAG, "Device reported select %u value %u", this->holding_register_, enum_value);
+    auto mappings = this->mappings_;
+    auto it = std::find(mappings.cbegin(), mappings.cend(), enum_value);
+    if (it == mappings.end()) {
+      ESP_LOGW(TAG, "Invalid value %u", enum_value);
+      return;
+    }
+    size_t mapping_idx = std::distance(mappings.cbegin(), it);
+    auto value = this->at(mapping_idx);
+    this->publish_state(value.value());
+  });
+}
+
 void JngeSelect::dump_config() {
   LOG_SELECT(TAG, "JngeMpptController Select", this);
+  ESP_LOGCONFIG(TAG, "  Select has register %u", this->holding_register_);
   ESP_LOGCONFIG(TAG, "  Options are:");
-  for (auto const &map : this->mapping_) {
-    ESP_LOGCONFIG(TAG, "    %s: %d", map.first.c_str(), map.second);
-  }
-}
-
-void JngeSelect::map_and_publish(uint16_t &value) {
-  ESP_LOGD(TAG, "Got value: %d", value);
-
-  for (auto const &map : this->mapping_) {
-    if (map.second == value) {
-      ESP_LOGD(TAG, "Found mapped option %s of value %d", map.first.c_str(), value);
-      this->publish_state(map.first);
-      return;
-    }
+  auto options = this->traits.get_options();
+  for (auto i = 0; i < this->mappings_.size(); i++) {
+    ESP_LOGCONFIG(TAG, "    %i: %s", this->mappings_.at(i), options.at(i).c_str());
   }
-
-  ESP_LOGD(TAG, "Could not find value %d in mapping", value);
 }
 
 void JngeSelect::control(const std::string &value) {
-  ESP_LOGD(TAG, "Got option: %s", value.c_str());
-  if (this->mapping_.find(value) != this->mapping_.end()) {
-    ESP_LOGD(TAG, "Found mapped value %d of option %s", this->mapping_[value], value.c_str());
-    this->parent_->write_register(this->holding_register_, this->mapping_[value]);
-  } else {
-    ESP_LOGD(TAG, "Could not find option %s in mapping", value.c_str());
+  auto idx = this->index_of(value);
+  if (idx.has_value()) {
+    uint16_t mapping = this->mappings_.at(idx.value());
+    ESP_LOGV(TAG, "Setting %u datapoint value to %u:%s", this->holding_register_, mapping, value.c_str());
+    this->parent_->write_register(this->holding_register_, mapping);
     return;
   }
+
+  ESP_LOGW(TAG, "Invalid value %s", value.c_str());
 }
 
 }  // namespace jnge_mppt_controller

components/jnge_mppt_controller/select/jnge_select.h

@@ -16,14 +16,14 @@ class JngeSelect : public Component, public select::Select {
  public:
   void set_parent(JngeMpptController *const parent) { this->parent_ = parent; }
   void set_holding_register(uint16_t holding_register) { this->holding_register_ = holding_register; };
-  void add_mapping(const std::string &key, const uint16_t &value) { this->mapping_[key] = value; }
+  void set_select_mappings(std::vector<uint16_t> mappings) { this->mappings_ = std::move(mappings); }
 
+  void setup() override;
   void dump_config() override;
-  void map_and_publish(uint16_t &value);
   void control(const std::string &value) override;
 
  protected:
-  std::map<std::string, uint16_t> mapping_;
+  std::vector<uint16_t> mappings_;
   JngeMpptController *parent_;
   uint16_t holding_register_;
 };

esp32-example-jnge-mppt-controller.yaml

@@ -205,8 +205,8 @@ select:
     battery_type:
       name: "${config} battery type"
       optionsmap:
-        "Lead Acid": 0
-        "Gel": 1
-        "Ternary Lithium": 2
-        "LiFePo4": 3
-        "Custom": 4
+        0: "Lead Acid"
+        1: "Gel"
+        2: "Ternary Lithium"
+        3: "LiFePo4"
+        4: "Custom"

esp8266-example-jnge-mppt-controller.yaml

@@ -201,8 +201,8 @@ select:
     battery_type:
       name: "${config} battery type"
       optionsmap:
-        "Lead Acid": 0
-        "Gel": 1
-        "Ternary Lithium": 2
-        "LiFePo4": 3
-        "Custom": 4
+        0: "Lead Acid"
+        1: "Gel"
+        2: "Ternary Lithium"
+        3: "LiFePo4"
+        4: "Custom"