sensor.snmp added decoding of "Opaque" types. #11239

ChristianKuehnel · 2017-12-19T23:07:36Z

Description:

SNMP does not support floating point data as native data types. It rather uses asn1 encoding and transfers them as bytes. The sensor.snmp uses pysnmp and that in turn uses pyasn1 to encode/decode these things. So as long as the encoding is supported by pyasn1, this change should be able to decode it.

Related issue (if applicable): fixes #2767

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

Example entry for `configuration.yaml` (if applicable):

see #2767

Checklist:

If the code does not interact with devices:

Local tests with tox run successfully. Your PR cannot be merged unless tests pass
Tests have been added to verify that the new code works.

Also added test cases for String, Integer and Real types.

houndci-bot · 2017-12-19T23:07:41Z

tests/components/sensor/test_snmp.py

+            udp.UdpTransport().openServerMode(('127.0.0.1', _PORT))
+        )
+        config.addV1System(self.snmpEngine, 'my-area', 'public')
+        config.addVacmUser(self.snmpEngine, 2, 'my-area', 'noAuthNoPriv', _BASE_OID)


line too long (84 > 79 characters)

houndci-bot · 2017-12-19T23:07:41Z

tests/components/sensor/test_snmp.py

+        if self.snmpEngine is not None:
+            self.snmpEngine.transportDispatcher.jobFinished(1)
+            self.snmpEngine.transportDispatcher.unregisterRecvCbFun(recvId=None)
+            self.snmpEngine.transportDispatcher.unregisterTransport(udp.domainName)


line too long (83 > 79 characters)

houndci-bot · 2017-12-19T23:07:41Z

tests/components/sensor/test_snmp.py

+    def tearDown(self):
+        if self.snmpEngine is not None:
+            self.snmpEngine.transportDispatcher.jobFinished(1)
+            self.snmpEngine.transportDispatcher.unregisterRecvCbFun(recvId=None)


line too long (80 > 79 characters)

houndci-bot · 2017-12-19T23:07:41Z

tests/components/sensor/test_snmp.py

@@ -0,0 +1,144 @@
+import unittest
+import os


'os' imported but unused

fanaticDavid · 2017-12-19T23:48:53Z

First of all, I would like to say thank you for giving this a go. Quite honestly, I had lost all hope that someone would ever look into this.

However, I replaced the snmp.py file on my system with your version and after restarting Home Assistant, I got the following error for each of my 4 broken snmp sensors:

2017-12-20 00:42:21 ERROR (MainThread) [homeassistant.components.sensor] Error on device update!
Traceback (most recent call last):
  File "/srv/hass_venv/lib/python3.6/site-packages/homeassistant/helpers/entity_component.py", line 217, in async_add_entity
    yield from entity.async_device_update(warning=False)
  File "/srv/hass_venv/lib/python3.6/site-packages/homeassistant/helpers/entity.py", line 306, in async_device_update
    yield from self.hass.async_add_job(self.update)
  File "/usr/local/lib/python3.6/asyncio/futures.py", line 332, in __iter__
    yield self  # This tells Task to wait for completion.
  File "/usr/local/lib/python3.6/asyncio/tasks.py", line 250, in _wakeup
    future.result()
  File "/usr/local/lib/python3.6/asyncio/futures.py", line 245, in result
    raise self._exception
  File "/usr/local/lib/python3.6/concurrent/futures/thread.py", line 56, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/srv/hass_venv/lib/python3.6/site-packages/homeassistant/components/sensor/snmp.py", line 122, in update
    self.data.update()
  File "/srv/hass_venv/lib/python3.6/site-packages/homeassistant/components/sensor/snmp.py", line 183, in update
    value, _ = decoder.decode(bytes(resrow[-1]))
  File "/srv/hass_venv/lib/python3.6/site-packages/pyasn1/codec/ber/decoder.py", line 1183, in __call__
    '%s not in asn1Spec: %r' % (tagSet, asn1Spec)
