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

Warning after HASS upgrade to 2023.8 #279

Closed
markus-lassfolk opened this issue Aug 4, 2023 · 10 comments
Closed

Warning after HASS upgrade to 2023.8 #279

markus-lassfolk opened this issue Aug 4, 2023 · 10 comments

Comments

@markus-lassfolk
Copy link

Notice showing up:

This stops working in version 2024.2.0. Please address before upgrading

Some MQTT entities have an entity name equal to the device name. This is not expected. The entity name is set to null as a work-a-round to avoid a duplicate name. Please inform the maintainer of the software application that supplies the affected entities to fix this issue.

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

@faanskit
Copy link
Contributor

faanskit commented Aug 6, 2023

I suspect the resolution to this issue resides in MqttClient.js, getOutputDeviceDiscoveryPayload().

As can be seen below, both name: and device: { name: } picks up its input from device.name, making the entity name equal to the device name as reported by Home Assistant Warning Message.

MqttClient.js

const getOutputDeviceDiscoveryPayload = (
  /** @type {import('./types/DeviceRegistry').OutputDevice} */ device,
) => ({
  name: device.name,
  unique_id: device.uniqueId,
  '~': getBaseTopic(device.uniqueId, device.type),
  state_topic: `~/${TOPIC_TYPES.STATE}`,
  command_topic: `~/${TOPIC_TYPES.COMMAND}`,
  availability_topic: `~/${TOPIC_TYPES.AVAILABILITY}`,
  optimistic: false,
  qos: 1,
  retain: true,
  device: {
    identifiers: `${device.uniqueId}`,
    manufacturer: 'Plejd',
    model: device.typeName,
    name: device.name,
    ...(device.roomName !== undefined ? { suggested_area: device.roomName } : {}),
    sw_version: device.version,
  },
  ...(device.type === MQTT_TYPES.LIGHT ? { brightness: device.dimmable, schema: 'json' } : {}),
});

image

A quick and dirty way to fix this would be to prefix/suffix the device name.

From what I can read (here), it also appears as it is possible to completely remove the device name.
image

I'm still not fully understood the model of the Device Registry of Home Assistant and how to map that to Plejd. @SweVictor did reach out to the developers of HA once when there was a discussion on Entity vs. Device before.

The complexity comes with devices like DIM-02 and REL-02, which currently is considered two devices and two entities. With the new and revised model of HA, it appears as the DIM-02/REL-02 should be considered one device with two entities and the MQTT discovery should make use of connection to link multiple entities to a single device.
But this is not proven right and likely a complex fix.

@SweVictor, are you still maintaining this integration on behalf of @icanos? As you can see, the integration will stop working from HA 2024.2.0

/Marcus

@SweVictor
Copy link
Collaborator

Nice heads up! And regarding "maintaining", maybe that's saying too much... Helping out from time to time 😄

"Stop working" might be a bit harsh, but it sounds like something we should address.

Diving a bit deeper into the documentation, I found this:

The friendly_name state attribute is generated by combining then entity name with the device name as follows:

The entity is not a member of a device: friendly_name = entity.name
The entity is a member of a device and entity.name is not None: friendly_name = f"{device.name} {entity.name}"
The entity is a member of a device and entity.name is None: friendly_name = f"{device.name}"
Entity names should start with a capital letter, the rest of the words are lower case (unless it's a proper noun or a capitalized abbreviation of course).

(from https://developers.home-assistant.io/docs/core/entity/#entity-naming)

So, it sounds like the expectation is that we set entity.name = Null.

So, I actually think we should send (from your example):

name: Null,
unique_id: ...
...
device: {
  name: Taklampor
  ...
}

Thoughts?
/Victor

@faanskit
Copy link
Contributor

faanskit commented Aug 9, 2023

"Stop working" might be a bit harsh, but it sounds like something we should address.

image

Stop working is actually what Home Assistant says. This message starts to appear with the upgrade to Home Assistant 2023.8.1.

Thoughts? /Victor

I think you are right and I fear this may be one of these breaking changes. Guess someone need to try this in a friendly environment first.

@timjackson
Copy link

Without having more knowledge of HA than as a user, I looked at this a bit, and also concluded that entity name should probably be null, using the current method of enumerating devices/entities.

However, I noted that technically the way devices/entities are modelled could possibly be made more "correct" according to the HA model - if I understand correctly (and I might not), today Plejd devices with multiple channels (e.g. DIM-02 or REL-02) are modelled as two separate HA devices with one entity each, whereas they should perhaps technically be one device with two entities - in which case the entity names should probably not be null, but perhaps named according to the devices they control. (e.g. a physical DIM-02 where one dimmer channel might be e.g. "Spotlights" and another "Ceiling light"). However, I'm not sure whether there is much (or anything) to be gained from such an effort, in particular because Plejd already abstracts the physical devices and the control surfaces, so the actual hierarchy of physical devices is not that interesting inside HA. Furthermore, we likely wouldn't have much of a meaningful name for the physical device (other than perhaps the room it's in, e.g. "Dimmer in Kitchen"), so the user experience in HA might well be worse by making this more "correct".

To summarise, as I understand it the current state for modelling a DIM-02 looks something like this:

Device name: Spotlights
- Entity name: Spotlights
Device name: Ceiling light
- Entity name: Ceiling light

and, the proposed easy fix:

Device name: Spotlights
- Entity name: Null
Device name: Ceiling light
- Entity name: Null

Possibly more "correct" but more complicated fix:

Device name: Dimmer in [room name]
- Entity name: Spotlights
- Entity name: Ceiling light

@SweVictor
Copy link
Collaborator

However, I noted that technically the way devices/entities are modelled could possibly be made more "correct" according to the HA model ...

Absolutely true. And if we're really ambitious, looking at services in the Plejd app you see a quite complex data model.

Ex: one DIM-01 has

  • Output
  • Input 1
  • Input 2
  • Rotary input
  • Accessory (showing the RTR-01)

@SweVictor
Copy link
Collaborator

Alright - did a quick fix of this in the https://github.com/icanos/hassio-plejd/tree/feature/set-device-name-null

The branch is built on the upcoming 0.10.0 release. Feel free to test at your own risk 😄

I did a very quick sanity check - it does not immediately break my system.

@ErikThorsell
Copy link

I did a very quick sanity check - it does not immediately break my system.

Spoken like a true developer!

Thank you for the fix. 👍🏼 If nothing else, I'm sure it's a step in the right direction.

@SweVictor
Copy link
Collaborator

This fix is now in the https://github.com/icanos/hassio-plejd/tree/develop branch since the release of 0.10.0.

Planned for the next release.

@timjackson
Copy link

[...] if we're really ambitious, looking at services in the Plejd app you see a quite complex data model.

Ex: one DIM-01 has
* Output
* Input 1
* Input 2
* Rotary input
* Accessory (showing the RTR-01)

True, although I'm not sure whether there's anything to be gained by trying to model it in that detail, or indeed if it's even possible (IIRC from a separate issue, the movement of a rotary input can't be determined separately from a command to dim the configured device, and similarly is it possible to get data about specific inputs?)

One possibly interesting entity to model (if it's possible) might be BAT-01, although even then I don't know if it's possible to fetch something useful to display in HA (e.g. battery level) from it.

Perhaps the discussion of modelling (vs stopping the HA complaints about entity name) should be moved to a separate issue?

Additionally, from the changelog, 2023.8.4 stops the complaints about entity names.

@SweVictor
Copy link
Collaborator

Fixed in 0.10.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants