Skip to content
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

Broadlink sensor issue #11795

Closed
Paxy opened this issue Jan 19, 2018 · 32 comments · Fixed by #12107 or #13761
Closed

Broadlink sensor issue #11795

Paxy opened this issue Jan 19, 2018 · 32 comments · Fixed by #12107 or #13761

Comments

@Paxy
Copy link
Contributor

Paxy commented Jan 19, 2018

Home Assistant release (hass --version):
0.61
But noticed issue few minor releases before.

Python release (python3 --version):
Python 3.5.2
Broadlink lib ver 0.5

Component/platform:
Broadlink Sensor

Description of problem:
On RM2 device, broadlink sensor stops recoding temperature state at some moment. There was no noticeable error in the log. At the same time, RM3 is working perfectly. When HA stops reading temperature, I can still get correct temperature using Broadlink e-Control. HA restart solve the issue. No RM2 restart is required.
My guess that issue is related to resolved issue over 6 months old when HA sometimes read faulty (huge number) temperature. That issue was related to python-broadlink.

Both RM2 and RM3 have the same firmware v20028.

Expected:
In case of RM2 temperature read error, broadlink sensor should restart and reinitiate connection to RM2 so it can keep trying to get temperature value.

@Paxy Paxy changed the title RM3 Broadlink sensor issue RM2 Broadlink sensor issue Jan 19, 2018
@talondnb
Copy link

talondnb commented Jan 19, 2018

I'm getting this too, on 0.61.1.

Update for sensor.living_room_temperature fails
Traceback (most recent call last):
  File "/usr/lib/python3.6/site-packages/homeassistant/helpers/entity.py", line 199, in async_update_ha_state
    yield from self.async_device_update()
  File "/usr/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/lib/python3.6/asyncio/futures.py", line 332, in __iter__
    yield self  # This tells Task to wait for completion.
  File "/usr/lib/python3.6/asyncio/tasks.py", line 250, in _wakeup
    future.result()
  File "/usr/lib/python3.6/asyncio/futures.py", line 245, in result
    raise self._exception
  File "/usr/lib/python3.6/concurrent/futures/thread.py", line 56, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/usr/lib/python3.6/site-packages/homeassistant/components/sensor/broadlink.py", line 96, in update
    self._broadlink_data.update()
  File "/usr/lib/python3.6/site-packages/homeassistant/util/__init__.py", line 306, in wrapper
    result = method(*args, **kwargs)
  File "/usr/lib/python3.6/site-packages/homeassistant/components/sensor/broadlink.py", line 124, in _update
    data = self._device.check_sensors_raw()
  File "/usr/lib/python3.6/site-packages/broadlink/__init__.py", line 449, in check_sensors_raw
    payload = self.decrypt(bytes(response[0x38:]))
  File "/usr/lib/python3.6/site-packages/broadlink/__init__.py", line 174, in decrypt_pycrypto
    return aes.decrypt(bytes(payload))
  File "/usr/lib/python3.6/site-packages/Crypto/Cipher/_mode_cbc.py", line 209, in decrypt
    raise ValueError("Error %d while decrypting in CBC mode" % result)
ValueError: Error 3 while decrypting in CBC mode

@talondnb
Copy link

I'm also getting a bunch of these in the logs:

Update of sensor.living_room_temperature is taking over 10 seconds

@Danielhiversen
Copy link
Member

The Broadlink sensor has not changed since August.

@Danielhiversen
Copy link
Member

Danielhiversen commented Jan 19, 2018

In case of RM2 temperature read error, broadlink sensor should restart and reinitiate connection to RM2 so it can keep trying to get temperature value.

If the received temperature is not between -50 and 150, we reconnect and ask for a new value 3 times. And then we will try again after a while

@Paxy
Copy link
Contributor Author

Paxy commented Jan 21, 2018

Last night the same issue occurred with the RM3 too.

Sensor stopped recording with following warning.
WARNING (MainThread) [homeassistant.helpers.entity] Update of sensor.temp_sprat_temperature is taking over 10 seconds

First error message before sensor read failure is following error, which might be unrelated:

ERROR (MainThread) [homeassistant.core] Error doing job: Fatal read error on socket transport
Traceback (most recent call last):
  File "/usr/lib/python3.5/asyncio/selector_events.py", line 662, in _read_ready
    data = self._sock.recv(self.max_size)
TimeoutError: [Errno 110] Connection timed out

@Paxy
Copy link
Contributor Author

Paxy commented Jan 21, 2018

Also, this related warning appears over time:
WARNING (MainThread) [homeassistant.components.sensor] Updating broadlink sensor took longer than the scheduled update interval 0:00:30

@Paxy
Copy link
Contributor Author

Paxy commented Jan 23, 2018

I have set independent script that collects temperature using broadlink lib. The script is collecting temperature every minute.
Last night, HA stoped recoding states and script showed temperature 249.0 for 9 minutes (measurements) than resume its normal temp collection.

