core icon indicating copy to clipboard operation
core copied to clipboard

Integration Kostal Plenticore sensor.scb_solar_power not as expected.

Open deadrabbit87 opened this issue 2 years ago • 11 comments

The problem

Hi,

i want to display the values of the inverter as it is done in original kostal ui: grafik

But sensor.scb_solar_power is not like i expect: If the battery is charging, the sensor.scb_solar_power goes to zero.

For example: The solar production is 1000 W and the battery is charging with 1000 W, sensor.scb_solar_power is in this case 0 W.

BW

What version of Home Assistant Core has the issue?

core-2023.2.2

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant OS

Integration causing the issue

kostal_plenticore

Link to integration documentation on our website

https://www.home-assistant.io/integrations/kostal_plenticore/

Diagnostics information

No response

Example YAML snippet

No response

Anything in the logs that might be useful for us?

No response

Additional information

No response

deadrabbit87 avatar Feb 07 '23 08:02 deadrabbit87

Hey there @stegm, mind taking a look at this issue as it has been labeled with an integration (kostal_plenticore) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of kostal_plenticore can trigger bot actions by commenting:

  • @home-assistant close Closes the issue.
  • @home-assistant rename Awesome new title Change the title of the issue.
  • @home-assistant reopen Reopen the issue.
  • @home-assistant unassign kostal_plenticore Removes the current integration label and assignees on the issue, add the integration domain after the command.

(message by CodeOwnersMention)


kostal_plenticore documentation kostal_plenticore source (message by IssueLinks)

home-assistant[bot] avatar Feb 07 '23 18:02 home-assistant[bot]

@stegm This is the history from the day 08.02.2023 with full sun all the day: grafik

Every time the battery charges or discharges, the difference is displayed and not the generated PV Power.

I would expect, that solar power is the sum of DC1 Power and DC2 Power.

deadrabbit87 avatar Feb 09 '23 07:02 deadrabbit87

I can confirm the same behavior: image

MeikTranel avatar Feb 15 '23 22:02 MeikTranel

I see the same behaviour. Maybe a automatic firmware update changed the meaning of the process value.

I will take a look at it.

stegm avatar Feb 16 '23 05:02 stegm

I see the same behaviour. Maybe a automatic firmware update changed the meaning of the process value.

I will take a look at it.

Hi,

could you find the time, checking the issue?

Thanks!

deadrabbit87 avatar Mar 02 '23 16:03 deadrabbit87

I see the same behaviour. Maybe a automatic firmware update changed the meaning of the process value. I will take a look at it.

Hi,

could you find the time, checking the issue?

Thanks!

No, not yet. Sorry.

stegm avatar Mar 05 '23 10:03 stegm

Hi, I've looked into this a little. Currently this integration uses the devices:local:Dc_P value for the "solar power" sensor, this might have been correct in older firmware versions, but currently it looks like the built in web interface sums the individual pv string powers to arrive at total pv power. It queries the processdata of the available PV strings devices:local:pvN (where N is the string number starting with 1) and then takes the sum of the P values of all strings from the response

From the obfuscated source:

        function el(l, n) {
            void 0 === l && (l = tl);
            try {
                if (n.type === nl) {
                    n.payload;
                    var u = function(l) {
                        return function(n) {
                            return n.id === l
                        }
                    }
                      , t = n.payload.reduce(function(l, n) {
                        var t = n.processdata
                          , e = {
                            I: t.find(u("I")).value,
                            P: t.find(u("P")).value,
                            U: t.find(u("U")).value
                        };
                        return [{
                            module_id: n.moduleid,
                            processdata_id_list: e
                        }].concat(l)
                    }, [])
                      , e = (t = t.reverse()).reduce(function(l, n, u) {
                        return n.processdata_id_list.P + l
                    }, 0);
                    l = {
                        isInitial: !1,
                        sum: e,
                        increase: e > l.sum,
                        values: t
                    }
                }
                return l
            } catch (l) {
                return console.warn("Fetching PV-Data Failed"),
                tl
            }
        }

at t = n.payload.reduce(function(l, n) { [...] the I, P, and U values of the individual pv strings are taken from the json returned by processdata and put into an array, then from that array, the P values are added up into a sum at

e = (t = t.reverse()).reduce(function(l, n, u) {
   return n.processdata_id_list.P + l
}, 0);

Then everything is packed into an object that is returned.

This seems to be the behavior this integrations needs to mimic.

eisbehr avatar May 31 '23 12:05 eisbehr

Hi,

thanks for your work. I think the best way would to check if there is a battery installed.

The third string input of a Kostal Plenticore could be used with a battery or with a third PV string.

In case of a third PV string, the value would be correct.

Otherwise not...

deadrabbit87 avatar May 31 '23 17:05 deadrabbit87

Currently this integration uses the devices:local:Dc_P value for the "solar power" sensor, this might have been correct in older firmware versions, but currently it looks like the built in web interface sums the individual pv string powers to arrive at total pv power. It queries the processdata of the available PV strings devices:local:pvN (where N is the string number starting with 1) and then takes the sum of the P values of all strings from the response

You are absolutly right - I already know this and prepared an PR for this integration. Unfortunately it was not accepted so I think about if I could place this logic in the pykoplenti library. But the implementation seems to be a lot more complicated there and currently I have no real idea how to implement this into it.

stegm avatar May 31 '23 18:05 stegm

Oh, I didn't know about the PR, what a bummer. I've taken a look at the code and I do see the issue. The way to get it accepted is probably to change the api of the library, not expose any of the module_ids or keys to the integration. This doesn't have to be as annoying a change as it sounds, you already have your SENSOR_PROCESS_DATA array. Add a unique key at the top of each entry that will be the library's way of referring to the data, then copy and paste the whole list over to the library. On the integration side you remove the module_id and key variables from each entry, on the library side you make the array into a dict, hoist out the newly added library key to be the dict key, then delete all variables but module_id and key from each entry. Now you got yourself a lookup table. In the get_process_value_data() method, change them to take your singular new key (or several "singular new keys", as it were), then first check if the key is the solar power one, do a special case here to calculate the summed value, else do a lookup from library key to api module_id and key in your new lookup dict, then just continue with the old operations as they were. The return value will also have to be adapted to return a dict from "new key" to "value". Same song and dance for the other query types. It's not a pleasant change to make, but at least it's straightforward.

eisbehr avatar May 31 '23 20:05 eisbehr

Interessting idea, though.

I have to look at it. Original I try to add "virtual" process data which are computed by the library using some of the original data. Unfortunately the API makes this not easy to implement.

stegm avatar Jun 05 '23 17:06 stegm

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍 This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.