Added Twitch v5 support to the twitch platform #16428
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
homeassistant
added
cla-signed
small-pr
ghost
added
the
houndci-bot
reviewed
Sep 4, 2018
| @@ -72,15 +81,15 @@ def entity_picture(self): | |||
| # pylint: disable=no-member | |||
| def update(self): | |||
| """Update device state.""" | |||
| from twitch.api import v3 as twitch | |||
| stream = twitch.streams.by_channel(self._channel).get('stream') | |||
| id = self._client.users.translate_usernames_to_ids([self._channel])[0].id | |||
There was a problem hiding this comment.
local variable 'id' is assigned to but never used
line too long (81 > 79 characters)
| @@ -34,16 +37,22 @@ | |||
| def setup_platform(hass, config, add_entities, discovery_info=None): | |||
| """Set up the Twitch platform.""" | |||
| channels = config.get(CONF_CHANNELS, []) | |||
| from twitch import TwitchClient as TwitchClient | |||
| client=TwitchClient(client_id=config.get(CONF_CLIENTID)) | |||
houndci-bot
reviewed
Sep 4, 2018
| channels = config.get(CONF_CHANNELS, []) | ||
| client=TwitchClient(client_id=config.get(CONF_CLIENTID)) |
houndci-bot
reviewed
Sep 4, 2018
| channels = config.get(CONF_CHANNELS, []) | ||
|
|
||
| add_entities([TwitchSensor(channel) for channel in channels], True) | ||
| client=TwitchClient(client_id=config.get(CONF_CLIENTID)) |
2 tasks
miawgogo
changed the title
miawgogo
changed the title
fabaff
requested changes
Sep 5, 2018
fabaff
changed the title
|
i assume this shoulent be in there, I have removed it from my branch |
houndci-bot
reviewed
Sep 5, 2018
| return False | ||
| else: | ||
| _LOGGER.error("Test API call failed") | ||
| return False |
There was a problem hiding this comment.
indentation is not a multiple of four
| _LOGGER.error("Test API call failed, Check your client_id") | ||
| return False | ||
| else: | ||
| _LOGGER.error("Test API call failed") |
There was a problem hiding this comment.
indentation is not a multiple of four
fabaff
approved these changes
Sep 6, 2018
| client.ingests.get_server_list() | ||
| except HTTPError: | ||
| _LOGGER.error("Client ID is not valid") | ||
| return False |
There was a problem hiding this comment.
Nothing is checking this return value.
There was a problem hiding this comment.
the orignial code did check this by checking for a 400 error, for the endpoint im using 400 error should only be caused by a missing client id
if http_error.response.status_code == 400:
_LOGGER.error("Test API call failed, Check your client_id")
else:
_LOGGER.error("Test API call failed")
return False```There was a problem hiding this comment.
No, nothing is checking this return value. Don't return False, just return.
Merged
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
THIS IS A BREAKING CHANGE
Description:
This is an update that changes the library use for the twitch sensor, this is a breaking change due to api Changes on twitch's side where all api users require a client_id. I have also made it set the sensor icon to the channels logo when they are not live, as it looks nice.
Middle is live, the others are offline
Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#6201
Example entry for
configuration.yaml(if applicable):Checklist:
tox. Your PR cannot be merged unless tests passIf user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
REQUIREMENTSvariable (example).requirements_all.txtby runningscript/gen_requirements_all.py.If the code does not interact with devices: