Merge pull request #2235 from MichaelDvP/dev

modbus fixes

CHANGELOG_LATEST.md

@@ -26,6 +26,7 @@ For more details go to [docs.emsesp.org](https://docs.emsesp.org/).
 - heatpump cost UOMs [#2188](https://github.com/emsesp/EMS-ESP32/issues/2188)
 - analog dac output and inputs on dac pins [#2201](https://github.com/emsesp/EMS-ESP32/discussions/2201)
 - api memory leak [#2216](https://github.com/emsesp/EMS-ESP32/issues/2216)
+- modbus multiple mixers [#2229](https://github.com/emsesp/EMS-ESP32/issues/2229)
 
 ## Changed
 

src/device_library.h

@@ -187,4 +187,16 @@
 // Generic - 0x40 or other with no product-id and no version
 {0, DeviceType::GENERIC, "unknown", DeviceFlags::EMS_DEVICE_FLAG_NONE}
 
+#if defined(EMSESP_STANDALONE)
+,
+{100, DeviceType::WATER, "IPM", DeviceFlags::EMS_DEVICE_FLAG_IPM},
+{102, DeviceType::WATER, "IPM", DeviceFlags::EMS_DEVICE_FLAG_IPM},
+{160, DeviceType::WATER, "MM100", DeviceFlags::EMS_DEVICE_FLAG_MMPLUS},
+{161, DeviceType::WATER, "MM200", DeviceFlags::EMS_DEVICE_FLAG_MMPLUS},
+{163, DeviceType::WATER, "SM100, MS100", DeviceFlags::EMS_DEVICE_FLAG_SM100},
+{164, DeviceType::WATER, "SM200, MS200", DeviceFlags::EMS_DEVICE_FLAG_SM100},
+{248, DeviceType::MIXER, "HM210", DeviceFlags::EMS_DEVICE_FLAG_MMPLUS},
+{157, DeviceType::THERMOSTAT, "RC120", DeviceFlags::EMS_DEVICE_FLAG_CR120}
+#endif
+
 // clang-format on

src/emsdevice.cpp

@@ -1928,7 +1928,7 @@ const char * EMSdevice::telegram_type_name(std::shared_ptr<const Telegram> teleg
 bool EMSdevice::handle_telegram(std::shared_ptr<const Telegram> telegram) {
     for (auto & tf : telegram_functions_) {
         if (tf.telegram_type_id_ == telegram->type_id) {
-            // for telegram desitnation only read telegram
+            // for telegram destination only read telegram
             if (telegram->dest == device_id_ && telegram->message_length > 0) {
                 tf.process_function_(telegram);
                 return true;
@@ -1996,8 +1996,9 @@ int EMSdevice::get_modbus_value(uint8_t tag, const std::string & shortname, std:
     // find device value by shortname
     // TODO replace linear search which is inefficient
     const auto & it = std::find_if(devicevalues_.begin(), devicevalues_.end(), [&](const DeviceValue & x) { return x.tag == tag && x.short_name == shortname; });
-    if (it == devicevalues_.end())
+    if (it == devicevalues_.end() && (it->short_name != shortname || it->tag != tag)) {
         return -1;
+    }
 
     auto & dv = *it;
 
@@ -2082,7 +2083,7 @@ int EMSdevice::modbus_value_to_json(uint8_t tag, const std::string & shortname,
 
     // find device value by shortname
     const auto & it = std::find_if(devicevalues_.begin(), devicevalues_.end(), [&](const DeviceValue & x) { return x.tag == tag && x.short_name == shortname; });
-    if (it == devicevalues_.end()) {
+    if (it == devicevalues_.end() && (it->short_name != shortname || it->tag != tag)) {
         return -1;
     }
 

src/emsesp.cpp

@@ -343,17 +343,14 @@ void EMSESP::dump_all_entities(uuid::console::Shell & shell) {
         for (const auto & device : device_library_) {
             if (device_class.first == device.device_type) {
                 uint8_t device_id = 0;
-                // Mixer class looks at device_id to determine type and the tag
+                // Water class looks at device_id to determine type and the tag
-                // so fixing to 0x28 which will give all the settings except flowSetTemp
+                if (device.device_type == DeviceType::WATER) {
-                if (device.device_type == DeviceType::MIXER) {
                     if (device.flags == EMSdevice::EMS_DEVICE_FLAG_MMPLUS) {
-                        if (device.product_id == 160) { // MM100
+                        device_id = 0x28; // dhw 1/2
-                            device_id = 0x28;           // dhw
+                    } else if (device.flags == EMSdevice::EMS_DEVICE_FLAG_SM100) {
-                        } else {
+                        device_id = 0x2A; // dhw 3
-                            device_id = 0x20; // hc
+                    } else if (device.flags == EMSdevice::EMS_DEVICE_FLAG_IPM) {
-                        }
+                        device_id = 0x40; // dhw 1
-                    } else {
-                        device_id = 0x20; // should cover all the other device types
                     }
                 }
 
@@ -385,17 +382,14 @@ void EMSESP::dump_all_telegrams(uuid::console::Shell & shell) {
         for (const auto & device : device_library_) {
             if (device_class.first == device.device_type) {
                 uint8_t device_id = 0;
-                // Mixer class looks at device_id to determine type and the tag
+                // Water class looks at device_id to determine type and the tag
-                // so fixing to 0x28 which will give all the settings except flowSetTemp
+                if (device.device_type == DeviceType::WATER) {
-                if (device.device_type == DeviceType::MIXER) {
                     if (device.flags == EMSdevice::EMS_DEVICE_FLAG_MMPLUS) {
-                        if (device.product_id == 160) { // MM100
+                        device_id = 0x28; // dhw 1/2
-                            device_id = 0x28;           // dhw
+                    } else if (device.flags == EMSdevice::EMS_DEVICE_FLAG_SM100) {
-                        } else {
+                        device_id = 0x2A; // dhw 3
-                            device_id = 0x20; // hc
+                    } else if (device.flags == EMSdevice::EMS_DEVICE_FLAG_IPM) {
-                        }
+                        device_id = 0x40; // dhw 1
-                    } else {
-                        device_id = 0x20; // should cover all the other device types
                     }
                 }
 

src/modbus.cpp

@@ -275,20 +275,8 @@ ModbusMessage Modbus::handleRead(const ModbusMessage & request) {
 
     auto register_offset = start_address - tag * REGISTER_BLOCK_SIZE;
 
-    const auto & dev_it =
-        std::find_if(EMSESP::emsdevices.begin(), EMSESP::emsdevices.end(), [&](const std::unique_ptr<EMSdevice> & x) { return x->device_type() == device_type; });
-
-    if (dev_it == EMSESP::emsdevices.end()) {
-        // device not found => invalid server ID
-        LOG_ERROR("device with type %d not found => invalid server ID", device_type);
-        response.setError(request.getServerID(), request.getFunctionCode(), ILLEGAL_DATA_ADDRESS);
-        return response;
-    }
-
-    const auto & dev = *dev_it;
-
     // binary search in modbus infos
-    auto key = EntityModbusInfoKey(dev->device_type(), tag_type, register_offset);
+    auto key = EntityModbusInfoKey(device_type, tag_type, register_offset);
 
     const auto & modbusInfo = std::lower_bound(std::begin(modbus_register_mappings),
                                                std::end(modbus_register_mappings),
@@ -312,7 +300,15 @@ ModbusMessage Modbus::handleRead(const ModbusMessage & request) {
     }
 
     auto buf        = std::vector<uint16_t>(num_words);
-    auto error_code = dev->get_modbus_value(tag, modbusInfo->short_name, buf);
+    int  error_code = -1;
+    for (const auto & emsdevice : EMSESP::emsdevices) {
+        if (emsdevice->device_type() == device_type) {
+            error_code = emsdevice->get_modbus_value(tag, modbusInfo->short_name, buf);
+            if (!error_code) {
+                break;
+            }
+        }
+    }
     if (error_code) {
         LOG_ERROR("Unable to read raw device value %s for tag=%d - error_code = %d", modbusInfo->short_name, (int)tag, error_code);
         response.setError(request.getServerID(), request.getFunctionCode(), SERVER_DEVICE_FAILURE);
@@ -370,22 +366,9 @@ ModbusMessage Modbus::handleWrite(const ModbusMessage & request) {
 
     LOG_DEBUG("Tag %d, offset %d", tag, register_offset);
 
-    const auto & dev_it =
-        std::find_if(EMSESP::emsdevices.begin(), EMSESP::emsdevices.end(), [&](const std::unique_ptr<EMSdevice> & x) { return x->device_type() == device_type; });
-
-    if (dev_it == EMSESP::emsdevices.end()) {
-        // device not found => invalid server ID
-        LOG_ERROR("device_type (%d) not found => invalid server ID", device_type);
-        response.setError(request.getServerID(), request.getFunctionCode(), ILLEGAL_DATA_ADDRESS);
-        return response;
-    }
-
-    const auto & dev = *dev_it;
-
-    LOG_DEBUG("found device '%s' of type %d", dev->name().c_str(), dev->device_type());
 
     // binary search in modbus infos
-    auto key        = EntityModbusInfoKey(dev->device_type(), tag_type, register_offset);
+    auto key        = EntityModbusInfoKey(device_type, tag_type, register_offset);
     auto modbusInfo = std::lower_bound(std::begin(modbus_register_mappings),
                                        std::end(modbus_register_mappings),
                                        key,
@@ -412,9 +395,16 @@ ModbusMessage Modbus::handleWrite(const ModbusMessage & request) {
     }
 
     JsonDocument input_doc;
-    JsonObject   input = input_doc.to<JsonObject>();
+    JsonObject   input      = input_doc.to<JsonObject>();
-
+    int          error_code = -1;
-    auto error_code = dev->modbus_value_to_json(tag, modbusInfo->short_name, data, input);
+    for (const auto & emsdevice : EMSESP::emsdevices) {
+        if (emsdevice->device_type() == device_type) {
+            error_code = emsdevice->modbus_value_to_json(tag, modbusInfo->short_name, data, input);
+            if (!error_code) {
+                break;
+            }
+        }
+    }
     if (error_code) {
         // error getting modbus value as json
         LOG_ERROR("error getting modbus value as json, error code = %d", error_code);
@@ -424,9 +414,9 @@ ModbusMessage Modbus::handleWrite(const ModbusMessage & request) {
 
     std::string path;
     if (tag < DeviceValueTAG::TAG_HC1) {
-        path = std::string("ems-esp/") + std::string(EMSdevice::device_type_2_device_name(dev->device_type())) + "/" + modbusInfo->short_name;
+        path = std::string("ems-esp/") + std::string(EMSdevice::device_type_2_device_name(device_type)) + "/" + modbusInfo->short_name;
     } else {
-        path = std::string("ems-esp/") + std::string(EMSdevice::device_type_2_device_name(dev->device_type())) + "/" + EMSdevice::tag_to_mqtt(tag) + "/"
+        path = std::string("ems-esp/") + std::string(EMSdevice::device_type_2_device_name(device_type)) + "/" + EMSdevice::tag_to_mqtt(tag) + "/"
                + modbusInfo->short_name;
     }
 

src/mqtt.cpp

@@ -592,8 +592,9 @@ void Mqtt::ha_status() {
     publish_system_ha_sensor_config(DeviceValueType::INT8, "Tx reads", "txreads", DeviceValueUOM::NONE);
     publish_system_ha_sensor_config(DeviceValueType::INT8, "Tx writes", "txwrites", DeviceValueUOM::NONE);
     publish_system_ha_sensor_config(DeviceValueType::INT8, "Tx fails", "txfails", DeviceValueUOM::NONE);
-    publish_system_ha_sensor_config(DeviceValueType::INT16, "WiFi reconnects", "wifireconnects", DeviceValueUOM::NONE);
+    if (!EMSESP::system_.ethernet_connected()) {
-
+        publish_system_ha_sensor_config(DeviceValueType::INT16, "WiFi reconnects", "wifireconnects", DeviceValueUOM::NONE);
+    }
     // This comes from the info MQTT topic - and handled in the publish_ha_sensor_config function
     publish_system_ha_sensor_config(DeviceValueType::STRING, "Version", "version", DeviceValueUOM::NONE);
 }