Adding MQTTTopicDiscoveryService publish method

Development
, ,
1

Hello all,

For a binding I am currently developing, I need to do an “active” scanning to discover devices in the network. The device implements a custom MQTT protocol, and my discovery service extends AbstractMQTTDiscovery.

Internally, the AbstractMQTTDiscovery provides me with an implementation of MQTTTopicDiscoveryService interface implemented in MqttBrokerHandlerFactory.

The methods currently exposed are subscribe and unsubscribe in order to receive MQTT messages and implement custom logic for thing discovery, but in order to implement discovery in my case, I need to also publish a trigger message. I intend to submit a PR for this change, and I wanted to check with you guys if this implementation makes sense:

  1. Extend MQTTTopicDiscoveryService with the following method:
    void publish(String topic, byte[] payload);

  2. Implement the publish method in MqttBrokerHandlerFactory and inside iterate the existing MQTT handlers and publish asynchronously the message.

Any thoughts are welcome.

Cheers,
Sebastian

2

Mqtt is not the right protocol for polling honestly.

The current architecture assumes that a user has configured multiple mqtt server connections. A local mosquitto maybe and a remote one for example from adafruit. Remote ones have a certain publish quota though or you pay per publish.

Your plan is to add a publish method for polling based discovery that would publish to all those mqtt connections.

The idiomatic way would be to register your mqtt clients to a certain topic and unregister them again via a last will.

Cheers David

3

Hi, Thanks for your reply!

I understand what you are saying, but what I am suggesting is merely adding the api, not actually publishing anything for existing bindings. My binding, and I assume I cannot be the only one who needs to publish something to trigger dialogue with the mqtt devices - which are sitting idly by.

I have patched the MqttBrokerHandlerFactory and I wanted to understand whether or not this patch is of broader interest or it conflicts current design choices. if it is not the case, I can share it with a PR.

The fact that a binding has the ability to discover devices by sending a broadcast does not mean that all MQTT bindings will have to send something, and at the same time, limitations that can occur with certain MQTT brokers I don’t think should limit what the API can support, everyone is free to use what ever broker they can or want. For example, I myself use a self hosted one.

Cheers,
Sebastian

4

For sake of consistency, the proposed API would actually be inline with upnp, mdns, please go ahead with a PR. If the javadoc informs the api consumer about possible implications, it should be alright.