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
73.77k stars 30.88k forks source link

Forecast.Solar: Daylight saving seems to be implemented incorrectly #115214

Open CommanderRedYT opened 7 months ago

CommanderRedYT commented 7 months ago

The problem

Since 31. of March, we have summertime in europe. Since this date, the forecast seems to be off about an hour, which is exactly how the clocks changed. Before the date, the graphs aligned.

image

What version of Home Assistant Core has the issue?

core-2024.4.0

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 7 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. - `@home-assistant add-label needs-more-information` Add a label (needs-more-information, problem in dependency, problem in custom component) to the issue. - `@home-assistant remove-label needs-more-information` Remove a label (needs-more-information, problem in dependency, problem in custom component) on the issue.

(message by CodeOwnersMention)


forecast_solar documentation forecast_solar source (message by IssueLinks)

klaasnicolaas commented 7 months ago

The question is where this problem arises and whether this is an Home Assistant integration related issue or whether it already arises with the API server of forecast.solar itself. The data we request is in the ISO8601 format and displays the local time.

./Klaas

klaasnicolaas commented 7 months ago

Could you check what system time you have set in your Home Assistant container?

./Klaas

CommanderRedYT commented 7 months ago

I ran this inside the docker container:

$ date +"%Z %z"
CEST +0200
Mariusthvdb commented 7 months ago

seeing the same on HA OS... so probably nothing to do with install method?

Scherm­afbeelding 2024-04-10 om 16 26 18
klaasnicolaas commented 7 months ago

In the past, issues have been made by people who ran either Home Assistant container or supervised. A possible solution might be to retrieve the data using UTC time. My graph of today:

image

suprimex commented 7 months ago

I am voting up, because of the same issue.

coderjohnjohn commented 6 months ago

Same issue here, on a HA Green device with factory installation, updated to latest. Am in GMT+13 Auckland timezone, which is +1hour for southern hemisphere winter, so the forecast is showing a 12pm peak production instead of an 11am peak. I'm guessing the system just doesn't have the info to know what the daylight saving time offset is?

kchiem commented 5 months ago

I have this issue as well, running HAOS. However in my case, forecast data seems to be an hour behind.

ScreenClip

[core-ssh ~]$ date +"%Z %z"
PDT -0700
iRonin commented 5 months ago

Not sure if it's related but in my case there is a discrepancy between the dashed line and sensor.power_highest_peak_time_today. I'm in GMT-4 without daylight saving time (Antigua). The dashed line seems okay showing the peak around noon but the sensor tells it's at 11am:

image image

(this is at 10:36am)

issue-triage-workflows[bot] commented 2 months ago

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.

CommanderRedYT commented 2 months ago

This issue still occurs for me with latest version of homeassistant

lukshan13 commented 2 months ago

Can confirm this issue exists. Would a configurable offset work in settings?

CommanderRedYT commented 2 months ago

I think knowing the offset via the location and timezone and date would be better. Automatic daylight saving etc.

klaasnicolaas commented 2 months ago

I wonder to what extent this is really an integration issue or rather something for the frontend repository for specifically this energy dashboard graph.

./Klaas

lukshan13 commented 2 months ago

I think knowing the offset via the location and timezone and date would be better. Automatic daylight saving etc.

Oh, 100%, but i remember viewing a Tom Scott video on the night mares of managing TimeZones. A configurable offSet value, that we can modify would be great, we can write our own automation to manage it too for automatic daylight savings. It's not the ideal case, but it would work.

I wonder to what extent this is really an integration issue or rather something for the frontend repository for specifically this energy dashboard graph.

./Klaas

There's quite a bit of posts on reddit about this issue. A lot of people are seeing the graphs to be offset by +- hour or two. Don't get me wrong, it's amazing work, but it would amazing if this feature would be enabled to let us change the offset a bit

klaasnicolaas commented 2 months ago

Setting an offset is not a neat way to solve this and I don't see us building something like that into the integration anytime soon. Besides, the integration does nothing more than retrieve data and throw it directly to the frontend of the energy dashboard to process it further.

There's quite a bit of posts on reddit about this issue.

This problem has been there for as long as the integration and the energy dashboard exist (2021), there have been multiple frontend issues created about this but often automatically closed due to inactivity.

Ultimately, I think that this issue also belongs better in the frontend repositoy than here in core. It goes wrong in the energy dashboard itself or in what the API gives us in combination with the processing of this in the python package.

./Klaas

lukshan13 commented 2 months ago

That's fair enough. I suspect that it's not going to be solved anytime soon on the front end. I know it's not a 'neat' solution but it would be a solution that could relatively quickly be implemented. What are your thoughts on if I or another developer create a pull request with the offset feature?

klaasnicolaas commented 2 months ago

Be my guest to work on it, bit curious how you think to solve this in code. Ultimately the review process will determine whether it will actually be merged.

Another thing I was thinking about is to no longer retrieve the data in ISO8601 (local time) but as UTC.

./Klaas

lukshan13 commented 2 months ago

Sure man, I'll take a look into it when I get some time. I'll take a look at both approaches. Is there a way to run the core integration code without needing to pass in an instance of HA? Never worked on an integration before

ximex commented 2 months ago

I would recommend to use UTC everywhere (API, Storage,...) except what geht displayed to the User, this should bei the local timezone including summer/winter time