← index #332PR #850
Likely Duplicate · high · value 3.161
QUERY · ISSUE

PINGRESP ignored in umqtt.simple.py

openby nfj25opened 2019-02-27updated 2026-02-06

Hello,

In wait_msg, the PINGRESP message is ignored:

if res == b"\xd0":  # PINGRESP
            sz = self.sock.read(1)[0]
            assert sz == 0
            return None

Shouldn't this return some information so that the calling code can determine if the server is indeed responding to pings?

11 comments
michaeka79 · 2022-11-05

I'm going to have to upvote this... I feel like this should be the case as well. umqtt.robust doesn't fix this either. We need to know if the connection is still active or not.

I found some code on https://github.com/AntonisKekempanos/SonoffMicropythonMQTT.git

Where they return b'PINGRESP' this seems like an appropriate return code for the ping.

andrewleech · 2022-11-05

I don't know much about mqtt however from a quick search it appears that clients (like this module) are supposed to send a PINGREQ to the broker regularly, to which the broker responds with PINGRESP.

Changing wait_msg to return something different when this comes in would affect existing users of the library who haven't had to expect this message and know how to deal with it.

Perhaps instead of returning something, The library could have a new attribute in the mqtt client class where a timestamp is updated every time one of these comes in, that way an application that cares about it can query that attribute to see how long it's been since the last server communication?

michaeka79 · 2022-11-05

+1 I love that idea! That would help with about half the code I'd be trying to write to check the time intervals between pings. So yes, love it!

michaeka79 · 2022-11-06

@andrewleech were you thinking of https://github.com/fizista/micropython-umqtt.simple2.git when you suggested having it be an attribute of the client? Because it seems like that's exactly what's been implemented.

andrewleech · 2022-11-06

I've never seen that library myself, to be honest I haven't used mqtt at all.

vitaliishchudlo · 2023-12-04

@michaeka79 Maybe you have any ideas to fix it?
@nfj25 did you solve your problem?

nfj25 · 2023-12-14

Not using it anymore...

trescenzi · 2024-04-21

Spent some time trying to figure out why pings weren't working before finding this issue. Is there a rationale behind this? The docs even seem to imply that this should be handling the ping

Ping server (response is automatically handled by wait_msg())

That implies to me not that the ping is ignored, but that wait_msg will tell you a ping has been responded to.

I've forked this and swapped the None return for a b"PINGRESP", as someone suggested above, and it seems to work fine. However I'm not using any subscriptions on my end so maybe this messes that up? If it does maybe we could instead set a flag on the client that indicates the ping has been responded to and then clear it upon the next ping?

I'm happy to make either change, they are very straightforward, not sure though if this is actually a desired change.

andrewleech · 2024-04-23

My understanding is that none of the current owners / maintainers of micropython-lib really use or know much about mqtt - hence this library hasn't seen any real work or expansion.

As such I really don't know if there's anyone who could really answer your question about the correct way to handle this, or what should be done to fix it.

trescenzi · 2024-04-23

I opened a pr with the solution I've been using locally. It's minimally invasive and non-breaking which is why I went with this approach although I'm not necessarily the most python oriented dev so it's possible this is an unusual approach in python. But I wanted to propose it since this issue has been open for some time and the current ping code doesn't seem as useful as it could be.

europrimus · 2026-02-06

Yes, there is some thing to change her. Maybe the doc, or the code. but I passed half a day searching to test if is connected
Finally, my function

    def is_connected(self):
        if not self._client:
            return False
        try :
            self._client.ping()
        except (AttributeError, OSError):
            return False
        return True

but not sure what append if server is down.

CANDIDATE · PULL REQUEST

umqtt.simple: add ping_response_received for ping handling

openby trescenziopened 2024-04-23updated 2024-04-23

Currently when using ping the response is swallowed which makes it not particularly useful. This PR adds a flag ping_response_received to the client which is set to True if a response has been received in wait_msg. I've been using this approach in a personal project without any issues and it's not particularly invasive so I wanted to propose it as a solution to #332.

Another possible approach is to actually respond in wait_msg with an actual ping response however I'm not certain how that would interact with subscriptions and those currently using wait_msg so this approach was taken in order to not be breaking while still being useful.

Keyboard

j / / n
next pair
k / / p
previous pair
1 / / h
show query pane
2 / / l
show candidate pane
c
copy suggested comment
r
toggle reasoning
g i
go to index
?
show this help
esc
close overlays

press ? or esc to close

copied