Skip to content

Add support to ignore selected gateways#72

Merged
syssi merged 12 commits intoDanielhiversen:masterfrom
kerwin612:master
May 17, 2018
Merged

Add support to ignore selected gateways#72
syssi merged 12 commits intoDanielhiversen:masterfrom
kerwin612:master

Conversation

@kerwin612
Copy link
Copy Markdown
Contributor

@kerwin612 kerwin612 commented May 13, 2018

Fix problem of crash when getting the key when there is no gateway.
Support disabled gateway.

@kerwin612 kerwin612 changed the title fix crash code fix crash code & support disabled gateway May 13, 2018
Copy link
Copy Markdown
Collaborator

@syssi syssi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to ignore unconfigured gateways per default and handle just configured ones.

@kerwin612
Copy link
Copy Markdown
Contributor Author

I also think so, but considering that if this is done, existing users will not be available if they are not configured. Of course, you can also consider setting the default value to true in home-assistant/core#14428

@syssi
Copy link
Copy Markdown
Collaborator

syssi commented May 13, 2018

You are right! All discovered gateways are used at the moment. There is no indicator for a unconfigured gateway (the security key isn't required). Your approach is fine!

Comment thread xiaomi_gateway/__init__.py Outdated
try:
ip_address = socket.gethostbyname(host)
if _disabled:
_LOGGER.info("Xiaomi Gateway %s is disabled in the configuration", sid)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"disabled by configuration"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

en, thanks

Comment thread xiaomi_gateway/__init__.py Outdated
self.gateways[ip_add] = XiaomiGateway(ip_add, port, sid, gateway_key, self._socket,
resp["proto_version"] if "proto_version" in resp else None)
if disabled:
_LOGGER.info("Xiaomi Gateway %s is disabled in the configuration",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get

host = gateway.get('host')
port = gateway.get('port')
sid = gateway.get('sid')
key = gateway.get('key')
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't move this line in this case. It's None if no key is available.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I removed it just to pass the lint check, because there are too many local variables and this variable is only used in one place. So I'll call it directly on the method argument.

@syssi syssi changed the title fix crash code & support disabled gateway Add support to ignore selected gateways May 14, 2018
@syssi syssi requested a review from Danielhiversen May 17, 2018 17:41
@syssi
Copy link
Copy Markdown
Collaborator

syssi commented May 17, 2018

@Danielhiversen Do you agree with these changes?

Copy link
Copy Markdown
Owner

@Danielhiversen Danielhiversen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@syssi syssi merged commit c271ea4 into Danielhiversen:master May 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants