home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
69.75k stars 28.91k forks source link

New eveing dampening of forecast solar does no seem to be working #100576

Open sargonas opened 9 months ago

sargonas commented 9 months ago

The problem

Forecast.Solar was documented to have been updated in 2023.9 with the ability to set separate dampening levels for both morning and evening, instead of the previous one universal offset. However, no matter what adjustments I try, I cannot seem to get an evening setting to actually affect the forecast graph. Any changes to the morning setting are reflected upon the next update quite obviously, but no values I have tested insofar for evening have changed the projection in any noticeable way.

I've ever gone to far as to take screen shots and try changing it everywhere form .25 all the way up to 5.0 in .5 increments, stacked the images, and all of them were identical.

What version of Home Assistant Core has the issue?

2023.9.2

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant Container

Integration causing the issue

Forecast.Solar

Link to integration documentation on our website

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

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

home-assistant[bot] commented 9 months ago

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

Code owner commands Code owners of `forecast_solar` can trigger bot actions by commenting: - `@home-assistant close` Closes the issue. - `@home-assistant rename Awesome new title` Renames the issue. - `@home-assistant reopen` Reopen the issue. - `@home-assistant unassign forecast_solar` Removes the current integration label and assignees on the issue, add the integration domain after the command.

(message by CodeOwnersMention)


forecast_solar documentation forecast_solar source (message by IssueLinks)

sargonas commented 9 months ago

Checking in on this one since it's been about 11 days? Just trying to get a sense on if I am doing something wrong, or if it is not working as expected.

klaasnicolaas commented 8 months ago

You could also compare by downloading different diagnostics from the integration and see what data comes in, perhaps the difference is minimal and otherwise you could also try using Postman to retrieve data directly with the API and see if that also differs.

This way you could investigate a bit and rule out what the cause is and whether there really is a problem.

./Klaas

sargonas commented 8 months ago

I have run a direct API call against forecast.solar for a given day without damping, and then again with morning/evening values of .25 and 1 respectively, and the result was as expected, with the hourly kWh estimates changing by the damping to the expected variations.

I then looked at the values for the same day in HA as provided by the integration, which was configured with the same damping values of .25 morning and 1 evening, and the hourly kWh estimates were not the same as the direct API call with very similar (but not exactly the same) morning values with the evening values being wildly different, seemingly skewing further and further away from the expected values the further one went in the day by hour.

sargonas commented 5 months ago

bumping this issue as I did as suggested and compared the results between the integration in HA and the FS website directly, and as determined above the HA integration is not reporting accurate values compared to the FS website API.

Unfortunately, that is the limitations of my skill set and knowledge in this area, as to what the is skewing the results between the API, integration, and rendering in the Energy panel, I'll need to defer to the experts who built this.

klaasnicolaas commented 5 months ago

At the moment I don't see anything strange in the code that could explain this. When I have time I'll look at it again.

./Klaas

sargonas commented 5 months ago

Thanks!

It's not game breaking so not urgent, I simply wanted to make sure this didn't fall off any radars and got a look when possible. Much appreciated!

cirrusflyer commented 5 months ago

I'm seeing this too.

nils-82 commented 5 months ago

I m seeing this as well. Br

bj00rn commented 3 months ago

It looks like the lib is setting ?damping=0 by default for all requests, in addition to any optional damping_evening, damping_morning from specified by the integration options.

https://github.com/home-assistant-libs/forecast_solar/blob/171deff67b95e80605806e3db4800e4a01ceddb6/forecast_solar/__init__.py#L36

Setting damping_morning = 1 and damping_evening= 1 in integration options will give the request:

https://api.forecast.solar/estimate/51.15/10.45/20/0/12.34?damping=0&damping_morning=1&damping_evening=1

From the looks of the docs above does not seem like a supported parameter format.

https://doc.forecast.solar/damping

According to the docs the supported request format is either:

?damping=<factor morning>[,<factor evening]

OR

?damping_morning=<factor morning>&damping_evening=<factor evening>

sargonas commented 2 months ago

This lines up with what my digging has uncovered as well.

klaasnicolaas commented 2 months ago

We've had a discussion about this in the past with the creator of the API. His answer at the time was that if you send damping_morning, damping_evening and damping as parameters, the morning/evening will be leading in the request.

./Klaas

sargonas commented 2 months ago

Interesting. So then I take it that it is safe to assume this is not the source of the inconsistency then, and we need to look elsewhere?

On Fri, Apr 12, 2024 at 4:31 PM Klaas Schoute @.***> wrote:

We've had a discussion about this in the past with the creator of the API. His answer at the time was that if you send damping_morning, damping_evening and damping as parameters, the morning/evening will be leading in the request.

./Klaas

— Reply to this email directly, view it on GitHub https://github.com/home-assistant/core/issues/100576#issuecomment-2052693054, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAALDF23WHGIUKYYDTS3KYDY5BVFTAVCNFSM6AAAAAA45YXMF6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANJSGY4TGMBVGQ . You are receiving this because you authored the thread.Message ID: @.***>

klaasnicolaas commented 2 months ago

What I was thinking about is that in Home Assistant we process the damping values as float, so 0,25. But the API wants a dot notation, so 0.25 and that might cause problems 🤷🏻‍♂️ In any case, worth checking to rule out some things.

./Klaas

bj00rn commented 2 months ago

What I was thinking about is that in Home Assistant we process the damping values as float, so 0,25. But the API wants a dot notation, so 0.25 and that might cause problems 🤷🏻‍♂️ In any case, worth checking to rule out some things.

./Klaas

Seems unlikely though. Float.__str__() is culture invariant methinks.

>>> dampning_evening = 0.25
>>> f"dampning_evenning={dampning_evening}"
'dampning_evenning=0.25'
>>>