Update hass_mqtt.py

[michas79de]

Hi @jblance

I'm using your great work for collecting inverter data into Home Assistant (HA), but I also like to improve it a bit:

I think, if we use "binary_sensor" instead of "sensor" for boolean parameters in the mqtt topics, HA will discover them as binary sensors, too. We should then see a blue/yellow eye icon depending on state and 'state_color: true' in the entity card. [https://www.home-assistant.io/integrations/binary_sensor/] [https://www.home-assistant.io/integrations/mqtt/#mqtt-discovery]

Parameters/Entities without unit don't need and better don't have "unit_of_measurement" declaration. Even with an empty "" unit statement HA uses them as with unit. So the history graph card in HA then shows an empty line graph instead of a horizontal bar chart. e.g.: https://imgur.com/3ha8K97 original graph and above a bar of a copy as a template entity without "unit_of_measurement" (sadly no change in the given time) [https://www.home-assistant.io/dashboards/history-graph]

I hope you can agree with my point of view, because I absolutely don't know how to change this in my running installation! And because I don't speak python you better doublecheck my changes! (There's no endif, isn't? Is it just the four spaces to right/left for loops?)

And if you take over my changes, I just have to enter "sudo pip install -U mppsolar" and
"systemctl --user restart mpp-solar" to update and restart mpp-solar on my pi?

Last but not least:
The PI30 command QMN queries the Inverter Model. This isn't included in your code and I didn't find the right file/place for a proposal. Maybe you find some time to add this. But while it's very clear to read (just with leading "(" and ending "³", it's no problem for me.

[jblance]

Hi @jblance

I'm using your great work for collecting inverter data into Home Assistant (HA), but I also like to improve it a bit:

thanks for the improvements

I think, if we use "binary_sensor" instead of "sensor" for boolean parameters in the mqtt topics, HA will discover them as binary sensors, too. We should then see a blue/yellow eye icon depending on state and 'state_color: true' in the entity card. [https://www.home-assistant.io/integrations/binary_sensor/] [https://www.home-assistant.io/integrations/mqtt/#mqtt-discovery]

Parameters/Entities without unit don't need and better don't have "unit_of_measurement" declaration. Even with an empty "" unit statement HA uses them as with unit. So the history graph card in HA then shows an empty line graph instead of a horizontal bar chart. e.g.: https://imgur.com/3ha8K97 original graph and above a bar of a copy as a template entity without "unit_of_measurement" (sadly no change in the given time) [https://www.home-assistant.io/dashboards/history-graph]

these sound good

I hope you can agree with my point of view, because I absolutely don't know how to change this in my running installation! And because I don't speak python you better doublecheck my changes! (There's no endif, isn't? Is it just the four spaces to right/left for loops?)

yes spacing is critical for python, loops are defined visually

And if you take over my changes, I just have to enter "sudo pip install -U mppsolar" and "systemctl --user restart mpp-solar" to update and restart mpp-solar on my pi?

you code is a little messed up, but I get what you mean, I will incorporate your changes directly and get a new version packaged

Last but not least: The PI30 command QMN queries the Inverter Model. This isn't included in your code and I didn't find the right file/place for a proposal. Maybe you find some time to add this. But while it's very clear to read (just with leading "(" and ending "³", it's no problem for me.

oh, I'll get that added to PI30

[michas79de]

you code is a little messed up, but I get what you mean, I will incorporate your changes directly and get a new version packaged

😎
¯\(ツ)/¯
I think this red/green code compare thing messed it up (visual) more than it is. I just want the algorithm do

if boolean
    send as binary sensor w/o units declaration
else
    if power
        do the power thing
    else if unitless
        send w/o units declaration
    else
        send as usual

But of couse it's your's, feel free to make it like you want to! 👍

Thank you very much in advance!

[jblance]

😎 ¯_(ツ)_/¯ I think this red/green code compare thing messed it up (visual) more than it is. I just want the algorithm do

if boolean
    send as binary sensor w/o units declaration
else
    if power
        do the power thing
    else if unitless
        send w/o units declaration
    else
        send as usual

Yeah, but its simpler to go

if boolean
    send as binary sensor w/o units declaration
else if power
    do the power thing
else if unitless
    send w/o units declaration
else
    send as usual

and there is a few lines in the bool logic that are common, so can be pull out a level - dont take it as a criticism any help is appreciated, its just the code is complicated anyway so I try my best to make it as simple as possible (especially regard duplication) otherwise weird bugs can occur.

[jblance]

er scratch all that, I see why you did it this way now - good job and ignore me

[Saentist]

@michas79de
What about hassd_mqtt module it's more organized in HA #269

See also #243 QPI#QMN#QGMN is maybe a most accurate way to detect inverter.

[michas79de]

@michas79de What about hassd_mqtt module it's more organized in HA #269

I did'n knew that, when I startet with mpp-solar. But now I'm using a lot of the sensors in cards, graphs or other sensors in HomeAssistant. So I don't want to change them all again.
Also I have just this one device and I'm using several sections with unique tags for each command in the mpp-solar.conf. So there's no further need for more organization.
But thanks anyway!

[Saentist]

Just change to hassd_mqtt, noting will change to sensors in HA,
just will have more organized view in Mosquitto broker.
see images in #269

Sooner or later hass_mqtt will migrate to hassd_mqtt.