[BUG] callbacks missing if there is more than one entity...

[jedie]

Describe the bug
command_callback do not work reliably when a device has multiple entities.

To Reproduce
Here a minimal example script.
It creates one device and add a Sensor and a Switch:

import time

from ha_mqtt_discoverable import DeviceInfo, Settings
from ha_mqtt_discoverable.sensors import Sensor, SensorInfo, Switch, SwitchInfo


mqtt_settings = Settings.MQTT(host="localhost")
device_info = DeviceInfo(name="My device", identifiers="device_id")


def get_temperature_sensor():
    temperature_sensor = Sensor(
        Settings(
            mqtt=mqtt_settings,
            entity=SensorInfo(
                name='Chip Temperature',
                unit_of_measurement='°C',
                state_class='measurement',
                device_class='temperature',
                unique_id="temperature",
                device=device_info,
            ),
        )
    )
    temperature_sensor.set_state(state=random.randint(0, 100))
    return temperature_sensor


class Relay:
    def __init__(self):
        self.relay_switch = Switch(
            Settings(
                mqtt=mqtt_settings,
                entity=SwitchInfo(
                    name='Relay',
                    unique_id="relay",
                    device=device_info,
                    payload_on='ON',
                    payload_off='OFF',
                ),
            ),
            command_callback=self.relay_callback,
        )
        self.relay_switch.on()

    def relay_callback(self, client, user_data, message):
        payload = message.payload.decode()
        print(f'\n**** Switch Callback: {client=} {user_data=} {payload=}')
        if payload == 'ON':
            self.relay_switch.on()
        else:
            self.relay_switch.off()


relay = Relay()
temperature_sensor = get_temperature_sensor() # <<< comment this out, then it works fine

while True:
    time.sleep(1)
    print('.', end='', flush=True)

It works, if the "temperature sensor" will be not initialized. Then you can switch the "Relay" ON and OFF in Home Assistant and the callback will be call every time.
But if the "temperature sensor" is also added, then the callback will be sometimes called and sometimes not.

[smeinecke]

Have the same problem, It seems it has something to do that the lib creates multiple instances of the mqtt connection in parallel and starts multiple mqtt_client loops.

Right now, I don't see any easy fix as the connection is established when the Device object is created. There is no way to setup a "global" mqtt client object.

[smeinecke]

I've implemented some ugly hacks to "fix" this for my local instance, but

1. It's ugly as heck as it stores the client connection inside of the MQTT settings object (baaad, but easiest to use). You've to set the "reuse_client" while creating the MQTT object.
2. The first message_callback which was defined is called instead of the matching callback of the topic/device. This can probably be fixed... but not by me.

I simply check in the callback method which topic was set for the message to know for which device the message was send. And its v0.13.1 as I am running this on raspi 1 which is still arm32 and which does not support pydantic2.

main...smeinecke:ha-mqtt-discoverable:v0.13.1

[hlehoux2021]

I have run into the same subject

if i understand correctly, you need to call _connect_client() whenever you setup a callback on an object.

for example i call my_button._connect_client() and i don't have the problem anymore.

this is documented in init of class Discoverable
Args:
settings: Settings for the entity we want to create in Home Assistant.
See the Settings class for the available options.
on_connect: Optional callback function invoked when the MQTT client
successfully connects to the broker.
If defined, you need to call _connect_client() to establish the
connection manually.

[hlehoux2021]

@jedie @smeinecke are you ok with that ?
@unixorn please confirm and then close this false bug ?

[smeinecke]

Oh, goods to know. Will try that in my Service.

[jedie]

I just expand/refactore and use my own solution: https://github.com/jedie/ha-services

[theeldermillenial]

@hlehoux2021 However, in the Subscriber class it allows you to set the command_callback, and in init it actually sets the command callback and inits the connection.

ha-mqtt-discoverable/ha_mqtt_discoverable/__init__.py

Line 871 in 0d07e6f

self._connect_client()

I just ran into an identical problem as @jedie and have been wracking my brain why my Switch isn't showing up.

[hlehoux2021]

@theeldermillenial , reading the source code i guess you're right; my error was to stop reading after the comment in Discoverable. Moreover, _connect_client() appears to be a "private" method (guess what, i'm currently learning python in fact).
i will retest the behavior with and without calling _connect_client()

[theeldermillenial]

No worries.

I actually didn't have the same issue after all. I had a configuration issue in my switch. I haven't had any issues assigning multiple switches and sensors to the same device.

I built a custom thermostat that uses a dht22 to get temp and humidity, then I have a relay switch controlling the heat and another relay switch controlling the furnace fan. I have the temperature and humidity I pull off the dht22 sent to separate sensors, and then I have two pin switches on the raspberry pi.

My home is now running on that, and I didn't have to do any hacks or bug fixes.

I will say that link to the repo with the systemd services gave me some ideas that I implemented.

[hlehoux2021]

I did new tests

• i do not experiment the bug with a code similar to the one of @jedie
• i do not need to call _connect_client

ready paho's example here: mqtt_subscribe.py i think the library does it exactly like the example.

maybe the problem occurs only on some hardware where threads launched by loop_start() are a problem?
I run on Raspberry 3B+

[darloxflyer]

I just started using this library, and have run into the same problem as others here. It seems like maybe others have found workaround solutions, but at least OP seems to have just written their own library rather than using this one...

I can confirm that at least on lower-spec Raspberry Pi devices (like RPi Zero W or Zero 2 W) the issue does indeed appear to be the creation of multiple MQTT loop_start() threads. Create a single entity and it reliably works. Create more than one entity in a single DeviceInfo() object, and it's suddenly random whether or not callbacks to entities will actually trigger.

I have created a device with 8 switch entities inside, and on each execution, the device properly registers all of the entities with the HA server. State changes triggered device-side are properly communicated to the HA server. But if you trigger an event from within HA, relying on a callback attached to the .../command MQTT topic to call your code, only some of the entities will actually receive those messages upon each execution. It's consistent WITHIN each execution -- meaning that if on Run 1, I'm getting callbacks to switch 2 and 7, I can toggle them back-and-forth and continue to get callbacks on 2 and 7. But the other 6 never receive callbacks. If I kill the program, delete the device in HA, and re-run the program, maybe next time I'll get callbacks on switches 1, 3 and 8. But 3 "live" switches is about as close to functional as I've ever come... usually it's 0-2 switches that end up getting callbacks.

So while I can't find any formal documentation on any sort of thread limit, that does seem to be what's happening here. If I run another test program using Paho MQTT where 8 dummy switches are inside of a single Client loop_start() thread, everything seems to work fine. But, of course, then all of the configuration JSON and topic subscriptions have to be handled manually...

This library offers a super convenient way of setting up and managing Discoverable devices, but the one-MQTT-Client-per-entity model appears to be untenable for certain host devices. Would be nice to refactor it to create a single MQTT Client that just appends each device/entity and their callbacks onto a single client object, but based on how this is currently structured, that appears to be a significant amount of work. Would require both abstraction of the MQTT client, and creation of "queue" lists for device/entity setups and subscriptions when client.connect() is finally called.

[leogaggl]

I have a similar setup with a relay board @darloxflyer and experience the same issues.

The issue really got highlighted when trying to add some Number entities as well. The callbacks for the number entities don't work at all.

[markus-seidl]

I've implemented some ugly hacks to "fix" this for my local instance, but

1. It's ugly as heck as it stores the client connection inside of the MQTT settings object (baaad, but easiest to use). You've to set the "reuse_client" while creating the MQTT object.
2. The first message_callback which was defined is called instead of the matching callback of the topic/device. This can probably be fixed... but not by me.

I simply check in the callback method which topic was set for the message to know for which device the message was send. And its v0.13.1 as I am running this on raspi 1 which is still arm32 and which does not support pydantic2.

main...smeinecke:ha-mqtt-discoverable:v0.13.1

I think there is an issue with this patch, as it doesn't connect to the client so there are cases where the setup fails because the client isn't connected.
All this connection handling by this library is confusing me (if a callback is defined, then do connect. But when you write a config then just don't care if you are connected at all, etc.)

Workaround (nasty): If you setup a sensor, which doesn't override init and doesn't manually call connect_client and it's also your first reuse_mqtt entity, you should sneak in a call to _connect_client() .

[markus-seidl]

Reuse client doesn't work, if you ever want callbacks. In the Subscriber there is this line

self.mqtt_client.on_message = command_callback

which always overwrites existing callbacks.