pyasn1.error.PyAsn1Error: [128:0:120] not in asn1Spec: None

fabaff · 2017-12-20T09:18:20Z

homeassistant/components/sensor/snmp.py

-                self.value = str(resrow[-1])
+                self.value = self._decode_value(resrow[-1])
+
+    def _decode_value(self, value):


Docstring is missing.

ChristianKuehnel · 2017-12-20T09:20:11Z

Getting closer:

you can run a ready-to-use SNMP agent using this docker image: https://hub.docker.com/r/polinux/snmpd/
after tinkering with the config file, I now get a float number for the current system load so I have something to test:

09:38 $ snmpget -v2c -c public localhost 1.3.6.1.4.1.2021.10.1.6.1 
iso.3.6.1.4.1.2021.10.1.6.1 = Opaque: Float: 0.787109

In the sensor.dnmp this gets me an <class 'pysnmp.proto.rfc1902.Opaque'> with data b'\x9fx\x04>\xde@\x00' with this config:

sensor:
  - platform: snmp
    host: 127.0.0.1
    baseoid: "1.3.6.1.4.1.2021.10.1.6.1"
    name: stringvar
    community: public
    port: 161

logger:
  default: info
  logs:
    homeassistant.components.sensor.snmp: debug

it seems pysnmp/pyasn1 does not support Float as defined in https://tools.ietf.org/html/draft-perkins-opaque-01 so I had to decode it myself using struct.unpack

So right now I can successfully decode a Float. Not sure if that is the most elegant solution...

fabaff · 2017-12-20T09:20:16Z

tests/components/sensor/test_snmp.py

+_OID3 = _BASE_OID + (3, 1, 1)
+
+
+class TestSnmp(unittest.TestCase):


Please add docstrings to the whole file. Thanks.

…into snmp

…nt into snmp

houndci-bot · 2017-12-20T09:23:10Z

tests/components/sensor/test_snmp.py

+from pysnmp.carrier.asyncore.dgram import udp
+from pysnmp.proto.api import v2c
+from pyasn1.codec.ber import encoder
+from pyasn1.type import univ


'pyasn1.type.univ' imported but unused

houndci-bot · 2017-12-20T09:23:10Z

tests/components/sensor/test_snmp.py

+from pysnmp.entity.rfc3413 import cmdrsp, context
+from pysnmp.carrier.asyncore.dgram import udp
+from pysnmp.proto.api import v2c
+from pyasn1.codec.ber import encoder


'pyasn1.codec.ber.encoder' imported but unused

ChristianKuehnel · 2017-12-20T09:31:48Z

I also added a feature request here: etingof/pysnmp#118

…rying to read from them

etingof · 2017-12-20T12:20:43Z

Just a quick note that you may also use pyasn1 infrastructure for reliably decoding whatever ASN.1 structures should occur inside the Opaque SNMP type.

fanaticDavid · 2017-12-23T00:35:12Z

I just replaced my snmp.py file once more and I can't believe it...it's working! 😮

Thank you so much, @ChristianKuehnel ! I know this can still be improved when @etingof implements your FR into pysnmp, but this is awesome already in my book! 👍

ChristianKuehnel · 2017-12-23T21:21:09Z

Hey @etingof

if I understand it correctly, the pyasn1 infrastructure does depend on some C/C++ libraries?
If so: I'm concerned about the portability of the code. I guess not all operating systems ship these native libraries...

etingof · 2017-12-23T21:22:46Z

if I understand it correctly, the pyasn1 infrastructure does depend on some C/C++ libraries?

Hey, no! pyasn1 is a pure-Python tool and has no external dependencies. pysnmp is totally dependent on pyasn1.

ChristianKuehnel · 2017-12-23T21:33:51Z

OK good.
Looking at the library, I did not see support for a "Float" data type. Did I miss something?

