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
Add Discogs Sensor platform #10957
Add Discogs Sensor platform #10957
Conversation
Not really sure how to fix linting errors from Travis:
Other components didn't seem to do this when importing asyncio |
"""Set up the discogs sensor.""" | ||
token = config.get(CONF_TOKEN) | ||
|
||
async_add_devices([DiscogsCollection(token)], True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if the token is not valid? I assume that there will be a traceback. It's preferred to check if the connection can be established, otherwise the users will end-up with a non-working platform.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I'm not at all sure how to do this, but I'll give it a try someday this week!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See DiscogsCollection#init which returns false if the user is not present
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess that with an invalid token a discogs_client.exceptions.HTTPError
will be thrown. This will happen before init return. The whole setup of the discogs client should happen in setup()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll try and check again but it probably won't be anytime soon.
|
||
REQUIREMENTS = ['discogs_client==2.2.1'] | ||
|
||
SCAN_INTERVAL = timedelta(seconds=7200) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or hours=2
.
"""Initialize the sensor.""" | ||
import discogs_client | ||
discogs = discogs_client.Client( | ||
'HomeAssistantDiscogs/0.1.0', user_token=token) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please use SERVER_SOFTWARE
(from homeassistant.helpers.aiohttp_client import SERVER_SOFTWARE
)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of the 'HomeAssistantDiscogs' string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you don't provide unit tests then you need to add your platform to .coveragerc
.
The file header is missing which is a docstring. E.g., """
Support for stored record collections on Discogs.
For more details about this platform, please refer to the documentation at
https://home-assistant.io/components/sensor.discogs/
""" |
|
||
self._ds_user = discogs.identity() | ||
self._state = None | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
blank line contains whitespace
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks 🐦
Description:
Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#4138
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:
tox
run successfully. Your PR cannot be merged unless tests passREQUIREMENTS
variable (example).requirements_all.txt
by runningscript/gen_requirements_all.py
..coveragerc
.If the code does not interact with devices:
tox
run successfully. Your PR cannot be merged unless tests pass