Telnet switch#8913
Conversation
emlove
left a comment
There was a problem hiding this comment.
Looks great overall! Just a few small questions/requests.
| vol.Required(CONF_COMMAND_ON): cv.string, | ||
| vol.Optional(CONF_COMMAND_STATE): cv.string, | ||
| vol.Optional(CONF_FRIENDLY_NAME): cv.string, | ||
| vol.Optional(CONF_OPTIMISTIC): cv.boolean, |
There was a problem hiding this comment.
Can we drop the optimistic configuration and just assume it's optimistic if there's no command state defined, or is there something I'm missing? For example, assumed_state can just be return self._command_state is None.
| """Update device state.""" | ||
| response = self.__telnet_command(self._command_state) | ||
| if response: | ||
| _LOGGER.error( |
There was a problem hiding this comment.
This should be debug level, correct?
| self._optimistic = optimistic | ||
| self._value_template = value_template | ||
|
|
||
| def __telnet_command(self, command): |
There was a problem hiding this comment.
Small nitpick, but single underscore prefix is preferred for normal internal methods.
|
Updated PR. |
|
Good to go once the docs PR is up-to-date. |
|
Should the encoding be hardcoded to ASCII? Doesn't it depend on what the host is configured to accept? |
|
Here it states that Telnet uses ASCII encoding: http://www.pcmicro.com/netfoss/telnet.html |
| """Update device state.""" | ||
| response = self._telnet_command(self._command_state) | ||
| if response: | ||
| _LOGGER.info( |
There was a problem hiding this comment.
Don't log this. Maybe a debug but not more.
|
I think if someone have a no ASCII Telnet device, he can make a PR for set encoding as options. |
| vol.Required(CONF_COMMAND_OFF): cv.string, | ||
| vol.Required(CONF_COMMAND_ON): cv.string, | ||
| vol.Optional(CONF_COMMAND_STATE): cv.string, | ||
| vol.Optional(CONF_FRIENDLY_NAME): cv.string, |
There was a problem hiding this comment.
Please use CONF_NAME instead of CONF_FRIENDLY_NAME.
|
|
||
| DEFAULT_PORT = 23 | ||
|
|
||
| SWITCH_SCHEMA = vol.Schema({ |
There was a problem hiding this comment.
Would be nice to keep it ordered.
|
|
||
| # pylint: disable=unused-argument | ||
| def setup_platform(hass, config, add_devices, discovery_info=None): | ||
| """Find and return switches controlled by shell commands.""" |
|
Ping @multiholle |
|
I'm quite busy at the moment. I'm trying to get back to the PR next week. |
|
How can I use this addon when authentication is required in order to perform the on/off commands |
|
Please use the forum or chat for support questions. |
Description:
New platform telnet switch. Allows to control a device with telnet commands for on and off. Can fetch the state of the device or work as optimistic switch.
Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#3165
Example entry for
configuration.yaml(if applicable):Checklist:
If user exposed functionality or configuration variables are added/changed:
.coveragerc.If the code communicates with devices, web services, or third-party tools:
toxrun successfully. Your PR cannot be merged unless tests pass