Achronite / mqtt-energenie-ener314rt

MQTT interface for Energenie ENER314-RT add-on board for the Raspberry Pi, designed for use by Home Assistant.
MIT License
13 stars 5 forks source link

Monitor Plug (MIHO004) MQTT Auto discovery issue #85

Closed genestealer closed 7 months ago

genestealer commented 7 months ago

MQTT Auto discovery issue. Monitor Plug (MIHO004)'s real power sensor entry are missing their names. REAL_POWER sensor is named sensor.server_plug

Whereas Smart Plug+ (MIHO005)'s REAL_POWER sensor is named sensor.dehumidifier_real_power

I.E. MIHO004 are missing the "_real_power" part of their name, without the word "power" in the sensor it's harder to find and filter on the entries.

image image

image

image image image

Achronite commented 7 months ago

We can only have one name of 'null' for each device AFAIK. So I thought that the switch should be the the 'main' entity (null for name) for the MIHO005 device; whereas it is 'real power' for the MIHO004 as it seems the most useful.

genestealer commented 7 months ago

Clause: Something funny in the Home Assistant MQTT Discovery code. Setting name: entity_name to null removes the "type" from the entity. This is ok when it's a light hall_light becomes light.hall but less helpful for server_real_power becoming sensor.server [device type power]

Fix, found by looking at Zigbee2MQTT examples.

In function publishDiscovery( device ){ the section on var dmsg = Object.assign change

name: entity_name,

to

// Remove name property if entity_name is null
...(entity_name !== null ? { name: entity_name } : {}),
Achronite commented 7 months ago

Just seen your update... So would you rather we dont have a null name for real power? If so it's an easy fix; just remove 'main: true in it's devices file.

genestealer commented 7 months ago

@Achronite Looking at more examples, the null works well, but only for things which you control. So "null" for lights, switches, covers, ect is 100% valid.

However, for monitor only things, the "null" mucks things up; My edit to remove name property if entity_name is null fixes that.

Warning, removing the "main": true, would cause issues, as this is the only time we load the full device_details into MQTT

image

genestealer commented 7 months ago

Screenshot of fixed entity:

image image image

genestealer commented 7 months ago

@Achronite No I was wrong! Removing the name = NULL will have a negative knock on effect of setting the entity name to "MQTT Switch" rather than just Switch.

I re-read mqtt-discovery and it says "If no name is set, a default name will be set by MQTT" which is why the the REAL_POWER sensor is named sensor.server_plug and not sensor.server_plug_real_power

Actual fix: Allways set name to something, else the first entity in the .json file will be missing the custom name.

Full example:

if (parameter.main){
    var device_details = {
        name: `${device_defaults.mdl} ${device.deviceId}`,
        ids: [`ener314rt-${device.deviceId}`],
        mdl: `${device_defaults.mdl} (${device_defaults.mdlpn}) [${device.deviceId}]`,
        mf: 'Energenie',
        sw: `mqtt-ener314rt ${APP_VERSION}`,
        via_device: 'mqtt-energenie-ener314rt'
    }
} else {
    var device_details = {ids: [`ener314rt-${device.deviceId}`]};
}

var dmsg = Object.assign({
    uniq_id: `ener314rt-${device.deviceId}-${parameter.id}`,
    device: device_details,
    "~": `${CONFIG.topic_stub}${device.productId}/${device.deviceId}/`,
    name: toTitleCase(parameter.id),
    avty_t: `${CONFIG.topic_stub}availability/state`,
    o: {
        name: `mqtt-energenie-ener314rt`,
        sw: `${APP_VERSION}`,
        url: `https://github.com/Achronite/mqtt-energenie-ener314rt`
    }
},
parameter.config );
Achronite commented 7 months ago

The entity_name=null functionality was added so that the main entity becomes the name of the device in line with HA naming guidelines.

I guess that for the 'monitor only' devices it makes less sense to do this; but I don't want to go back to the old way for the control devices, where it still makes sense to have a default entity. I'll have a think about how best to solve this and preserving the change where the device details are only defined once.

Achronite commented 7 months ago

OK - I've had a think, here is my proposal; still set the name when it is a type "sensor"

//
// To save on network/processing only the main entity will contain the details of the device that they belong to
//
if (parameter.main){
    var device_details = {
          name: `${device_defaults.mdl} ${device.deviceId}`,
          ids: [`ener314rt-${device.deviceId}`],
          mdl: `${device_defaults.mdl} (${device_defaults.mdlpn}) [${device.deviceId}]`,
          mf: 'Energenie',
          sw: `mqtt-ener314rt ${APP_VERSION}`,
          via_device: 'mqtt-energenie-ener314rt'
        }
        // To align to HA standards, the main parameter should not be appended to the entity name
        // Except when the main component is type sensor, where we must preserve it
        if ( parameter.component == "sensor"){
            var entity_name = toTitleCase(parameter.id);
        } else {
            var entity_name = null;
        }

} else {
      var entity_name = toTitleCase(parameter.id);
      var device_details = {ids: [`ener314rt-${device.deviceId}`]};
}

var dmsg = Object.assign({
          uniq_id: `ener314rt-${device.deviceId}-${parameter.id}`,
          device: device_details,
          "~": `${CONFIG.topic_stub}${device.productId}/${device.deviceId}/`,
          name: entity_name,
...

For ref from our discussion before:

Device Description 'main' entity type
MIHO004 MiHome Smart Monitor Plug (Pink) real power sensor
MIHO005 MiHome Smart Plug+ (Purple) switch switch
MIHO006 MiHome House Monitor apparent power sensor
MIHO013 MiHome Radiator Valve climate control climate control
MIHO032 MiHome Motion sensor motion binary_sensor
MIHO033 MiHome Open Sensor contact binary_sensor
MIHO069 MiHome Heating Thermostat climate control climate control
MIHO089 MiHome Click voltage sensor

So in other words only the MIHO004, MIHO006 & MIHO089 are affected