Season sensor#8958
Conversation
|
@w1ll1am23, thanks for your PR! By analyzing the history of the files in this pull request, we identified @balloob, @fabaff and @rmkraus to be potential reviewers. |
There was a problem hiding this comment.
continuation line over-indented for visual indent
There was a problem hiding this comment.
continuation line over-indented for visual indent
There was a problem hiding this comment.
continuation line over-indented for visual indent
There was a problem hiding this comment.
continuation line over-indented for visual indent
There was a problem hiding this comment.
continuation line over-indented for visual indent
There was a problem hiding this comment.
continuation line over-indented for visual indent
There was a problem hiding this comment.
continuation line over-indented for visual indent
There was a problem hiding this comment.
continuation line over-indented for visual indent
There was a problem hiding this comment.
continuation line over-indented for visual indent
There was a problem hiding this comment.
Should not be part of this pr
There was a problem hiding this comment.
Hmm not sure how that got there, all I did was run gen_requirements_all.py
w1ll1am23
force-pushed
the
season_sensor
branch
from
4ee494b to
bae17d8
Compare
There was a problem hiding this comment.
Please don't use STATE_UNKNOWN, instead use None and let Home Assistant handle unknown states.
There was a problem hiding this comment.
I think that a nice cleanup would be to extract this method from this class and just make it a method in this file . Have it pass datetime as a parameter.
That way it will make your tests also a lot easier, you can test the method directly.
There was a problem hiding this comment.
It would be faster to create datetimes and just replace the year instead of going via strings
There was a problem hiding this comment.
Is it really necessary to have all the datetimes of starting points of seasons, couldn't you just check versus the month ?
if 6 <= datetime.month < 9:
season = STATE_SUMMERThere was a problem hiding this comment.
This is because the seasons don't start with the month. For example this year the first day of fall in the northern hemisphere is 9/22 but in 2019 it will be on 9/23. That is of course if the user is using astronomical seasons.
There was a problem hiding this comment.
undefined name 'DEVICE'
line too long (95 > 79 characters)
There was a problem hiding this comment.
undefined name 'DEVICE'
line too long (95 > 79 characters)
There was a problem hiding this comment.
line too long (81 > 79 characters)
There was a problem hiding this comment.
line too long (80 > 79 characters)
There was a problem hiding this comment.
'homeassistant.const.STATE_UNKNOWN' imported but unused
|
@balloob I think I have addressed all of your suggestions. |
There was a problem hiding this comment.
continuation line over-indented for visual indent
There was a problem hiding this comment.
continuation line over-indented for visual indent
w1ll1am23
force-pushed
the
season_sensor
branch
from
e03bac3 to
44c6456
Compare
|
I think you can convert this into a async component in future |
* upstream/dev: (113 commits) Fix fitbit error when trying to access token after upgrade. (home-assistant#9183) Upgrade sendgrid to 5.0.1 (home-assistant#9215) Upgrade pyasn1 to 0.3.3 and pyasn1-modules to 0.1.1 (home-assistant#9216) directv: extended discovery via REST api, bug fix (home-assistant#8800) Bayesian Binary Sensor (home-assistant#8810) Add cloud auth support (home-assistant#9208) Abode push events and lock, cover, and switch components (home-assistant#9095) Lint Sonarr tests Upgrade pymysensors to 0.11.1 (home-assistant#9212) Refactor rfxtrx (home-assistant#9117) Issue home-assistant#6893 in rfxtrx (home-assistant#9130) Support for season sensor (home-assistant#8958) Add counter component (home-assistant#9146) Fix and optimize digitalloggers platform (home-assistant#9203) Prevent error when no forecast data was available (home-assistant#9176) Add "status" to Sonarr sensor (home-assistant#9204) fix worldtidesinfo home-assistant#9184 (home-assistant#9201) Update pushbullet.py (home-assistant#9200) Fix dht22 when no data was read initially home-assistant#8976 (home-assistant#9198) Prevent iCloud exceptions in logfile (home-assistant#9179) ...
|
This PR did not include documentation 😞 |
|
Dang, you are right I totally forgot. I'll try to get that added in the next day or two. |
|
I've added a stub now since it's release day. |
ghost
added
8 participants
Description:
This sensor will display the current astronomical or meteorological season (Spring, Summer, Autumn, Winter) based on the users setting in the config file.
All information about how the seasons work was taken from Wikipedia:
https://en.wikipedia.org/wiki/Season#Astronomical
https://en.wikipedia.org/wiki/Equinox
https://en.wikipedia.org/wiki/Solstice
I don't expect this to be very useful for anyone in tropical regions because their seasons aren't really anything like the reset of the world. Maybe someone that knows more about that can add support for them? Wet/Dry seasons?
Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.github.io#<home-assistant.github.io PR number goes here>
Example entry for
configuration.yaml(if applicable):Checklist:
If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
toxrun successfully. Your PR cannot be merged unless tests passREQUIREMENTSvariable (example).requirements_all.txtby runningscript/gen_requirements_all.py.