Add Broadlink switch support #4528
Conversation
|
Great work.
|
|
Please commit! |
|
I'm already committed? |
|
Is this working with Broadlink RM 3 Mini? |
|
@dony71 Yes |
|
Hey, I want to implement support for Broadlink SP2/Mini smart plugs, which work pretty similar to RM2/3 except for the on/off/status read packets: How do you think it would make sense to join those with this module? maybe splitting the broadlink common functions somewhere and make the smartplug support to a separate component? or adding a |
|
Hi, |
|
I noticed there is work being done for harmony devices and a remote component is being created instead of a switch #4254 |
|
I've fixed python-broadlink for Python3 on mjg59/python-broadlink#20, i'll try to give a shot to integrating it. |
| if auth: | ||
| broadlink.send_data(base64.b64decode(self._command_off)) | ||
|
|
||
|
|
There was a problem hiding this comment.
I think that @balloob will require that the Broadlink class is exported to an external library before this can be merged
There was a problem hiding this comment.
Where is the right place to export class?
| broadlink.send_data(base64.b64decode(self._command_on)) | ||
|
|
||
| def turn_off(self, **kwargs): | ||
| """Turn the device off.""" |
There was a problem hiding this comment.
To avoid duplicating the code from turn_on, you can extract the code to a _send_data function that both the turn_on and turn_off function can use
| class BroadlinkSwitch(SwitchDevice): | ||
| """Representation of an Broadlink switch.""" | ||
|
|
||
| def __init__(self, hass, object_id, friendly_name, command_on, |
| def turn_on(self, **kwargs): | ||
| """Turn the device on.""" | ||
| import base64 | ||
| self._state = True |
There was a problem hiding this comment.
Should only be sat to True if successfully sent the signal, so should be moved to end of function
| _LOGGER.info('Broadlink connection successfully established.') | ||
|
|
||
| except ValueError as error: | ||
| _LOGGER.error(error) |
There was a problem hiding this comment.
Add return statement here?
| except ValueError as error: | ||
| _LOGGER.error(error) | ||
|
|
||
| auth = broadlink.auth() |
| _LOGGER = logging.getLogger(__name__) | ||
|
|
||
| SWITCH_SCHEMA = vol.Schema({ | ||
| vol.Optional(CONF_COMMAND_OFF, default='true'): cv.string, |
| self._mac_addr = mac_addr | ||
|
|
||
| @staticmethod | ||
| def _switch(command): |
| """Return if the state is based on assumptions.""" | ||
| return self._optimistic | ||
|
|
||
| def update(self): |
There was a problem hiding this comment.
Not necessary, so should be removed
| vol.Optional(CONF_COMMAND_OFF, default=None): cv.string, | ||
| vol.Optional(CONF_COMMAND_ON, default=None): cv.string, | ||
| vol.Optional(CONF_FRIENDLY_NAME, default=DEFAULT_NAME): cv.string, | ||
| vol.Optional(CONF_OPTIMISTIC, default=True): cv.boolean, |
There was a problem hiding this comment.
Why would optimistic be configurable ? We should either be optimistic or not based on how the device reports their state ?
There was a problem hiding this comment.
I have one of these devices, and to my knowledge it doesn't keep state. Maybe this component was adapted from one that does?
|
|
||
| def setup_platform(hass, config, add_devices, discovery_info=None): | ||
| """Setup Broadlink switches.""" | ||
| import binascii |
There was a problem hiding this comment.
This is a Python built-in module, please add import to the top.
|
|
||
| def _sendpacket(self, packet): | ||
| """Send packet to device.""" | ||
| import base64 |
| for object_id, device_config in devices.items(): | ||
|
|
||
| switches.append( | ||
| BroadlinkSwitch( |
There was a problem hiding this comment.
Possible to add an auth check before each device gets added and report that back to the user if they added some wrong info ?
| _LOGGER.error(error) | ||
|
|
||
|
|
||
| class Broadlink(): |
There was a problem hiding this comment.
Please extract this into a third party library. Home Assistant should not contain any protocol specific info.
| @@ -0,0 +1,238 @@ | |||
| """Script to enter Broadlink RM devices in learning mode.""" | |||
There was a problem hiding this comment.
This should also be part of your library. You can register a service in your switch platform to make this functionality available through Home Assistant.
| @@ -40,10 +40,10 @@ apcaccess==0.0.4 | |||
| apns2==0.1.1 | |||
|
|
|||
| # homeassistant.components.sun | |||
| astral==1.3.2 | |||
| astral==1.2 | |||
There was a problem hiding this comment.
You should not change these values.
|
#4834 builds on this, and was released in 0.35 |
Description:
This
Broadlink RMswitch platform allow to you control Broadlink RM2 Pro and RM mini IR+RF devicesPull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#1485
Example entry for
configuration.yaml(if applicable):Configuration variables:
How to obtain IR/RF packet while in learning mode?
Run: hass --script broadlink --ip
YOUR_DEVICE_IP--macYOUR_DEVICE_MACChecklist:
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..coveragerc.If the code does not interact with devices:
toxrun successfully. Your PR cannot be merged unless tests pass