tsightler / ring-mqtt

Ring devices to MQTT Bridge
MIT License
578 stars 104 forks source link

Bug: Some HA discovery topics have no name #755

Closed jshatch closed 11 months ago

jshatch commented 11 months ago

Describe the Bug

Some HA discovery topics have an undefined name in the config JSON

Steps to Reproduce

Load your favorite MQTT browser and look in the homeassistant/# topic for alarm, glassbreak, and contact and they will have no name defined

Expected Behavior

The name should be defined so discovery can complete (this throws an error in OpenHAB)

Log Output

Here is an example of what to look for, I'll include the entire (IDs redacted) log in an email.

2023-11-11T05:29:52.679Z ring-disc HASS config topic: homeassistant/alarm_control_panel/9****5-4***n-0/2c247b89-xxxx-xxxx-xxxx-439ec67298f6_alarm/config
2023-11-11T05:29:52.680Z ring-disc {
  name: '',
  unique_id: '2c247b89-xxxx-xxxx-xxxx-439ec67298f6',
  state_topic: 'ring/9****5-4***n-0/alarm/2c247b89-xxxx-xxxx-xxxx-439ec67298f6/alarm/state',
  command_topic: 'ring/9****5-4***n-0/alarm/2c247b89-xxxx-xxxx-xxxx-439ec67298f6/alarm/command',
  json_attributes_topic: 'ring/9****5-4***n-0/alarm/2c247b89-xxxx-xxxx-xxxx-439ec67298f6/alarm/attributes',
  supported_features: [ 'arm_home', 'arm_away' ],
  availability_topic: 'ring/9****5-4***n-0/alarm/2c247b89-xxxx-xxxx-xxxx-439ec67298f6/status',
  payload_available: 'online',
  payload_not_available: 'offline',
  device: {
    ids: [ '2c247b89-xxxx-xxxx-xxxx-439ec67298f6' ],
    name: 'Home Alarm',
    mf: 'Ring',
    mdl: 'Alarm Control Panel'
  }
}

The "name:" is missing.  Here is a proper entry:

2023-11-11T05:29:52.680Z ring-disc HASS config topic: homeassistant/switch/9****5-4***n-0/2c247b89-xxxx-xxxx-xxxx-439ec67298f6_siren/config
2023-11-11T05:29:52.681Z ring-disc {
  name: 'Siren',
  unique_id: '2c247b89-xxxx-xxxx-xxxx-439ec67298f6_siren',
  state_topic: 'ring/9****5-4***n-0/alarm/2c247b89-xxxx-xxxx-xxxx-439ec67298f6/siren/state',
  command_topic: 'ring/9****5-4***n-0/alarm/2c247b89-xxxx-xxxx-xxxx-439ec67298f6/siren/command',
  icon: 'mdi:alarm-light',
  availability_topic: 'ring/9****5-4***n-0/alarm/2c247b89-xxxx-xxxx-xxxx-439ec67298f6/status',
  payload_available: 'online',
  payload_not_available: 'offline',
  device: {
    ids: [ '2c247b89-xxxx-xxxx-xxxx-439ec67298f6' ],
    name: 'Home Alarm',
    mf: 'Ring',
    mdl: 'Alarm Control Panel'
  }
}

Screenshots

No response

Config File

{
    "mqtt_url": "mqtt://ring:***@openhab.local.network:1883",
    "mqtt_options": "",
    "livestream_user": "",
    "livestream_pass": "",
    "disarm_code": "",
    "enable_cameras": false,
    "enable_modes": false,
    "enable_panic": false,
    "hass_topic": "openhab/status",
    "ring_topic": "ring",
    "location_ids": []
}

Install Type

Docker

Version

v5.6.3

Operating System

Ubuntu 20.04.6 LTS

Architecture

x86_64

Machine Details

VM (KVM, CentOS 7.9)

jshatch commented 11 months ago

Log entry that I believe is caused by this:

00:56:18.977 [WARN ] [assistant.internal.DiscoverComponents] - HomeAssistant discover error: Label for a ChannelType must not be empty.
00:56:21.024 [WARN ] [ommon.WrappedScheduledExecutorService] - Scheduled runnable ended with an exception: 
java.lang.IllegalArgumentException: Label for a ChannelGroupType must not be empty.
    at org.openhab.core.thing.type.ChannelGroupTypeBuilder.instance(ChannelGroupTypeBuilder.java:45) ~[?:?]
    at org.openhab.binding.mqtt.homeassistant.internal.component.AbstractComponent.getType(AbstractComponent.java:253) ~[?:?]
    at org.openhab.binding.mqtt.homeassistant.internal.component.AbstractComponent.addChannelTypes(AbstractComponent.java:161) ~[?:?]
    at org.openhab.binding.mqtt.homeassistant.internal.handler.HomeAssistantThingHandler.accept(HomeAssistantThingHandler.java:292) ~[?:?]
    at org.openhab.binding.mqtt.homeassistant.internal.handler.HomeAssistantThingHandler.accept(HomeAssistantThingHandler.java:1) ~[?:?]
    at org.openhab.binding.mqtt.generic.tools.DelayedBatchProcessing.run(DelayedBatchProcessing.java:110) ~[?:?]
    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539) ~[?:?]
    at java.util.concurrent.FutureTask.run(FutureTask.java:264) ~[?:?]
    at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:304) ~[?:?]
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136) [?:?]
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635) [?:?]
    at java.lang.Thread.run(Thread.java:833) [?:?]
jshatch commented 11 months ago

I manually published the topic and added the missing name for alarm and the channel was immediately discovered and works. So the missing name indeed seems to be the cause of this error.

tsightler commented 11 months ago

You'll need to open an issue with OpenHAB and have them update their HA discovery functions to follow the Home Assistant entity naming rules that went into effect in Aug 2023. Under these new rules entities with no name are allowed and are used to identify the default entity for a given device. From the Home Assistant MQTT Entity Naming documentation:

Unnamed binary_sensor, button, number and sensor entities will now be named by their device class instead of being named “MQTT binary sensor” etc. It’s allowed to set an MQTT entity’s name to None (use null in YAML) to mark it as the main feature of a device.

You can also reference the Home Assistant MQTT developer blog from August:

https://developers.home-assistant.io/blog/2023/07/21/change-naming-mqtt-entities/#naming-of-mqtt-entities

jshatch commented 11 months ago

OK, I went back and found that you changed this in 5.6.0, installing 5.5.1 has fixed the issue for me. I'll open a bug report with openHAB.

jshatch commented 11 months ago

Looks like they're already on it. https://github.com/openhab/openhab-addons/pull/15427

jshatch commented 10 months ago

For reference: https://github.com/openhab/openhab-addons/pull/15918

tsightler commented 10 months ago

Interesting, I had tried to use null when I initially coded this but I couldn't get it to work, maybe I should change this on my side.