I guess that means that HE sensor is not able to recover after multiple incorrect reads.

@Paxy Paxy changed the title RM2 Broadlink sensor issue Broadlink sensor issue Jan 23, 2018
@Danielhiversen
Copy link
Member

Hmm, can find any errors in the code.
Since I don have this problem, it difficult for me to debug it.

Here is the code https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/sensor/broadlink.py#L102
So maybe you can debug it yourself?

@Paxy
Copy link
Contributor Author

Paxy commented Jan 23, 2018

No, this time no error or warning at all in time of misread or later on.
I will check out the code but my skill with python is not too high. Java is my world :)

@Danielhiversen
Copy link
Member

Danielhiversen commented Jan 23, 2018

Ok, just if there something you do not understand.

The data are validated here: https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/sensor/broadlink.py#L126

If invalid data are received it tries to reauthorize and ask for new data: https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/sensor/broadlink.py#L134

You can try to add
_LOGGER.debug(data)
on line 133.
Then enable debugging for the broadlink sensor

@Paxy
Copy link
Contributor Author

Paxy commented Jan 23, 2018

An idea by looking at the code.
It possible that RM device reboots itself after some time of incorrect data readings. If this happens, it will lose authentication. I will try to add auth() between line 123 and 124. That way auth will be done before every read, witch my script does anyway.

@Danielhiversen
Copy link
Member

@Paxy
Copy link
Contributor Author

Paxy commented Jan 23, 2018

Oh right, it would do reauth on first retry. Ok, I will add some debugging messages so I can see is it some kind of Throttle class issue that stops triggering update.

@Danielhiversen
Copy link
Member

If the data object does not contain the type we are asking this will fail:
https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/sensor/broadlink.py#L99

So it would be possible to change that line to
self._state = self._broadlink_data.data.get(self._type)

Then self._state will be sat to None and displayed in the gui as Unknown

@Paxy
Copy link
Contributor Author

Paxy commented Jan 23, 2018

Hmm, that does not pass at update:

Traceback (most recent call last):
  File "/usr/local/lib/python3.5/dist-packages/homeassistant/helpers/entity_component.py", line 217, in async_add_entity
    yield from entity.async_device_update(warning=False)
  File "/usr/local/lib/python3.5/dist-packages/homeassistant/helpers/entity.py", line 306, in async_device_update
    yield from self.hass.async_add_job(self.update)
  File "/usr/lib/python3.5/asyncio/futures.py", line 361, in __iter__
    yield self  # This tells Task to wait for completion.
  File "/usr/lib/python3.5/asyncio/tasks.py", line 296, in _wakeup
    future.result()
  File "/usr/lib/python3.5/asyncio/futures.py", line 274, in result
    raise self._exception
  File "/usr/lib/python3.5/concurrent/futures/thread.py", line 55, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/usr/local/lib/python3.5/dist-packages/homeassistant/components/sensor/broadlink.py", line 99, in update
    self._state = self._broadlink_data.data(self._type)
TypeError: 'dict' object is not callable

@Danielhiversen
Copy link
Member

You are missing a get

self._state = self._broadlink_data.data.get(self._type)

@Paxy
Copy link
Contributor Author

Paxy commented Jan 23, 2018

Oh right. Sorry.
I have added some debug messages to display data structure that device return in case of a fault.
Maybe this already solves it, but we will see next time it happens. (Probably in few days).

@Danielhiversen
Copy link
Member

I do not think this solve your problem.
The data dictionary should never be empty: https://github.com/mjg59/python-broadlink/blob/master/broadlink/__init__.py#L468

@Paxy
Copy link
Contributor Author

Paxy commented Jan 24, 2018

Did not have to wait for long. Here is what I have found:

2018-01-23 23:44:33 WARNING (Thread-20) [homeassistant.components.sensor.broadlink] Validating data {'humidity': 55.0, 'ligh$
2018-01-23 23:44:33 ERROR (MainThread) [homeassistant.helpers.entity] Update for sensor.broadlink_sensor_temperature fails
Traceback (most recent call last):
  File "/usr/local/lib/python3.5/dist-packages/homeassistant/components/sensor/broadlink.py", line 133, in _update
    self.data = self._schema(data)
  File "/usr/local/lib/python3.5/dist-packages/voluptuous/schema_builder.py", line 221, in __call__
    return self._compiled([], data)
  File "/usr/local/lib/python3.5/dist-packages/voluptuous/schema_builder.py", line 538, in validate_dict
    return base_validate(path, iteritems(data), out)
  File "/usr/local/lib/python3.5/dist-packages/voluptuous/schema_builder.py", line 370, in validate_mapping
    raise er.MultipleInvalid(errors)
voluptuous.error.MultipleInvalid: not a valid value for dictionary value @ data['light']

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/lib/python3.5/dist-packages/homeassistant/helpers/entity.py", line 199, in async_update_ha_state
    yield from self.async_device_update()
  File "/usr/local/lib/python3.5/dist-packages/homeassistant/helpers/entity.py", line 306, in async_device_update
    yield from self.hass.async_add_job(self.update)
  File "/usr/lib/python3.5/asyncio/futures.py", line 361, in __iter__
    yield self  # This tells Task to wait for completion.
  File "/usr/lib/python3.5/asyncio/tasks.py", line 296, in _wakeup
    future.result()
  File "/usr/lib/python3.5/asyncio/futures.py", line 274, in result
    raise self._exception
  File "/usr/lib/python3.5/concurrent/futures/thread.py", line 55, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/usr/local/lib/python3.5/dist-packages/homeassistant/components/sensor/broadlink.py", line 96, in update
    self._broadlink_data.update()
  File "/usr/local/lib/python3.5/dist-packages/homeassistant/util/__init__.py", line 306, in wrapper
    result = method(*args, **kwargs)
  File "/usr/local/lib/python3.5/dist-packages/homeassistant/components/sensor/broadlink.py", line 142, in _update
    _LOGGER.error(error)
UnboundLocalError: local variable 'error' referenced before assignment

The first Warning msg is my debug message to show data from the sensor. There you can see that sensor itself return some bad data.
The second should show validation complete when it is right.
Error msg should be ignored as I incorrectly setup error log but anyway point is that sensor is not recovered once validation fails.
I am not sure why, but I will try to avoid validation process to check my theory.

Data that made exception:
Validating data {'humidity': 96.6, 'light': 236, 'noise': 35, 'temperature': 186.7, 'air_quality': 204}
Strange thing is that in the same time, my scrypt just get temperature 249.0 and no other values.

@Danielhiversen
Copy link
Member

  File "/usr/local/lib/python3.5/dist-packages/homeassistant/components/sensor/broadlink.py", line 142, in _update
    _LOGGER.error(error)
UnboundLocalError: local variable 'error' referenced before assignment

I think you have introduced this error.

But it seems that we do not except the correct error. Try to change this line https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/sensor/broadlink.py#L132 to;
except vol.Invalid, vol.MultipleInvalid:

@Paxy
Copy link
Contributor Author

Paxy commented Jan 24, 2018

Ok. Yea, it looks like an unhandled exception was the issue.
Corrected catch block as suggested.

@Paxy
Copy link
Contributor Author

Paxy commented Jan 24, 2018

Another issue appears. Debug messages showing the attempt to read temperature but failing at auth(). (do not succeed in any retry).
At the same time, my script kept reading temp without the issue. Broadlink eControl showed device offline on first read attempt but succeed on others.
Probable cause is a communication issue, but also a problem caused by connection brake.
I will try to put connection establish initiator in auth exception block - before https://github.com/home-assistant/home-assistant/blob/a0a001db7171fee39926d84ea39e9943a0e03947/homeassistant/components/sensor/broadlink.py#L143

@Paxy
Copy link
Contributor Author

Paxy commented Feb 1, 2018

Looks like the problem does not appear anymore in 0.62.0 (0.62.1). At least for now.

@Paxy
Copy link
Contributor Author

Paxy commented Mar 28, 2018

We need to reopen this issue.
Since last update to HA 0.65.3 every morning I had to restart HA to get Broadlink sensors working.

The solution is to switch to the non-persistent connection for sensors. That way, every time sensor needs to be read, connection and auth to Broadlink RM is established.

Solution that work for me is at the link

@matisaul
Copy link

matisaul commented Apr 7, 2018

@Paxy I'm sorry, Where I need to put this file? /custom_components/sensor/ ?

@Paxy
Copy link
Contributor Author

Paxy commented Apr 7, 2018

I have replaced original one /components/sensors/broadlink.py with modified version. Since than, no single freeze ocured.

@Paxy
Copy link
Contributor Author

Paxy commented Apr 7, 2018

@Danielhiversen can you make such modifications as part of future releases?

@matisaul
Copy link

matisaul commented Apr 7, 2018

Thanks, I will try that.

@matisaul
Copy link

matisaul commented Apr 7, 2018

This file fix the issue, when I have 2 RM2 sensors? I can't get working both at the same time...

@Paxy
Copy link
Contributor Author

Paxy commented Apr 7, 2018

This issue does not have anything to do with it.
I have too RM2s (actually RM2 and RM3) and they works at the same time with conf:

  - platform: broadlink
    update_interval: 60
    host: fristIP
    mac: firstMAC
    monitored_conditions:
     - temperature      
  - platform: broadlink
    name: temp_sprat
    host: secondIP
    mac: secondMAC
    update_interval: 60
    monitored_conditions:
     - temperature

@Danielhiversen
Copy link
Member

@Paxy : You are welcome to open a pull request we your changes

@Paxy
Copy link
Contributor Author

Paxy commented Apr 8, 2018

@Danielhiversen, I have added pull request. Just saw that I have added it to DEV branch with broadlink lib 0.8.0. Should I add to Master branch too?

@home-assistant home-assistant locked and limited conversation to collaborators Jul 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants