Danielhiversen / PyXiaomiGateway

PyXiaomiGateway
MIT License
152 stars 55 forks source link

Fix race condition in a _send_cmd() #104

Closed PaulAnnekov closed 5 years ago

PaulAnnekov commented 5 years ago

Quick fix for #45 by creating separate socket for each send.

This line should be removed after merge to make build successful: https://github.com/Danielhiversen/PyXiaomiGateway/blob/b7407d22118529d17bbfd99050f755edfc085c74/tests/test_e2e.py#L96

Barrysv commented 5 years ago

Been testing pyxiaomigateway 0.11.0 with my 0.79.3 Homeassistant (ran hass with --skip-pip to be sure that the latest Xiaomi library is being used.) the devices are working fine but I did notice the 'cannot connect to gateway' and 'no data in response from hub none' messages today. Am I testing it correctly or is something missing? Cheers Confirm the installed library is as follows: (homeassistant) homeassistant@hass:~ $ pip show pyxiaomigateway Name: PyXiaomiGateway Version: 0.11.0

PaulAnnekov commented 5 years ago

@Barrysv it will be better to see your Home Assistant logs with PyXiaomiGateway 0.10.0 and 0.11.0.

Barrysv commented 5 years ago

@PaulAnnekov - saw the message yesterday 3 times in the logs - Haven't seen it come up again today but in the previous version (<0.11.0) I only got the error messages sporadically. I have 8 xiaomi switches, 2 motion sensors, 4 temp/humidity sensors, 2 lights, 3 IR remotes working in my network (separate vlan). Do I need to pull the dev 0.80.0 homeassistant to make use of the updated features in 0.11.0? Cheers

PaulAnnekov commented 5 years ago

Do I need to pull the dev 0.80.0 homeassistant to make use of the updated features in 0.11.0

No.

saw the message yesterday 3 times in the logs

It's hard to debug this issue w/o your logs.

Barrysv commented 5 years ago

2018-10-08 21:01:55 WARNING (MainThread) [homeassistant.bootstrap] Skipping pip installation of required modules. This may cause issues 2018-10-08 21:01:55 WARNING (MainThread) [homeassistant.components.http] legacy_api_password support has been enabled. If you don't require it, remove the 'api_password' from your http config. 2018-10-08 21:02:01 WARNING (MainThread) [homeassistant.loader] You are using a custom component for sensor.technicolor which has not been tested by Home Assistant. This component might cause stability problems , be sure to disable it if you do experience issues with Home Assistant. 2018-10-09 07:20:29 ERROR (Thread-2) [xiaomi_gateway] Cannot connect to Gateway 2018-10-09 07:20:29 ERROR (Thread-2) [xiaomi_gateway] No data in response from hub None

Hi Paul, this is a pretty short log - I restarted last night after reconfiguring iOS notifications. The technicolor sensor is a custom component that reads from my technicolor modem. It has been running well for the past 2 months.

On Tue, Oct 9, 2018 at 1:57 AM Paul Annekov notifications@github.com wrote:

Do I need to pull the dev 0.80.0 homeassistant to make use of the updated features in 0.11.0

No.

saw the message yesterday 3 times in the logs

It's hard to debug this issue w/o your logs.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Danielhiversen/PyXiaomiGateway/pull/104#issuecomment-427925224, or mute the thread https://github.com/notifications/unsubscribe-auth/AiP03dyaKHZ5n9b94H_DEmxUQzRhklCUks5ui5IUgaJpZM4W3ibu .

PaulAnnekov commented 5 years ago

@Barrysv add config for additional logs:

logger:
  logs:
    xiaomi_gateway: debug
    homeassistant.components.xiaomi_aqara: debug
Barrysv commented 5 years ago

@PaulAnnekov I've uploaded an extract from the log. It starts getting interesting around 15:41:08 when a motion sensor triggers a few lights and we see the connection error occur. Thanks

home_asistant_xiaomi_debug_log.txt

PaulAnnekov commented 5 years ago

@Barrysv ok, I see. HA sends 2 requests in parallel, but receives response only for one. Try to collect these erroneous cases and find a pattern:

Barrysv commented 5 years ago

@PaulAnnekov- so far it certainly seems to be the case that when two simultaneous requests are triggered, sometimes only one acknowledgement comes back from the gateway. In this new log you see an automation trigger the state of a HA group and again the gateway (internal light) is one of the entities. I have other scripts that trigger multiple xiaomi switches (both on and off) but haven't yet seen them misbehave in this latest version. I have tried toggling groups in HA that affect multiple xiaomi switch entities and the internal gateway light but haven't yet seen the error to show on demand. Will keep monitoring home_asistant_xiaomi_debug_log_2.txt

Barrysv commented 5 years ago

@PaulAnnekov It seems that most of the time the Xiaomi gateway can handle two simultaneous requests - but on occasion only one ack comes back. See filtered log attached xiaomi_cmd_summary.txt

PaulAnnekov commented 5 years ago

Run:

git clone https://github.com/Danielhiversen/PyXiaomiGateway.git
cd PyXiaomiGateway
pip install -e .

Download file test.txt into PyXiaomiGateway folder, rename to test.py. Run:

python3 test.py 34ce0088c937 158d00010decac [key] on

It will switch on/off gateway and plug 20 times simultaneously. You can execute it multiple times. Check if errors appear during execution.

Barrysv commented 5 years ago

Hi Paul, I just tested it and can run it several times without any errors. But on occasion I do notice an invalid key error (apologies for the screen shot attached instead of a log file). I ran this test.py in my homeassistant virtual environment so I didn't need to clone the gateway git and home assistant was running in the background also. I have noticed now in HA 0.80 since there is more frequent polling of the switch power status I have noticed that in the HA logs some switch requests don't get a reply e8945f5b-f2a8-4716-91e2-c1e2a1833245

PaulAnnekov commented 5 years ago

But on occasion I do notice an invalid key error

I think that's because you have not stopped HA.

Can you make HA 0.80.0 to use PyXiaomiGateway 0.10.0? Check whether you will see some errors.

Barrysv commented 5 years ago

@PaulAnnekov pip installed pyxiaomigateway 0.10.0 and tested your script again a few times - log attached rpi_20181016_112320.txt

PaulAnnekov commented 5 years ago

@Barrysv now can you run HA 0.80.0 + pyxiaomigateway 0.10.0 for several days?