marvinroger / async-mqtt-client

📶 An Arduino for ESP8266 asynchronous MQTT client implementation
MIT License
834 stars 266 forks source link

Thanks (not a bug) #197

Closed einglis closed 3 years ago

einglis commented 4 years ago

I've used your library in my projects for some years now. Recently, I started looking into some strange behaviours, which turned out to be with the Will topic pointer issue referenced elsewhere. So I think it's time to move on.

But I wanted to say thanks all the same. A certain newer implementation takes a very condescending tone towards your library, but entirely fails to acknowledge the years of service it has provided to many. It isn't bug-free but neither is it anywhere as awful as implied.

Cheers.

TSJim commented 4 years ago

Hey Einglis...

What is the new MQTT library are you moving to?

The reason for my question is that I can't seem to find an MQTT library that provides:

o Support for ESP32 (not ESP8266) o QOS 2 for pub and sub o Arduino IDE support (not Espressif IDE only)

Thanks!

qcasey commented 4 years ago

They're referring to PangolinMQTT.

For what it's worth, I've used async-mqtt-client on my ESP-32 (albeit with QOS0 and PIO) for quite some time now without a ton of hassle. Still haven't found a better alternative. This library has gotten me pretty far and I'd also like to share my thanks @marvinroger

bertmelis commented 4 years ago

I still use this lib, but only QoS0. I did add the copy flag though.

I abandoned Pangolin. I just can't convince myself using a lib from someone that confused a compiler warning with incompatible. Furthermore it limits the message size to fit in memory. OTA over MQTT is a must have for me.

marvinroger commented 4 years ago

Hi everyone,

I'd like to thank you for these kind words.

I never heard of this library, because I'm not following closely the ESP8266/ESP32 ecosystem anymore, but the author does take a condescending tone towards the lib. I've seen some of his replies on GitHub, and it seems that he's also condescending towards people, too, so I don't feel personally attacked.

AsyncMqttClient definitely have flaws, big flaws, but at the time, MQTT was not as widely used as today and the ESP8266/32 ecosystem was not so developed and stable. Also, and maybe most importantly: I had like a 1 year of hobby-experience with C/C++ and embedded programming. So yeah, it's easy to take a pioneer library (after PubSubClient), identify its flaws years after, and criticize / attack that way without any acknowledgements. People should remember that every OSS project is basically a free donation.

Anyway, I am proud of what we've done with this library. I don't have time to maintain this project anymore, and having alternatives is always a good thing. So, good luck with your project @philbowles. Just a small, obvious tip, but if you want your projects to be genuinely successful, work with your community, not against it.

luebbe commented 4 years ago

The problem is, that nobody is maintaining this project anymore. There a hundred forks with different fixes floating around, but this is the original, which is included in so many other OSS projects.

proddy commented 4 years ago

I'm with @bertmelis here. I gave up with Pangolin too mainly due to the author's arrogance and behavior towards community feedback and contributions. I also agree that this library could do with a cleanup and picking the best bits from all the various forks.

marvinroger commented 4 years ago

I would be happy to give collaborator rights to someone who'd like to maintain the project!

luebbe commented 4 years ago

Pangolin is dead for me unless the author changes his attitude. @marvinroger, I could become "contributor" and accept pull requests here, but I don't know anything about this codebase so I would have to ask other people for review. The same problem exists with https://github.com/homieiot/homie-esp8266. I know at least a bit about that codebase, so I feel more comfortable there.

Pablo2048 commented 4 years ago

@luebbe I'm glad to help with the reviews at least as my time allow me to do so...

luebbe commented 4 years ago

@Pablo2048 @bertmelis @ anyone else: Let's do it together?

kleini commented 4 years ago

I can try to help with review. At least I understand most parts of the homie-esp8266 code and I am able to dive deeper into code, if it is necessary.

sparkplug23 commented 4 years ago

I just added this project today and the first messages about it is this haha. Currently using pubsubclient that I have heavily modified. I'll take a look into this though and see if it can replace it. If it's worth swapping for me I'll happily make changes and place fixes for the community.

Edit: my curiosity was peaked at the hatred for pangolin shown here so went and checked out the repo. Simply wow. The responses and the code itself, wow.

bertmelis commented 4 years ago

I will contribute to the best of my ability. In the end, there are big flaws but not many flaws.

proddy commented 4 years ago

me too. All my projects use this library and rely on it.

sparkplug23 commented 4 years ago

@bertmelis have these big issues been listed anywhere? Curious to see what needs addressing.

I recently switching to using asyncwebserver and basically rewrote the thing to remove have heavily it relied on strings. I was happy to see async Mqtt does not.

bertmelis commented 4 years ago

Well, there is a list here: https://github.com/philbowles/PangolinMQTT/blob/master/docs/bugs.md

But long story short: 1) COPY flag has to be enabled when writing to AsyncTCP to avoid dangling pointers. 2) QoS 1 and 2 are broken

Point 2 is a hard one and I'd even say too much on some points. For some QoS specs you'd have to have persistence across reboots.

Now looking at alternatives:

So I'd first fix obvious easy to fix bugs, get basic stuff working (QoS0 sending, QoS0/1 receiving) and then move onwards.

Pablo2048 commented 4 years ago

I suggest to merge pending PR's 156, 157, 159, 180 on first step, then add the copy flag - I did this manually and for checking purposes sent it here https://github.com/philbowles/PangolinMQTT/issues/20#issuecomment-674818170 so everybody can take a look and check it...

sparkplug23 commented 4 years ago

I am confused, are we talking about fixing pangolin or async Mqtt ?

Pablo2048 commented 4 years ago

Async MQTT. I put my (patched version) of Async MQTT as an attachment to Pangolin MQTT thread...

marvinroger commented 4 years ago

@luebbe @bertmelis You're the ones I know the must through your contributions to Homie / AsyncMqttClient. I just gave you Collaborator rights, you should have received an e-mail.

@kleini @proddy @Pablo2048 thanks for your help on the reviews, if you have time to do so. Also, if you'd like to have the actual rights, don't hesitate to drop me an e-mail - I am not against it, it's just that I am not comfortable giving permissions to 5 people at once. 😉

kleini commented 4 years ago

I can become contributor later. My patched version is here. Now it is summer time for me and I go flying instead of working on home automation, but I will come back to this, if it becomes outside not comfortable any more.

sparkplug23 commented 4 years ago

Likewise happy to help in a few months. Currently focusing on my changes to asyncwebserver so need to get that running well first then will turn my attention back to mqtt.

proddy commented 4 years ago

we should also bring in @obrain17 who's forked @bertmelis's code and added QOS1+2: https://github.com/obrain17/asyncmqtt

bertmelis commented 4 years ago

Euh, do I have code that is different from this repo?

proddy commented 4 years ago

Euh, do I have code that is different from this repo?

Not sure, haven't checked. But the repo says 13 commits ahead:

Screen Shot 2020-09-02 at 18 27 19
HamzaHajeir commented 4 years ago

we should also bring in @obrain17 who's forked @bertmelis's code and added QOS1+2: https://github.com/obrain17/asyncmqtt

To mention, This lib was originally started by philbowles (author of PangolinMQTT), then, he rewrite it again and call it PangolinMQTT.

I think this point worth's mentioning.

bertmelis commented 3 years ago

Kudos to @marvinroger