Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Environment Canada forecast not showing correctly on Weather Card #47605

Closed
twrecked opened this issue Mar 8, 2021 · 11 comments · Fixed by #49826
Closed

Environment Canada forecast not showing correctly on Weather Card #47605

twrecked opened this issue Mar 8, 2021 · 11 comments · Fixed by #49826

Comments

@twrecked
Copy link

twrecked commented Mar 8, 2021

The problem

After a certain point in the day Environment Canada removes the "high temperature" from its forecast, at that point the temperatures showing in the weather card are incorrectly aligned.

What is version of Home Assistant Core has the issue?

core-2021.3.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

Environment Canada

Link to integration documentation on our website

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

Example YAML snippet

I'm just using this setup, the component is finding my location perfectly.

- platform: environment_canada

Anything in the logs that might be useful for us?

No logs.

# Put your logs below this line

I looked at the code and when the "high" temperature isn't present the code does two things:

  • it creates the first entry with today's low and tomorrow's high
  • it creates the remaining entries with the day's high being the day's low and the day's low being the next day's high.

This is what's shown:
weather-1

This is what Environment Canada is saying:
weather-2

Forgive me, I played around with the code and I think a better solution is to use the currently hourly forecast if the "high" temperature is missing.

--- /home/steve/ha/src/3rdparty/home-assistant/homeassistant/components/environment_canada/weather.py   2021-03-07 23:33:41.753559086 -0500
+++ ec/weather.py       2021-03-07 23:32:39.886876233 -0500
@@ -202,16 +202,17 @@
                     ATTR_FORECAST_TEMP_LOW: int(half_days[1]["temperature"]),
                 }
             )
+            half_days = half_days[2:]
         else:
             today.update(
                 {
+                    ATTR_FORECAST_TEMP: int(ec_data.hourly_forecasts[0].get("temperature")),
                     ATTR_FORECAST_TEMP_LOW: int(half_days[0]["temperature"]),
-                    ATTR_FORECAST_TEMP: int(half_days[1]["temperature"]),
                 }
             )
+            half_days = half_days[1:]

         forecast_array.append(today)
-        half_days = half_days[2:]

         for day, high, low in zip(range(1, 6), range(0, 9, 2), range(1, 10, 2)):
             forecast_array.append(

It's not perfect but with this fix, it looks like this:
weather-4

And Environment Canada is showing:
weather-5

Another solution might be to keep the last known high and use that and fall back to the hourly one if necessary.

Many thanks for the integration, can't believe it took me this long to find it!

@probot-home-assistant
Copy link

environment_canada documentation
environment_canada source
(message by IssueLinks)

@twrecked
Copy link
Author

twrecked commented Mar 8, 2021

Just to add to this. After midnight the days don't line up. EC keeps their "Night" forecast well past midnight.

I'm playing with diff now but I won't know for sure until after midnight tonight.

--- /home/steve/ha/src/3rdparty/home-assistant/homeassistant/components/environment_canada/weather.py	2021-03-07 23:33:41.753559086 -0500
+++ ec/weather.py	2021-03-08 08:25:22.812995985 -0500
@@ -185,35 +185,33 @@
     if forecast_type == "daily":
         half_days = ec_data.daily_forecasts
 
-        today = {
-            ATTR_FORECAST_TIME: dt.now().isoformat(),
-            ATTR_FORECAST_CONDITION: icon_code_to_condition(
-                int(half_days[0]["icon_code"])
-            ),
-            ATTR_FORECAST_PRECIPITATION_PROBABILITY: int(
-                half_days[0]["precip_probability"]
-            ),
-        }
-
-        if half_days[0]["temperature_class"] == "high":
-            today.update(
-                {
-                    ATTR_FORECAST_TEMP: int(half_days[0]["temperature"]),
-                    ATTR_FORECAST_TEMP_LOW: int(half_days[1]["temperature"]),
-                }
-            )
-        else:
-            today.update(
-                {
+        # Environment Canada will remove the "high" forecast late in the day.
+        # This will cause mis-alignment on the weather card. So, when the
+        # "high" is removed:
+        # - if hour of day is in the afternoon (>12) we will add in current
+        #   forecast so the high/lows line up
+        # - if hour of day is in the morning we will just drop first entry so
+        #   the days line up
+        day_start = 0
+        if half_days[0]["temperature_class"] != "high":
+            if dt.now().hour > 12:
+                forecast_array.append({
+                    ATTR_FORECAST_TIME: dt.now().isoformat(),
+                    ATTR_FORECAST_TEMP: int(ec_data.hourly_forecasts[0].get("temperature")),
                     ATTR_FORECAST_TEMP_LOW: int(half_days[0]["temperature"]),
-                    ATTR_FORECAST_TEMP: int(half_days[1]["temperature"]),
-                }
-            )
-
-        forecast_array.append(today)
-        half_days = half_days[2:]
+                    ATTR_FORECAST_CONDITION: icon_code_to_condition(
+                            int(half_days[0]["icon_code"])
+                    ),
+                    ATTR_FORECAST_PRECIPITATION_PROBABILITY: int(
+                            half_days[0]["precip_probability"]
+                    ),
+                })
+                day_start = 1
+            half_days = half_days[1:]
 
-        for day, high, low in zip(range(1, 6), range(0, 9, 2), range(1, 10, 2)):
+        day_end = int(len(half_days)/2)
+        for day, high, low in zip(range(day_start, day_end + 1), range(0, 9, 2), range(1, 10, 2)):
+            print(f"day={day},high={high},low={low}")
             forecast_array.append(
                 {
                     ATTR_FORECAST_TIME: (

@michaeldavie
Copy link
Contributor

Please assign this to me as codeowner

@dave-dube
Copy link

@michaeldavie Curious to know if you got any progress on this issue. Thanks

@michaeldavie
Copy link
Contributor

Sorry, I haven't had a chance to look at this yet. I see the problem though, I just need to think through the logic again for the two possible cases (both high and low for today and just low).

@michaeldavie
Copy link
Contributor

@twrecked @dave-dube

I've fixed the logic so that None is returned as the current day's high when it's no longer available. The small card displays only the low temperature, but the expanded card is filling in the missing high as 0ºC. However, to me this is a front end issue. Let me know what you think.

Screen Shot 2021-04-27 at 10 03 38 PM

Screen Shot 2021-04-27 at 9 58 30 PM

@dave-dube
Copy link

@michaeldavie I think your solution make sense. Good job !

@michaeldavie
Copy link
Contributor

I opened a frontend issue at home-assistant/frontend#9047

@dave-dube
Copy link

@michaeldavie I'm not familiar with the full process, so sorry for dumb question.... do you have any idea when this issue will be merged ? As far as i know, the FrontEnd issue has been merged several weeks ago.

@michaeldavie
Copy link
Contributor

michaeldavie commented May 27, 2021

Unfortunately I don't. I left a comment on the PR six days ago but haven't heard anything. Usually it doesn't take long to get simple changes like this merged.

I think the frontend fix is still pending on this PR: home-assistant/frontend#9066

@dave-dube
Copy link

ok thanks for feedback

@github-actions github-actions bot locked and limited conversation to collaborators Jul 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants