[BUG] Incoming commands get lost when using multiple subscribers on existing client #509
Description
Describe the bug
I want to define multiple subscribers (Number, Select, whatever). I'm passing a preexisting client as outlined in the Readme, "using an existing MQTT client" section (to save on Thread count).
Outgoing messages work so far, and entities show up in Home Assistant. However, only one of the clientside subscriber objects (the last one) will receive commands sent from Home Assistant.
The reason is that the client's on_connect method is replaced upon construction of each Subscriber. Call chain Subscriber.__init__ -> Discoverable.__init__ -> Discoverable._setup_client.
Since client is shared, the previous on_connect handlers are forgotten. The effect is that Subscriber.__init__.on_client_connected is only called for the last-created Subscriber object. Thus, only the last subscriber is actually subscribed (to listen for command messages), and all previous Subscribers won't react to commands.
To Reproduce
Minimal Example to reproduce the behavior:
import logging
from paho.mqtt.client import Client
from ha_mqtt_discoverable import Settings
from ha_mqtt_discoverable.sensors import Number, NumberInfo, DeviceInfo
def demonstrate_the_bug():
client = Client()
client.username = "username_here"
client.password = "password_here"
client.connect("ip_address_here")
client.loop_start()
def make_callback(name):
def callback(client, userdata, message):
print(f"Received message for {name}: {message.payload.decode()}")
return callback
device = DeviceInfo(name="mqttbug", identifiers=["mqttbug"])
number1 = Number(
Settings(
mqtt=Settings.MQTT(client=client),
entity=NumberInfo(name="number1", unique_id="number1", device=device)
),
make_callback("number1")
)
number2 = Number(
Settings(
mqtt=Settings.MQTT(client=client),
entity=NumberInfo(name="number2", unique_id="number2", device=device)
),
make_callback("number2")
)
# publish to make them appear
number1.set_value(1)
number2.set_value(2)
input("Press Enter to exit...")
client.loop_stop()
logging.basicConfig(level="INFO")
demonstrate_the_bug()
# Move the sliders in Home assistant now. Only the callback for number2 is called,
# because number1 was not subscribed.
Expected behavior
Discoverable._setup_client must not discard old on_connect handler.
There are several solutions, e.g. using a callback that forwards to dynamic list of child callbacks or wrapping the old callback when setting the new one. Ideally, it should also not break 3rd party handlers set on the same client.
Screenshots
none
Desktop (please complete the following information):
- OS: Ubuntu 24.something, using venv
- Browser: why?
- Version: main branch e3f7402