settimeout for mqtt socket #890
Conversation
| def connect(self, clean_session=True): | ||
| self.sock = socket.socket() | ||
| self._sock = self.sock = socket.socket() |
There was a problem hiding this comment.
I think it would be better to instead add a timeout=None argument to this connect() method. Then call self.sock.settimeout(timeout) after creating the socket.
That would mean that the connect call below, and wrap_socket, would also apply the timeout.
There was a problem hiding this comment.
Hi @dpgeorge ,
Agree with you. That will be better.
There was a problem hiding this comment.
@prabhu-yu Will you update this PR with the above suggestion? If not I can do it.
There was a problem hiding this comment.
Hi @dpgeorge,
Thanks for reminding me.
Here is the change and test.
Please let me know your comments.
Test:
I set the some timeout=3
mqtt_client.connect(clean_session=True, timeout=3)
and begin publishing with below call.
mqtt_client.publish(b'dev/1/ota/ota_server_tx', msg, qos=1)
(I used MQTTX to monitor msgs)
After sometime, to cause n/w failure, I physically removed n/w cable of my PC.
I observed that OSError exception thrown exactly after the 3 seconds.
However, pl note: qos=0 in publish() does not have any effect due to this change. This is expected, I think. ie, if one wants exception upon n/w failure, he or should use qos=1.
I repeated tests with qos=1 for different timeout.
⟶ micropython test.py
client_id=555555555,
broker=<somebroker.emqxsl.com>
connection ok
publishing 1 msg per second
publishing 1 msg per second
publishing 1 msg per second
publishing 1 msg per second
publishing 1 msg per second
got OSError exception during publish!
See https://github.com/tve/mqboard/tree/master/mqtt_async |
Thank you for the link. |
Actually, this is a better version: https://github.com/peterhinch/micropython-mqtt Eventually we would like to add links from this repository to external/third-party libraries like the above, to make them easier to install, and have better visibility. |
This will be a lot helpful to all those who use async. Async really shines as it gives better control to developer over threads/processes in terms of scheduling. This in turn solves many locking issues that are common in threads. in anycase, async mqtt blends nicely into all async applications. |
| self.sock = socket.socket() | ||
| if timeout is not None: | ||
| self.sock.settimeout(timeout) |
There was a problem hiding this comment.
If you pass None to settimeout then it sets it to blocking mode, which is also the default after creating a fresh socket.
So, the if timeout is not None line is not needed. It can just set the timeout unconditionally and still have the same behaviour as before. Doing it that way makes the code smaller and simpler.
There was a problem hiding this comment.
Hi @dpgeorge ,
Agree with you. I submitted the change.
(I was trying to be little cautious by avoiding to execute settimeout when default value arrives! I agree, this is not a clean solution!)
There was a problem hiding this comment.
@dpgeorge
May you please suggest the next step?
Should I create a PR for this?
There was a problem hiding this comment.
This change looks good now, thank you.
But I see on your commits that the author is Your Name <you@example.com>. Is that intentional? Usually we require a proper name and email, for attribution of the work.
If you want to remain anonymous, I can change the author to my name.
There was a problem hiding this comment.
Hi @dpgeorge ,
Thank you for notifying me 🙏.
I have updated my Name/E-mail id and pushed it again.
Please let me know if I need to do something further.
prabhu-yu
force-pushed
the
mqtt_settimeout
branch
from
a66b4ce
to
44c55e0
Compare
Problem statement:
If there are any network issues, mqtt will block on socket non-deterministically.
In such cases, only way to come out of the blocking is to reboot using watch dog timers.
This is costly solution.
Solution:
Alternatively, developer can set the max timeout for the socket.
Upon any issue, mqtt lib will throw exception. Developer can catch it, take
right actions like, restarting the task without rebooting the whole device.
This brings determinism and gives the control to developer to choose right time for her/his use case. This fix works for async applications too.
(I plan to make whole umqtt async compatible.)