etingof · 2017-12-24T06:50:54Z

Right, there is no Float type in ASN.1 because they borrowed it from the IEEE standard. So unpacking float itself look good to me.

What I meant though is that the wrapping BER encoding could be handled by pyasn1 like this:

floatType = OctetString().subtype(
    implicitTag=Tag(tagClassContext, tagFormatSimple, 120),
    subtypeSpec=ValueSizeConstraint(4, 4)
)
ieee_encoded_float, _ = decode(b'\x9f\x78\x04\x00\x00\x00\x00', asn1Spec=floatType)

fanaticDavid · 2018-01-01T03:49:21Z

Hmmm I can't seem to apply a value_template to Opaque types.

I have the following SNMP sensors (the bottom 4 are Opaque types):

I wish to change the shown state for the battery voltage sensor (for obvious reasons) using the following: value_template: '{{ (value | float) | round(1) }}'.
After restarting Home Assistant, this results in the following error in the log file:

2018-01-01 04:34:02 ERROR (MainThread) [homeassistant.components.sensor] Error on device update!
Traceback (most recent call last):
  File "/srv/hass_venv/lib/python3.6/site-packages/homeassistant/helpers/entity_component.py", line 217, in async_add_entity
    yield from entity.async_device_update(warning=False)
  File "/srv/hass_venv/lib/python3.6/site-packages/homeassistant/helpers/entity.py", line 306, in async_device_update
    yield from self.hass.async_add_job(self.update)
  File "/usr/local/lib/python3.6/asyncio/futures.py", line 332, in __iter__
    yield self  # This tells Task to wait for completion.
  File "/usr/local/lib/python3.6/asyncio/tasks.py", line 250, in _wakeup
    future.result()
  File "/usr/local/lib/python3.6/asyncio/futures.py", line 245, in result
    raise self._exception
  File "/usr/local/lib/python3.6/concurrent/futures/thread.py", line 56, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/srv/hass_venv/lib/python3.6/site-packages/homeassistant/components/sensor/snmp.py", line 131, in update
    value, STATE_UNKNOWN)
  File "/srv/hass_venv/lib/python3.6/site-packages/homeassistant/helpers/template.py", line 129, in render_with_possible_json_value
    error_value).result()
  File "/usr/local/lib/python3.6/concurrent/futures/_base.py", line 432, in result
    return self.__get_result()
  File "/usr/local/lib/python3.6/concurrent/futures/_base.py", line 384, in __get_result
    raise self._exception
  File "/srv/hass_venv/lib/python3.6/site-packages/homeassistant/util/async.py", line 167, in run_callback
    future.set_result(callback(*args))
  File "/srv/hass_venv/lib/python3.6/site-packages/homeassistant/helpers/template.py", line 147, in async_render_with_possible_json_value
    variables['value_json'] = json.loads(value)
  File "/usr/local/lib/python3.6/json/__init__.py", line 348, in loads
    'not {!r}'.format(s.__class__.__name__))
TypeError: the JSON object must be str, bytes or bytearray, not 'float'

I know my template is fine as I'm able to use it with other sensors (e.g. InfluxDB sensors).

ChristianKuehnel · 2018-01-01T13:17:10Z

Hmm, OK. I added a type cast to string before returning the float. Can you give it another shot?

fanaticDavid · 2018-01-01T14:17:17Z

@ChristianKuehnel Just tested your change, it works! 😀

ChristianKuehnel · 2018-01-01T14:28:16Z

weird test results --> closing/opening to trigger new build.
Here the build was successful: https://travis-ci.org/ChristianKuehnel/home-assistant/builds/323770286

ChristianKuehnel · 2018-01-02T20:28:06Z

This is weird. The error on Travis is:

requirements_test_all.txt is not up to date
Please run script/gen_requirements_all.py
ERROR: InvocationError: '/home/travis/build/home-assistant/home-assistant/.tox/lint/bin/python script/gen_requirements_all.py validate'

