Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reconfigure MQTT binary_sensor component if discovery info is changed #18169

Merged
116 changes: 78 additions & 38 deletions homeassistant/components/binary_sensor/mqtt.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
https://home-assistant.io/components/binary_sensor.mqtt/
"""
import logging
from typing import Optional

import voluptuous as vol

Expand Down Expand Up @@ -79,57 +78,89 @@ async def _async_setup_entity(hass, config, async_add_entities,
value_template.hass = hass

async_add_entities([MqttBinarySensor(
config.get(CONF_NAME),
config.get(CONF_STATE_TOPIC),
config.get(CONF_AVAILABILITY_TOPIC),
config.get(CONF_DEVICE_CLASS),
config.get(CONF_QOS),
config.get(CONF_FORCE_UPDATE),
config.get(CONF_OFF_DELAY),
config.get(CONF_PAYLOAD_ON),
config.get(CONF_PAYLOAD_OFF),
config.get(CONF_PAYLOAD_AVAILABLE),
config.get(CONF_PAYLOAD_NOT_AVAILABLE),
value_template,
config.get(CONF_UNIQUE_ID),
config.get(CONF_DEVICE),
discovery_hash,
config,
discovery_hash
)])


class MqttBinarySensor(MqttAvailability, MqttDiscoveryUpdate,
MqttEntityDeviceInfo, BinarySensorDevice):
"""Representation a binary sensor that is updated by MQTT."""

def __init__(self, name, state_topic, availability_topic, device_class,
qos, force_update, off_delay, payload_on, payload_off,
payload_available, payload_not_available, value_template,
unique_id: Optional[str], device_config: Optional[ConfigType],
discovery_hash):
def __init__(self, config, discovery_hash):
"""Initialize the MQTT binary sensor."""
MqttAvailability.__init__(self, availability_topic, qos,
payload_available, payload_not_available)
MqttDiscoveryUpdate.__init__(self, discovery_hash)
MqttEntityDeviceInfo.__init__(self, device_config)
self._name = name
self._config = config
self._state = None
self._state_topic = state_topic
self._device_class = device_class
self._payload_on = payload_on
self._payload_off = payload_off
self._qos = qos
self._force_update = force_update
self._off_delay = off_delay
self._template = value_template
self._unique_id = unique_id
self._discovery_hash = discovery_hash
self._state_unsub = None
self._delay_listener = None

self._name = None
self._state_topic = None
self._device_class = None
self._payload_on = None
self._payload_off = None
self._qos = None
self._force_update = None
self._off_delay = None
self._template = None
self._unique_id = None

# Load config
self._setup_from_config(config)

availability_topic = config.get(CONF_AVAILABILITY_TOPIC)
payload_available = config.get(CONF_PAYLOAD_AVAILABLE)
payload_not_available = config.get(CONF_PAYLOAD_NOT_AVAILABLE)
device_config = config.get(CONF_DEVICE)

MqttAvailability.__init__(self, availability_topic, self._qos,
payload_available, payload_not_available)
MqttDiscoveryUpdate.__init__(self, discovery_hash,
self.discovery_update)
MqttEntityDeviceInfo.__init__(self, device_config)

async def async_added_to_hass(self):
"""Subscribe mqtt events."""
await MqttAvailability.async_added_to_hass(self)
await MqttDiscoveryUpdate.async_added_to_hass(self)
await self._subscribe_topics(None)

async def discovery_update(self, discovery_payload):
"""Handle updated discovery message."""
config = PLATFORM_SCHEMA(discovery_payload)
old_state_topic = self._state_topic
emontnemery marked this conversation as resolved.
Show resolved Hide resolved
self._setup_from_config(config)
await self._subscribe_topics(old_state_topic)
self.async_schedule_update_ha_state()

def _setup_from_config(self, config):
"""(Re)Setup the entity."""
self._name = config.get(CONF_NAME)
self._state_topic = config.get(CONF_STATE_TOPIC)
self._device_class = config.get(CONF_DEVICE_CLASS)
self._qos = config.get(CONF_QOS)
self._force_update = config.get(CONF_FORCE_UPDATE)
self._off_delay = config.get(CONF_OFF_DELAY)
self._payload_on = config.get(CONF_PAYLOAD_ON)
self._payload_off = config.get(CONF_PAYLOAD_OFF)
value_template = config.get(CONF_VALUE_TEMPLATE)
if value_template is not None:
value_template.hass = self.hass
self._template = value_template

# TODO: What if unique ID is changed?
emontnemery marked this conversation as resolved.
Show resolved Hide resolved
self._unique_id = config.get(CONF_UNIQUE_ID)

# TODO: Handle changed availability_topic / payload?
# config.get(CONF_AVAILABILITY_TOPIC),
# config.get(CONF_PAYLOAD_AVAILABLE),
# config.get(CONF_PAYLOAD_NOT_AVAILABLE),

# TODO: Handle changed device?
# config.get(CONF_DEVICE),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@OttoWinter What do you suggest to do if device info is changed, just update the MqttEntityDeviceInfo?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not the same as availability_discovery_update, just called device_info_discovery_update - this method would set the internal self._device_info object to the latest values.

(It might be the case though that the device_info is only read when the device is initially added to hass, then I guess we ignore the update to this attribute, as it's probably not used too often)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be the case though that the device_info is only read when the device is initially added to hass

Yes, this seems to be the cased based on some quick testing.
Either just ignore this case as you suggest, or delete + re-add the component.


async def _subscribe_topics(self, old_state_topic):
"""(Re)Subscribe to topics."""
@callback
def off_delay_listener(now):
"""Switch device off after a delay."""
Expand Down Expand Up @@ -162,8 +193,17 @@ def state_message_received(_topic, payload, _qos):

self.async_schedule_update_ha_state()

await mqtt.async_subscribe(
self.hass, self._state_topic, state_message_received, self._qos)
if self._state_topic != old_state_topic:
if self._state_unsub is not None:
self._state_unsub()
self._state_unsub = await mqtt.async_subscribe(
self.hass, self._state_topic, state_message_received,
self._qos)

async def async_will_remove_from_hass(self):
"""Unsubscribe when removed."""
if self._state_unsub is not None:
emontnemery marked this conversation as resolved.
Show resolved Hide resolved
self._state_unsub()

@property
def should_poll(self):
Expand Down
7 changes: 6 additions & 1 deletion homeassistant/components/mqtt/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -864,9 +864,10 @@ def available(self) -> bool:
class MqttDiscoveryUpdate(Entity):
"""Mixin used to handle updated discovery message."""

def __init__(self, discovery_hash) -> None:
def __init__(self, discovery_hash, discovery_update=None) -> None:
"""Initialize the discovery update mixin."""
self._discovery_hash = discovery_hash
self._discovery_update = discovery_update
self._remove_signal = None

async def async_added_to_hass(self) -> None:
Expand All @@ -886,6 +887,10 @@ def discovery_callback(payload):
self.hass.async_create_task(self.async_remove())
del self.hass.data[ALREADY_DISCOVERED][self._discovery_hash]
self._remove_signal()
emontnemery marked this conversation as resolved.
Show resolved Hide resolved
elif self._discovery_update:
# Non-empty payload: Notify component
_LOGGER.info("Updating component: %s", self.entity_id)
self.hass.async_create_task(self._discovery_update(payload))

if self._discovery_hash:
self._remove_signal = async_dispatcher_connect(
Expand Down
22 changes: 12 additions & 10 deletions homeassistant/components/mqtt/discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,30 +214,32 @@ async def async_device_message_received(topic, payload, qos):

discovery_hash = (component, discovery_id)

if discovery_hash in hass.data[ALREADY_DISCOVERED]:
_LOGGER.info(
"Component has already been discovered: %s %s, sending update",
component, discovery_id)
async_dispatcher_send(
hass, MQTT_DISCOVERY_UPDATED.format(discovery_hash), payload)
elif payload:
# Add component
if payload:
platform = payload.get(CONF_PLATFORM, 'mqtt')
if platform not in ALLOWED_PLATFORMS.get(component, []):
_LOGGER.warning("Platform %s (component %s) is not allowed",
platform, component)
return

payload[CONF_PLATFORM] = platform

if CONF_STATE_TOPIC not in payload:
payload[CONF_STATE_TOPIC] = '{}/{}/{}{}/state'.format(
discovery_topic, component,
'%s/' % node_id if node_id else '', object_id)

hass.data[ALREADY_DISCOVERED][discovery_hash] = None
payload[ATTR_DISCOVERY_HASH] = discovery_hash

if discovery_hash in hass.data[ALREADY_DISCOVERED]:
# Dispatch update
_LOGGER.info(
"Component has already been discovered: %s %s, sending update",
component, discovery_id)
async_dispatcher_send(
hass, MQTT_DISCOVERY_UPDATED.format(discovery_hash), payload)
elif payload:
# Add component
_LOGGER.info("Found new component: %s %s", component, discovery_id)
hass.data[ALREADY_DISCOVERED][discovery_hash] = None

if platform not in CONFIG_ENTRY_PLATFORMS.get(component, []):
await async_load_platform(
Expand Down
30 changes: 30 additions & 0 deletions tests/components/binary_sensor/test_mqtt.py
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,36 @@ async def test_discovery_removal_binary_sensor(hass, mqtt_mock, caplog):
assert state is None


async def test_discovery_update_binary_sensor(hass, mqtt_mock, caplog):
"""Test removal of discovered binary_sensor."""
entry = MockConfigEntry(domain=mqtt.DOMAIN)
await async_start(hass, 'homeassistant', {}, entry)
data1 = (
'{ "name": "Beer",'
' "status_topic": "test_topic" }'
)
data2 = (
'{ "name": "Milk",'
' "status_topic": "test_topic" }'
)
async_fire_mqtt_message(hass, 'homeassistant/binary_sensor/bla/config',
data1)
await hass.async_block_till_done()
state = hass.states.get('binary_sensor.beer')
assert state is not None
assert state.name == 'Beer'
async_fire_mqtt_message(hass, 'homeassistant/binary_sensor/bla/config',
data2)
await hass.async_block_till_done()
await hass.async_block_till_done()
state = hass.states.get('binary_sensor.beer')
assert state is not None
assert state.name == 'Milk'

state = hass.states.get('binary_sensor.milk')
assert state is None


async def test_entity_device_info_with_identifier(hass, mqtt_mock):
"""Test MQTT binary sensor device registry integration."""
entry = MockConfigEntry(domain=mqtt.DOMAIN)
Expand Down