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

Improvement: Entities containing "light_" in their name produce an erroneous publication #17594

Closed
ericgu08 opened this issue May 8, 2023 · 3 comments
Labels
problem Something isn't working

Comments

@ericgu08
Copy link

ericgu08 commented May 8, 2023

What happened?

In sinope.js, several thermostats expose the backlight_auto_dim parameter. The publication of the payload is always followed by a second publication such as:

Info 2023-05-08 00:24:50MQTT publish: topic 'zigbee2mqtt/FRIENDLY NAME/auto_dim', payload '{"backlight":null}'

This publication is made from splitting backlight_ and auto_dim, and do not correspond to actual data.

The issue appears to come from homeassistant.ts, in the following function:

    @bind async onPublishEntityState(data: eventdata.PublishEntityState): Promise<void> {
        /**
         * In case we deal with a lightEndpoint configuration Zigbee2MQTT publishes
         * e.g. {state_l1: ON, brightness_l1: 250} to zigbee2mqtt/mydevice.
         * As the Home Assistant MQTT JSON light cannot be configured to use state_l1/brightness_l1
         * as the state variables, the state topic is set to zigbee2mqtt/mydevice/l1.
         * Here we retrieve all the attributes with the _l1 values and republish them on
         * zigbee2mqtt/mydevice/l1.
         */
        const entity = this.zigbee.resolveEntity(data.entity.name);
        if (entity.isDevice() && this.discovered[entity.ieeeAddr]) {
            for (const objectID of this.discovered[entity.ieeeAddr].objectIDs) {
                const lightMatch = /light_(.*)/.exec(objectID);
                const coverMatch = /cover_(.*)/.exec(objectID);

                const match = lightMatch || coverMatch;

                if (match) {
                    const endpoint = match[1];
                    const endpointRegExp = new RegExp(`(.*)_${endpoint}`);
                    const payload: KeyValue = {};
                    for (const key of Object.keys(data.message)) {
                        const keyMatch = endpointRegExp.exec(key);
                        if (keyMatch) {
                            payload[keyMatch[1]] = data.message[key];
                        }
                    }

                    await this.mqtt.publish(
                        `${data.entity.name}/${endpoint}`, stringify(payload), {},
                    );
                }
            }
        }

I'm not very familiar with Javascript regular expression, but maybe it's possible to narrow the match pattern further or add additional logic in order to perform this logic only for the intended case

The same behaviour happens if the name contains cover_ instead

What did you expect to happen?

I expect only the thermostat payload would be publish.

How to reproduce it (minimal and precise)

When Zigbee2MQTT is integrated with Homeassistant,
Expose an entity containing "light_" or "cover_", like backlight_auto_dim.
When the device payload is published, it will be followed by a second publish made from the split at light_.

Zigbee2MQTT version

1.30.4-1

Adapter firmware version

6.10.3.0 build 297

Adapter

Itead Sonoff Dongle-E

Debug log

No response

@ericgu08 ericgu08 added the problem Something isn't working label May 8, 2023
@ericgu08 ericgu08 changed the title Improvement: Entities containing "light_" in its name produce an erroneous publication Improvement: Entities containing "light_" in their name produce an erroneous publication May 8, 2023
@Koenkk
Copy link
Owner

Koenkk commented May 8, 2023

Can you try with const lightMatch = /^light_(.*)/.exec(objectID);

@ericgu08
Copy link
Author

ericgu08 commented May 9, 2023

Hello Koen, yes that works perfectly! The erroneous publication is gone. I trust that you remember #3003 and this change will not cause a regression.

It's not a critical issue, but I have to admit that I spent quite a bit of time figuring out where this auto_dim could have come from. Glad the solution is simple. Tonight I learned how to run Z2M in standalone, rebuild and even connect to my home assistant. More tools to put in my toolbox.

Thanks!

@Koenkk
Copy link
Owner

Koenkk commented May 9, 2023

Integrated the fix, thanks!

Changes will be available in the dev branch in a few hours from now. (https://www.zigbee2mqtt.io/advanced/more/switch-to-dev-branch.html)

@Koenkk Koenkk closed this as completed May 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
problem Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants