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
Allow separate command and state OIDs and payloads in SNMP switch #11075
Conversation
Hi @nkaminski, 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! |
[SnmpSwitch(name, host, port, community, baseoid, version, payload_on, | ||
payload_off)], True) | ||
[SnmpSwitch(name, host, port, community, baseoid, command_oid, version, payload_on, | ||
payload_off, command_payload_on, command_payload_off)], True) |
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)
version = config.get(CONF_VERSION) | ||
payload_on = config.get(CONF_PAYLOAD_ON) | ||
payload_off = config.get(CONF_PAYLOAD_OFF) | ||
|
||
add_devices( | ||
[SnmpSwitch(name, host, port, community, baseoid, version, payload_on, | ||
payload_off)], True) | ||
[SnmpSwitch(name, host, port, community, baseoid, command_oid, version, payload_on, |
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 (91 > 79 characters)
@@ -36,6 +39,9 @@ | |||
|
|||
PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({ | |||
vol.Required(CONF_BASEOID): cv.string, | |||
vol.Optional(CONF_COMMAND_OID, default=None): vol.Any(cv.string, None), | |||
vol.Optional(CONF_COMMAND_PAYLOAD_ON, default=None): vol.Any(cv.string, None), | |||
vol.Optional(CONF_COMMAND_PAYLOAD_OFF, default=None): vol.Any(cv.string, None), |
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)
@@ -36,6 +39,9 @@ | |||
|
|||
PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({ | |||
vol.Required(CONF_BASEOID): cv.string, | |||
vol.Optional(CONF_COMMAND_OID, default=None): vol.Any(cv.string, None), | |||
vol.Optional(CONF_COMMAND_PAYLOAD_ON, default=None): vol.Any(cv.string, None), |
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 (82 > 79 characters)
Hi @nkaminski, 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! |
payload_off)], True) | ||
[SnmpSwitch(name, host, port, community, baseoid, command_oid, version, | ||
payload_on, payload_off, | ||
command_payload_on, command_payload_off)], True) |
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.
continuation line under-indented for visual indent
[SnmpSwitch(name, host, port, community, baseoid, version, payload_on, | ||
payload_off)], True) | ||
[SnmpSwitch(name, host, port, community, baseoid, command_oid, version, | ||
payload_on, payload_off, |
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.
continuation line under-indented for visual indent
vol.Optional(CONF_COMMAND_PAYLOAD_ON, default=None): | ||
vol.Any(cv.string, None), | ||
vol.Optional(CONF_COMMAND_PAYLOAD_OFF, default=None): | ||
vol.Any(cv.string, None), |
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.
continuation line unaligned for hanging indent
vol.Optional(CONF_COMMAND_OID, default=None): | ||
vol.Any(cv.string, None), | ||
vol.Optional(CONF_COMMAND_PAYLOAD_ON, default=None): | ||
vol.Any(cv.string, None), |
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.
continuation line unaligned for hanging indent
@@ -36,6 +39,12 @@ | |||
|
|||
PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({ | |||
vol.Required(CONF_BASEOID): cv.string, | |||
vol.Optional(CONF_COMMAND_OID, default=None): | |||
vol.Any(cv.string, None), |
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.
continuation line unaligned for hanging indent
vol.Optional(CONF_COMMAND_PAYLOAD_OFF, default=None): | ||
vol.Any( | ||
cv.string, | ||
None), |
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.
Don't set it default None and do not allow set None as payload like others. Please use the same style
if commandoid: | ||
self._commandoid = commandoid | ||
else: | ||
self._commandoid = baseoid |
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.
use style: ... = commandiod or baseoid
if command_payload_on: | ||
self._command_payload_on = command_payload_on | ||
else: | ||
self._command_payload_on = payload_on |
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.
same too
if command_payload_off: | ||
self._command_payload_off = command_payload_off | ||
else: | ||
self._command_payload_off = payload_off |
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.
same too
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.
Changes made as requested
Follow PEP8 style properly More PEP8 style fixes Fully PEP8 compliant
@pvizeli Changes made as requested. Ready for review. |
Awesome! Congrats on your first contribution! 🎉 🌮 🐬 |
* upstream/master: (465 commits) Update pyhomematic to 0.1.38 (home-assistant#11936) Implement Alexa temperature sensors (home-assistant#11930) Fixed rfxtrx binary_sensor KeyError on missing optional device_class (home-assistant#11925) Allow setting climate devices to AUTO mode via Google Assistant (home-assistant#11923) fixes home-assistant#11848 (home-assistant#11915) Add "write" service to system_log (home-assistant#11901) Update frontend to 20180126.0 Version 0.62 Allow separate command and state OIDs and payloads in SNMP switch (home-assistant#11075) Add ERC20 tokens to etherscan.io sensor (home-assistant#11916) Report scripts and groups as scenes to Alexa (home-assistant#11900) Minor fix to configuration validation and related log line. (home-assistant#11898) Multi-Room Support for Greenwave Reality (home-assistant#11364) Added Xeoma camera platform (home-assistant#11619) Improve foscam library exception support (home-assistant#11701) Iota wallet (home-assistant#11398) New venstar climate component (home-assistant#11639) Update python-wink version and multiple wink fixes/updates. (home-assistant#11833) Use API to discover Hue if no bridges specified (home-assistant#11909) Clarify emulated hue warning (home-assistant#11910) ...
Description:
It is evident that some switch devices, such as the PDU/CDU units made by Server Technology, use different OIDs and payloads for reading vs. writing the on/off state of power outlets. Therefore, I have expanded the SNMP switch component in a fully backwards-compatible manner to allow for these devices which use separate command and state OIDs and variable mappings to be used with HA.
Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#4190
Example entry for
configuration.yaml
(if applicable):Checklist:
If user exposed functionality or configuration variables are added/changed: