search

dauden1184 / RaspiNukiBridge

Simple Nuki Bridge implementation using asyncio
GNU General Public License v3.0
48 stars 19 forks source link

Feature/improvestability #4

Closed peteh closed 2 years ago

peteh commented 2 years ago

Hi,

I tried to improve connection stability. This is still work in progress. Main changes:

@dauden1184 I have a question

  1. there is a forced disconnect after 30s (_timeout), what is it used for? I think it's meant for cleaning up but it might still trigger in the middle of another execution. As far as I can see the actions all call disconnect() through _send_data() at the end anyway. So it seems to be redundant and might kill an action if it takes ~30s.
  2. All actions are marked as async. However, bluez seems not to be threadsafe. Might this become an issue, if for example updatestate and lockaction is called at the same time? It seems like if I call another action it runs into trouble.
dauden1184 commented 2 years ago
  • there is a forced disconnect after 30s (_timeout), what is it used for? I think it's meant for cleaning up but it might still trigger in the middle of another execution. As far as I can see the actions all call disconnect() through _send_data() at the end anyway. So it seems to be redundant and might kill an action if it takes ~30s.

_send_data() call disconnect only if every retry of the connection failed.

Normally, the connection is closed when the software received from he nuki that the command is completed: here, or here for example.

If, for any reason, the software doesn't receive anything from the smartlock, the connection will stay up indefinitely, draining the smartlock battery: that's the reason for the 30 seconds timeout that close the connection (30 seconds should be enough to complete any Nuki actions).

  • All actions are marked as async. However, bluez seems not to be threadsafe. Might this become an issue, if for example updatestate and lockaction is called at the same time? It seems like if I call another action it runs into trouble.

Async and thread are 2 different things (async is not multithread). However, this software doesn't manage very well 2 commands at the same time: usually the first one is executed and the second one fail (as with the real bridge, if I'm not wrong), but it is not a clean process so I'm not sure it works without issues every time. I will work on this (the Bridge API should respond with a error 503, this is not happening now).

Did you try the experimental flag? It seems to perform better for me.

peteh commented 2 years ago

I'll try with experimental flag. However it seems that increasing the timeout in the connect() of the client actually also improves the situation a bit.