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

#21072 Match endpoint name in the mqtt topic based on endpoints listed in the converter #21214

Merged
merged 12 commits into from
Feb 7, 2024

Conversation

grafalex82
Copy link
Contributor

@grafalex82 grafalex82 commented Feb 4, 2024

Original issue report: #21072

Problem: Zigbee2mqtt uses predefined endpoint names list when matching mqtt topic name or mqtt message payload. If a device is using a non-standard endpoint name, mqtt messages sent to this endpoint are not recognized properly.

Fix: This change enables matching of mqtt topics and payload without using the predefined endpoints list. Instead it gets a list of endpoint names from the device definition's (converter).

The are 2 changes to make this happen:

  • Topic name matching is corrected so that does not use the predefined endpoints list. This involves solving an ambiguouty between device/name/with/slashes and device/endpoint.
  • Matching attribute_endpoint-like payload involves collecting the list of device endpoint and matching the endpoint name according to that list.

In addition to make tests happy, the following 3 changes were done:

  • device.ts: Not all converters provide the full name-to-id map (for example https://github.com/Koenkk/zigbee-herdsman-converters/blob/master/src/devices/gledopto.ts#L310 provide names only for 2 of 4 endpoints). As result reverse match from ID to endpoint name failed for endpoints without name. Now the null is returned instead.
  • publish.test.js: The test tries to get state_center from the device that does not have center endpoint. Previous implementation treated center as endpoint name and displayed Device 'wall_switch_double' has no endpoint 'center'. As soon as center endpoint is not stated on the endpoint list, the new code treats state_center as an attribute name, and therefore the error message changes to No converter available for 'state_center'.
  • publish.ts: if(localTarget == null) block was removed - endpoint name shall be matched by the regexp above (which now matches only defined endpoint names), so getting endpoint by name shall always suceed.

@Koenkk
Copy link
Owner

Koenkk commented Feb 4, 2024

Looks good!

Could we also get rid of

const endpointNames = [
?

@grafalex82
Copy link
Contributor Author

I think this is possible. But do you think it worth doing that in a single PR?

I did a quick look, and this list is used in 4-5 places across the code. Some are easy to fix, other will require some work, and probably additional changes accross the codebase.

@Koenkk
Copy link
Owner

Koenkk commented Feb 5, 2024

I'm fine with doing it in a separate pr. I will release a hotfix z2m release soon and will merge this PR after that.

@grafalex82
Copy link
Contributor Author

Thanks.

FYI: another PR on getting rid of endpointNames. It is independent of this PR, and can be merged in any order.

@grafalex82
Copy link
Contributor Author

Since this PR is waiting to be merged anyway, I did a small additional change on the same topic. It was reasonable to make it based on this PR change, rather than separately. Please take a look before merging.

@Koenkk Koenkk changed the base branch from master to dev February 7, 2024 19:12
@Koenkk Koenkk enabled auto-merge (squash) February 7, 2024 19:12
@Koenkk
Copy link
Owner

Koenkk commented Feb 7, 2024

Thanks!

@Koenkk Koenkk disabled auto-merge February 7, 2024 19:12
@Koenkk Koenkk merged commit a5a87a7 into Koenkk:dev Feb 7, 2024
8 checks passed
@kirovilya
Copy link
Contributor

I discovered that the logic for working with endpoints has changed here.
As a result, some Tuya-devices with several “dummy”-endpoints stopped working.

For example, like here https://github.com/Koenkk/zigbee-herdsman-converters/blob/master/src/devices/tuya.ts#L1745
In this case, states "state_l1", "state_l2", etc. are created in the web-frontend. But, when trying to manage them, an error appears like: No converter available for 'state_l1'.

Reason - the corresponding endpoint names were not found in the list https://github.com/Koenkk/zigbee2mqtt/blob/dev/lib/extension/publish.ts#L199 - for such Tuya-devices, there is one last endpoint ['l4'].
I tried to solve it here #21550, but all the tests are not adapted and diverge.

@grafalex82
Copy link
Contributor Author

Thanks for reporting, I think I understand the issue.
I'll take a look early next week.

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

Successfully merging this pull request may close these issues.

3 participants