unixorn / ha-mqtt-discoverable

Python module to create MQTT entities that are automatically discovered by Home Assistant
Apache License 2.0
82 stars 21 forks source link

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

Open jedie opened 4 months ago

jedie commented 4 months ago

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 commented 4 weeks ago

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 commented 4 weeks ago

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.

https://github.com/unixorn/ha-mqtt-discoverable/compare/main...smeinecke:ha-mqtt-discoverable:v0.13.1