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

Xiaomi Aqara: Remove/Add device service added #10150

Merged

Conversation

syssi
Copy link
Member

@syssi syssi commented Oct 26, 2017

Description:

I have refactored the service handling and added some schema pre-validation by voluptuous. Furthermore two new service (add_device & remove_device) are introduced.

The service "add_device" enables the join_permission of the Xiaomi Gateway for 30 seconds.
The service "remove_device" removes a sub-device identified by the device address (f.e. 158d000xxxxxc2)

Related issue: fixes #9571

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#3836

gateway.write_to_hub(gateway.sid, **join_permission)
break
else:
_LOGGER.error('Unknown gateway sid: %s was specified.', gw_sid)
Copy link
Member

Choose a reason for hiding this comment

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

Should we add a persistent notification telling the user to press a button on the device with in 30s ?

Example: https://github.com/home-assistant/home-assistant/blob/3430c1c8bc2b541b29024b633855ebe4cf4ca651/homeassistant/components/switch/broadlink.py#L106

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point! I will extend the PR.

Copy link
Member

@fabaff fabaff left a comment

Choose a reason for hiding this comment

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

🐦

@fabaff fabaff merged commit e95b48c into home-assistant:dev Oct 30, 2017
@balloob balloob mentioned this pull request Nov 2, 2017
for xiaomi_aqara_service in SERVICE_TO_METHOD:
schema = SERVICE_TO_METHOD[xiaomi_aqara_service].get(
'schema', XIAOMI_AQARA_SERVICE_SCHEMA)
service_handler = SERVICE_TO_METHOD[xiaomi_aqara_service].get('method')
Copy link
Member

Choose a reason for hiding this comment

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

This is wrong and has broken the Xiaomi Aqara component. You are passing a string as service handler instead of the actual service. If you would have run this code locally, you would have noticed this right away 🤔

There is no need to make it complicated and use SERVICE_TO_METHOD dict. Instead just directly register the services.

if ring_id is None or gw_sid is None:
_LOGGER.error("Mandatory parameters is not specified.")
return
ring_id = int(call.data.get(ATTR_RINGTONE_ID))
Copy link
Member

Choose a reason for hiding this comment

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

Why cast to int if schema already does this?


ring_id = int(ring_id)
if ring_id in [9, 14-19]:
Copy link
Member

Choose a reason for hiding this comment

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

lol wut? 14-19 is not a range, it's -5

@balloob
Copy link
Member

balloob commented Nov 3, 2017

Come on people, we can do better than this.

@balloob balloob mentioned this pull request Nov 3, 2017
5 tasks
@balloob
Copy link
Member

balloob commented Nov 3, 2017

Put a fix in #10302

@home-assistant home-assistant locked and limited conversation to collaborators Mar 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

xiaomi_aqara issue when device is/was connected to multiple gateways
6 participants