If I run the check locally, it passes.
If I run script/gen_requirements_all.py, there is not change on the file system, I cannot commit anything...
--> not sure how to fix this.

ChristianKuehnel · 2018-01-13T09:46:51Z

problem resolved with a merge of Dev and a re-run of gen_requirements_all

fanaticDavid · 2018-01-16T21:48:53Z

Should this have been included in 0.61.0? I've been using the code in this PR for the past 2 weeks without issues 😁

ChristianKuehnel · 2018-01-16T22:14:58Z

As the change was not yet merged, it's not contained in the 0.61 release.

It seems with my latest merge of the Dev branch I broke some tests. I'll have a investigate this on the weekend...

MartinHjelmare · 2018-02-07T09:47:14Z

homeassistant/components/sensor/snmp.py

+                decoded_value, _ = decoder.decode(bytes(value))
+                return str(decoded_value)
+            # pylint: disable=broad-except
+            except Exception as decode_exception:


Isn't it possible to catch a more specific error, like ValueError?

MartinHjelmare · 2018-02-07T09:47:51Z

homeassistant/components/sensor/snmp.py

+            # pylint: disable=broad-except
+            except Exception as decode_exception:
+                _LOGGER.error('SNMP error in decoding opaque type: %s',
+                              str(decode_exception))


Copy to string shouldn't be necessary.

MartinHjelmare · 2018-02-07T09:49:08Z

tests/components/sensor/test_snmp.py

+    """Test basic functionality of sensor.snmp."""
+
+    def setUp(self):
+        """Setup a hoss instance for testing."""


MartinHjelmare · 2018-02-07T09:55:11Z

tests/components/sensor/test_snmp.py

