sbtinstruments / aiomqtt

The idiomatic asyncio MQTT client, wrapped around paho-mqtt
https://sbtinstruments.github.io/aiomqtt
BSD 3-Clause "New" or "Revised" License
392 stars 71 forks source link

Topic.matches should be used when doing == #280

Closed Adair-GA closed 4 months ago

Adair-GA commented 4 months ago

Hi! This may be a kind of a neat pick, but shouldn't ==, when used with two topics call topic.matches()? I know it's a really "meaningless" thing but I just spent like 15 minutes trying to work out why my code was not working and it was because of that lol.

So, if there's a reason for it not being the case, no worries, if not, I'd be happy to make the PR. Thanks in advance!

empicano commented 4 months ago

Hi there! 😊 It's not entirely clear to me from your description why your code was not working, or what changes you are suggesting. Could you post a minimal working example?

Adair-GA commented 4 months ago

This is the code I was using:

if message.topic == "/app/device/property/DAEBZ5KEB033484":
      await sio.emit('delta', message.payload)
elif message.topic == "/app/device/property/R601ZEB5HF260973":
      await sio.emit('river', message.payload)

and it wasn't working, the condition was never true, so I changed the == to message.topic.matches(), like this

if message.topic.matches("/app/device/property/DAEBZ5KEB033484"):
      await sio.emit('delta', message.payload)
elif message.topic.matches("/app/device/property/R601ZEB5HF260973"):
      await sio.emit('river', message.payload)

My suggestion is for == to do the same as .matches(), if no other case requieres the == comparation to be what it is now

empicano commented 4 months ago

Thanks for the example! 👍

Topic.matches() can match filters as well, so something like Topic("a/b/c").matches("a/+/c") returns true, as shown in the Filtering messages section in our docs. If you want, you can access the topic string directly via Topic.value.

Replacing Topic.matches() entirely by == (by implementing __eq__ for Topic) would lead to something unexpected, namely that Topic("a/b/c") == "a/+/c" would return true and "a/+/c" == Topic("a/b/c") would return false.

Does that make sense? 🙂

Adair-GA commented 4 months ago

Oh, okay, I wasn't aware that I could access the value field like that. Kind of guilty of not reading the docs and relying on Intellisense 😅. Great work with this library, it's really useful, and thanks for the pleasant response!