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
Added lumitek/ankuoo recswitch component #15764
Added lumitek/ankuoo recswitch component #15764
Conversation
Hi @marcolertora, It seems you haven't yet signed a CLA. Please do so here. Once you do that we will be able to review and accept this pull request. Thanks! |
|
||
def get_gpio_status(self): | ||
from pyrecswitch import RecSwitchProtocol | ||
return self.send_packet(RecSwitchProtocol.get_gpio_status(self.device_config, flag=0)) |
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.
line too long (94 > 79 characters)
|
||
def set_gpio_status(self, state): | ||
from pyrecswitch import RecSwitchProtocol | ||
return self.send_packet(RecSwitchProtocol.set_gpio_status(self.device_config, flag=0, state=state)) |
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.
line too long (107 > 79 characters)
|
||
def heart_beat(self): | ||
from pyrecswitch import RecSwitchProtocol | ||
return self.send_packet(RecSwitchProtocol.heart_beat(self.device_config)) |
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.
line too long (81 > 79 characters)
|
||
def send_packet(self, packet): | ||
self.device_config.next_message_index() | ||
return self.connection_pool.send_packet(packet, self.ip_address, self.port) |
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.
line too long (83 > 79 characters)
if not self.gpio_state == response.state: | ||
self.gpio_state = response.state | ||
self.schedule_update_ha_state() | ||
_LOGGER.info('command: {} set state to: {}'.format(RecSwitchCommand(command).name, self.gpio_state)) |
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.
line too long (116 > 79 characters)
|
||
|
||
|
||
|
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 at end of file
|
||
hass.data[DATA_RSH] = RecSwitchUDPServerProtocol() | ||
listener = hass.loop.create_datagram_endpoint(lambda: hass.data[DATA_RSH], | ||
local_addr=(local_ip_address, local_ip_port)) |
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.
line too long (95 > 79 characters)
|
||
def packet_received(self, remote_address, device_config, command, response): | ||
if device_config.mac_address in self.devices: | ||
self.devices[device_config.mac_address].receive_packet(command, response) |
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.
line too long (89 > 79 characters)
class RecSwitchUDPServerProtocol(RecSwitchServerProtocol): | ||
devices = dict() | ||
|
||
def packet_received(self, remote_address, device_config, command, response): |
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.
line too long (84 > 79 characters)
def setup(hass, base_config): | ||
from pyrecswitch import RecSwitchDeviceConfig, RecSwitchServerProtocol | ||
config = base_config.get(DOMAIN, dict()) | ||
local_ip_port = config.get(CONF_PORT, RecSwitchDeviceConfig.default_udp_port) |
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.
line too long (81 > 79 characters)
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.
More code should move to the interface library.
updated the pyrecswitch and move there all the device logic |
Could you help me to understand what is the problem with the Travis CI failure? |
Run There are also some missing docstrings. |
fixed |
Support for Ankuoo RecSwitch MS6126 devices. | ||
|
||
For more details about this platform, please refer to the documentation at | ||
https://home-assistant.io/components/switch.recswitch/ |
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.
The url is wrong.
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 write the documentation.. I promise!
|
||
async def async_setup(hass, base_config): | ||
"""Setup recswitch network and start datagram endpoint.""" | ||
config = base_config.get(DOMAIN, dict()) |
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.
config = base_config[DOMAIN]
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, but this change willl make the component config section required. I mean....
recswitch:
switch01:
- platform: recswitch
name: Garden Lights
host: 192.168.X.X
mac: 'F0:FE:6B:XX:XX:XX'
It should be right? or I miss something?
actually, I read more more than one time the developer doc but I'm still unsure about if I really need to create a component and a platform for my devices..
I found this way to share the udp socket server across more platform instances. This is required because all this devices send udp message to the same port to the host.
Suggestions are welcome!
Thanks
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 we want to share some device connection between platform types a component is required. We are also moving to use config entries instead as an alternative to the config sections in yaml, and then a component is required.
If you're planning to add more platform types, keep the component. But if there will only be the switch platform it would also work to save the server instance in hass.data
and reuse it for many switch platform instances.
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, reusing the server instance for more switch is what I thought as alternative.
I don't known if exists other kind of devices based on this protocol (such as lights or sensors...) I've no connection with the manufactur I've just bought an ankuoo device thinking it was already supported (I misunderstood a thread on the forum ..) so I spent 'some' hours in reverse engineering..
May be sharing the server instance across switch devices is the easiest way.. I'll code in this way.
Remove component leave only the 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.
We can return the mac address in the unique id property.
I still think it's weird that we both have a callback for state updates and have the entity be polling. When is the callback called?
Yes it's a bit weird but the callback alone is not enough. I think the polling is required because of the broadcast propagation limit and the missing information about initial state. So, we can, at least, remove the callback and leave only the polling. |
Sounds good to remove the callback. |
Please don't squash commits after review has started. Squashing makes it hard to follow changes. |
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.
Looks good. Some minor comments left.
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.
Good!
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.
Sorry, please add the module to .coveragerc
.
Also add a link to a docs PR. All of these instructions were in the PR template that you removed. It would have been good to keep it. |
HI, |
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! Can be merged when build passes.
In case someone is interested, I've made a new firmware for these devices. |
Description:
Added support for lumitek/ankuoo recswitch to homeassistant.
the device is known as:
Searching on the web it look like the device is made by lumitek and branded and sold on amazon by ankuoo:
The chipset inside the device should be the HF-LPB100.
Home Assistant Support has been discussed and requested in this thread:
Example entry for
configuration.yaml
:Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#6548
Checklist:
REQUIREMENTS
variable.requirements_all.txt
by runningscript/gen_requirements_all.py
..coveragerc
.