gieljnssns / kostalpiko-sensor-homeassistant

A custom component to get the readings of a Kostal Piko inverter
MIT License
12 stars 18 forks source link

Better support for multiple Piko's and adding the status as a sensor #7

Closed Vedeneb closed 4 years ago

Vedeneb commented 4 years ago

Hi,

first of all, thanks for this integration, it works flawlessly! I have two suggestions/feature requests:

First, I have three identical Kostal Pikos (because we have a huge roof with lots of Solar Panels :)). I added them all the same way to the config:

- platform: kostal
  host: http://192.168.178.251
  username: pvserver
  password: aaa
  monitored_conditions:
    - current_power
    - ....
- platform: kostal
  host: http://192.168.178.252
  username: pvserver
  password: aaa
  monitored_conditions:
    - current_power
    - ....
- platform: kostal
  host: http://192.168.178.253
  username: pvserver
  password: aaa
  monitored_conditions:
    - current_power
    - ....

It does work, all entities have been registered successfully. However, they are called like this: sensor.kostal_piko_current_power sensor.kostal_piko_current_power_2 sensor.kostal_piko_current_power_3

which is ok, however I think a little configuration option for a custom name would be the better solution. So if I would set up the integration like this:

- platform: kostal
  name: Solar 1
  host: http://192.168.178.251
  username: pvserver
  password: aaa
  monitored_conditions:
    - current_power
    - ....

the entities would be called sensor.solar_1_current_power and so on.

The second that would be great if you could add it would be a sensor exposing the current status of the Piko. image

That way one could easily build automations to send notifications e.g. if the state changes to an error.

Thanks in advance and thanks again for the integration!

Vedeneb

gieljnssns commented 4 years ago

Hello, about naming the different inverters; You could try the latest version ... But, about the current status ... The KostalPyko package searches the web server for white areas (#FFFFFF) and extracts the values. And since the status is not in such a box, it is difficult to find this value. And I also don't know what the web server looks like in different languages.

Vedeneb commented 4 years ago

I just realized that having multiple Pikos is quite buggy actually. The entity ID's are not persisent - after a restart of Hass my first Piko suddenly had it's entity ID's ending with _2 which before was the second piko. I didn't realize it before because untily yesterday I was only using a template sensor to show the total energy of all three pikos, which - of course - was always correct

Edit: Wow, just saw your answer popping in when I was writing my reply! Thanks for the update! I'll check it out right away. ;)

Vedeneb commented 4 years ago

Thank you so much, I just installed the updated and the name option works absolutely perfect!

About the status, that's what my webinterface looks like: image

Basically identical to yours, except the language (german in my case). Shouldn't it be possible to get the status using xPath/CSS selectors? That's the xPath for mine: /html/body/form/font/table[2]/tbody/tr[8]/td[3]

Do you think I should open an feature request at the KostalPiko package? Or can you add it that way? Would be great :)

Thanks again and have a great day!

gieljnssns commented 4 years ago

Yes you can open a feature request at KostalPyko... I'm not good in html, I just forked that package and adjusted it a bit to use requests for the support of python3.

gieljnssns commented 4 years ago

I'm not good in html

Btw my python skills are also trial and error...

Vedeneb commented 4 years ago

Still the integration works absolutely perfect ;) I'll have a look, perhaps I find a way to add the status myself.

gieljnssns commented 4 years ago

Pull requests are welcome...

Vedeneb commented 4 years ago

I'll give it a try. Not promissing anything tho :)

Vedeneb commented 4 years ago

I just forked that package and adjusted it a bit

The integration has "kostalpyko==0.3" as a requierment. Doesn't that install the original version though? Or is the official python package your fork?

gieljnssns commented 4 years ago

The original package is listed as pikopy on Pypi

Vedeneb commented 4 years ago

Okay, so I would create the pull request here which would update this package? Correct?

gieljnssns commented 4 years ago

totally correct

Vedeneb commented 4 years ago

I realized that the package creates a new request to the piko for every single type of data. So if I have the sensor set up like this (which I do):

 monitored_conditions:
    - current_power
    - total_energy
    - daily_energy
    - string1_voltage
    - string1_current
    - string2_voltage
    - string2_current
    - l1_voltage
    - l1_power
    - l2_voltage
    - l2_power
    - l3_voltage
    - l3_power

the integration makes 13 requests to the Piko every 30 seconds, although every request contains all data anyway. Wouldn't it be better to have one update() method which saves the raw_content in a variable and the methods for getting the single values uses this variable. If you agree, I would change the package to do it this way and create a pull request.

Vedeneb commented 4 years ago

I already got warnings in Home Assistant that the integration takes more than 10 seconds to update, now I know why😄

Vedeneb commented 4 years ago

Never mind. I didn't look at your component, you don't actually use that functions provided by KostalPyko and only use the raw_content. So I guess all I have to do is to add the status information to the raw_data in the KostalPyko and expose it as a sensor in your component.

gieljnssns commented 4 years ago

I think it should work now?

Vedeneb commented 4 years ago

Just updated the component using HACS and yes - everythink works! Thanks!