PicoMQTT::Client::subscribe immediately unsubscribes

[salberternst]

Hi!

First of all thanks for the great library. I'm currently working on implementing PicoMqtt into esphome (click for source) (doesn't provide working TLS for esp8266) and I stumbled across a problem.

I'm using PicoMQTT::Client subscribe but the client immediately unsubscribes from the topic and therefore no data is ever received. From my understanding calling subscribe calls

https://github.com/mlesniew/PicoMQTT/blob/49ad5cff55aa7d4937cea6151e97d572a0f6b702/src/PicoMQTT/client.cpp#L218-L223

Which subscribes using Packet::SUBSCRIBE and calls SubscribedMessageListener::subscribe:

https://github.com/mlesniew/PicoMQTT/blob/49ad5cff55aa7d4937cea6151e97d572a0f6b702/src/PicoMQTT/subscriber.cpp#L95-L100

Which calls (in line 97) unsubscribe from the PicoMQTT:Client class:

https://github.com/mlesniew/PicoMQTT/blob/49ad5cff55aa7d4937cea6151e97d572a0f6b702/src/PicoMQTT/client.cpp#L224-L228

Which calls BasicClient::unsubscribe (in line 226) which finally sends Packet::UNSUBSCRIBE just after subscribing.

I'm currently build a workaround which and removed the unscribe in the SubscribedMessageListener and instead checks if there is already a subscription for that topic. (I'm not sure if reusing the id is even a thing/allowed)

Subscriber::SubscriptionId SubscribedMessageListener::subscribe(const String & topic_filter, MessageCallback callback) {
    TRACE_FUNCTION
    auto subscription = subscriptions.find(topic_filter);
    if (subscription != subscriptions.end()) {
        return subscription->first.id;
    }
    auto pair = subscriptions.emplace(std::make_pair(Subscription(topic_filter), callback));
    return pair.first->first.id;
}

Am I doing something wrong or is this a bug?

Beside that, the library is working fine for me and allows me to use TLS. So thanks again!

Best

[mlesniew]

Hey, that's an interesting corner case! I think you've identified correctly what causes the problem.

As you have noticed, the library only allows setting one callback for each topic pattern.

When subscribe is called with the same topic pattern for a second time, the intended behavior was to remove the old subscription callback and replace it with the new one.

Your suggested solution does the opposite. It keeps the old callback assigned, ignoring the new one.

While this approach is also ok, I think it's a bit less intuitive.

What I would suggest as a fix is reversing the order of calls in Client::subscribe(const String & topic_filter, MessageCallback callback), so that SubscribedMessageListener::subscribe is called first. It's a simple fix but it should work.

In theory, in this scenario we could skip sending the unsubscribe and subscribe messages to the broker altogether. We're still interested in receiving messages matching the topic pattern, but internally we want to fire a different callback to handle it. The broker doesn't care about that.

In practice however the resubscribe is harmless and it's impact on performance should be marginal considering that subscriptions are typically set up only during initialization.

I'm really happy to hear you're integrating the library with esphome. It's such a great project. I really wish a PicoMQTT integration will be merged into the esphome mainline someday...

[salberternst]

Thanks for the quick reply! I will try out your suggestion and get back to you.

[hjahend]

Would welcome a PicoMQTT solution is ESPHome (YAML), usable on ESP8266 and ESP32. Keep up the good work and thanks in advance for the effort.

[mlesniew]

Hey @salberternst, how is it going? Do you need help with the fix?