+        config.addTransport(
+            self.snmpEngine,
+            udp.domainName,
+            udp.UdpTransport().openServerMode(('127.0.0.1', _PORT))


I think the engine should be mocked. Why do we need a real engine? We only need to test the home assistant side of the logic.

balloob · 2018-04-09T04:17:39Z

what's the status?

pvizeli · 2018-04-17T11:04:06Z

Please reopen the PR after all comments are addressed

sensor.snmp added decoding of "Opaque" types.

839c2eb

Also added test cases for String, Integer and Real types.

homeassistant added cla-signed platform: sensor.snmp labels Dec 19, 2017

houndci-bot reviewed Dec 19, 2017

View reviewed changes

ChristianKuehnel mentioned this pull request Dec 19, 2017

SNMP sensor not processing float values (correctly) #2767

Closed

making the hound happy

15c07bd

homeassistant added the cla-signed label Dec 19, 2017

added more log output

bc8cfce

fabaff reviewed Dec 20, 2017

View reviewed changes

implemented manual unpacking of Float values

b674269

fabaff reviewed Dec 20, 2017

View reviewed changes

ChristianKuehnel added 2 commits December 20, 2017 10:20

Merge branch 'dev' of ssh://github.com/home-assistant/home-assistant …

31d6fbb

…into snmp

Merge branch 'snmp' of ssh://github.com/ChristianKuehnel/home-assista…

2d82560

…nt into snmp

houndci-bot reviewed Dec 20, 2017

View reviewed changes

cleanup of the code

b8cf07e

ChristianKuehnel added 3 commits December 20, 2017 11:26

ran script/gen_requirements_all.py

2b80ac3

fixed pylint findings

938f37b

added blocking to make sure that the components were created before t…

2f1a31e

…rying to read from them

balloobbot added the platform label Dec 20, 2017

fixed test requirements

be12343

homeassistant added the cla-signed label Dec 20, 2017

ChristianKuehnel added 2 commits December 20, 2017 11:47

fixed comment

678407f

fixed comment

f0f74ef

added type case to string before returning the value

7ca57f4

homeassistant added the cla-signed label Jan 1, 2018

ChristianKuehnel closed this Jan 1, 2018

ChristianKuehnel reopened this Jan 1, 2018

ChristianKuehnel added 2 commits January 13, 2018 10:44

Merge remote-tracking branch 'upstream/dev' into snmp

29b5b46

re-run of gen_requirements

d8ace49

homeassistant added the cla-signed label Jan 13, 2018

MartinHjelmare reviewed Feb 7, 2018

View reviewed changes

pvizeli closed this Apr 17, 2018

home-assistant locked and limited conversation to collaborators Jul 26, 2018

ghost added integration: snmp and removed platform: sensor.snmp labels Mar 21, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sensor.snmp added decoding of "Opaque" types. #11239

sensor.snmp added decoding of "Opaque" types. #11239

ChristianKuehnel commented Dec 19, 2017 •

edited

houndci-bot Dec 19, 2017

houndci-bot Dec 19, 2017

houndci-bot Dec 19, 2017

houndci-bot Dec 19, 2017

fanaticDavid commented Dec 19, 2017

fabaff Dec 20, 2017

ChristianKuehnel Dec 20, 2017

ChristianKuehnel commented Dec 20, 2017

fabaff Dec 20, 2017

ChristianKuehnel Dec 20, 2017

houndci-bot Dec 20, 2017

houndci-bot Dec 20, 2017

ChristianKuehnel commented Dec 20, 2017

etingof commented Dec 20, 2017

fanaticDavid commented Dec 23, 2017 •

edited

ChristianKuehnel commented Dec 23, 2017

etingof commented Dec 23, 2017

ChristianKuehnel commented Dec 23, 2017

etingof commented Dec 24, 2017

fanaticDavid commented Jan 1, 2018 •

edited

ChristianKuehnel commented Jan 1, 2018

fanaticDavid commented Jan 1, 2018

ChristianKuehnel commented Jan 1, 2018 •

edited

ChristianKuehnel commented Jan 2, 2018

ChristianKuehnel commented Jan 13, 2018

fanaticDavid commented Jan 16, 2018

ChristianKuehnel commented Jan 16, 2018

MartinHjelmare Feb 7, 2018

MartinHjelmare Feb 7, 2018

MartinHjelmare Feb 7, 2018

MartinHjelmare Feb 7, 2018

balloob commented Apr 9, 2018

pvizeli commented Apr 17, 2018

		_OID3 = _BASE_OID + (3, 1, 1)


		class TestSnmp(unittest.TestCase):

sensor.snmp added decoding of "Opaque" types. #11239

sensor.snmp added decoding of "Opaque" types. #11239

Conversation

ChristianKuehnel commented Dec 19, 2017 • edited

Description:

Example entry for configuration.yaml (if applicable):

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fanaticDavid commented Dec 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChristianKuehnel commented Dec 20, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChristianKuehnel commented Dec 20, 2017

etingof commented Dec 20, 2017

fanaticDavid commented Dec 23, 2017 • edited

ChristianKuehnel commented Dec 23, 2017

etingof commented Dec 23, 2017

ChristianKuehnel commented Dec 23, 2017

etingof commented Dec 24, 2017

fanaticDavid commented Jan 1, 2018 • edited

ChristianKuehnel commented Jan 1, 2018

fanaticDavid commented Jan 1, 2018

ChristianKuehnel commented Jan 1, 2018 • edited

ChristianKuehnel commented Jan 2, 2018

ChristianKuehnel commented Jan 13, 2018

fanaticDavid commented Jan 16, 2018

ChristianKuehnel commented Jan 16, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

balloob commented Apr 9, 2018

pvizeli commented Apr 17, 2018

ChristianKuehnel commented Dec 19, 2017 •

edited

Example entry for `configuration.yaml` (if applicable):

fanaticDavid commented Dec 23, 2017 •

edited

fanaticDavid commented Jan 1, 2018 •

edited

ChristianKuehnel commented Jan 1, 2018 •

edited