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

Prepare to move specific ZDO requests out of Adapter #1187

Merged
merged 32 commits into from
Sep 21, 2024

Conversation

Nerivec
Copy link
Collaborator

@Nerivec Nerivec commented Sep 14, 2024

Final goal is to remove the following functions from Adapter in favor of a generic sendZdo with requests built and sent directly from Controller/Device/Endpoint:

  • changeChannel
  • permitJoin
  • lqi
  • routingTable
  • nodeDescriptor
  • activeEndpoints
  • simpleDescriptor
  • bind
  • unbind
  • removeDevice

This should make the API far more robust against changes (by not requiring updates in every adapter) and offer full payloads to the upstream code (instead of the current limited set of properties), which could lead to a few new features.
Due to the sizeable overhaul required in some stacks, this PR will focus first on introducing the sendZdo function while keeping above functions in place until they can safely be removed (all adapters are required to have support before that can happen).

A working example (albeit using an older implementation) of the full update for ember (as well as some tentative partial implementations in other stacks) can be found in this branch.

TODO:

  • Determine & implement per-stack request payloads variations (see *-zdo.refactor.test.ts)
  • Refactor above functions to use sendZdo
    • ember
    • zstack
    • deconz
    • zigate (need testing)
    • zboss
    • ezsp
  • Refactor payload parsing to use Zdo.Buffalo.readResponse for ZDO frames
    • ember
    • zstack
    • deconz
    • zigate (need testing)
    • zboss
    • ezsp

CC: @kirovilya @ChrisHae @G1K @devbis contributions/ideas on specific stacks welcome!

@Nerivec
Copy link
Collaborator Author

Nerivec commented Sep 18, 2024

@kirovilya Could you test out zboss?
@devbis @fairecasoimeme Could you test out zigate?

Lots of moving parts, need to confirm nothing broke. If you can report back issues.
A round of permit join + join device (interview) + network map + remove device should pretty much trigger all involved ZDOs.
Change channel requires changing the channel in configuration.yaml then starting Z2M again. (Though depending on the start logic of the adapter, it might not trigger at all.)

@kirovilya

This comment was marked as resolved.

@Koenkk Koenkk merged commit d84725b into Koenkk:master Sep 21, 2024
1 check passed
@Koenkk
Copy link
Owner

Koenkk commented Sep 21, 2024

@devbis @fairecasoimeme please test the ZiGate adapter with the latest dev branch to ensure it will not break with the next release.

A round of permit join + join device (interview) + network map + remove device should pretty much trigger all involved ZDOs.

Changes will be available in the dev branch in a few hours from now.

@devbis
Copy link
Contributor

devbis commented Sep 23, 2024

@Nerivec @Koenkk
For zigate this code is broken:
https://github.com/Koenkk/zigbee-herdsman/blob/master/src/adapter/zigate/adapter/zigateAdapter.ts#L238C32-L240

z2m: Request 'zigbee2mqtt/bridge/request/device/configure_reporting' failed with error: 'Bind 0xa4c1381d13758a9b/1 haElectricalMeasurement from '0x00158d0003976ca2/1' failed (offset is out of bounds)'

payload.length = 21
zeroes = -6 (!)

the payload here (in hex) was "a4c1381d13758a9b010b040300158d0003976ca201"

I suppose that the problem is in the ADDRESS_WITH_TYPE_DEPENDENCY field. It can be 2 or 8 bytes long.
Changing the buffer length to 21 works in this case.

@devbis
Copy link
Contributor

devbis commented Sep 24, 2024

@Nerivec
Sorry to deliver feedback by portions but I don't have an opportunity to test it thoroughly at the moment.
Removing devices also doesn't work for zigate.

[2024-09-24 10:55:17] info: 	z2m: Removing device '0xa4c1381d13758a9b' (block: false, force: false)
[2024-09-24 10:55:17] debug: 	zh:zigate:driver: ZDO LEAVE_REQUEST(cmd code: 76) 5696a4c1381d13758a9b0000
[2024-09-24 10:55:17] debug: 	zh:zigate:driver: ZDO {"msgCodeBytes":{"type":"Buffer","data":[0,76]},"msgLengthBytes":{"type":"Buffer","data":[0,12]},"checksumBytes":{"type":"Buffer","data":[183]},"msgPayloadBytes":{"type":"Buffer","data":[86,150,164,193,56,29,19,117,138,155,0,0]},"rssiBytes":{"type":"Buffer","data":[]},"msgLengthOffset":0}
[2024-09-24 10:55:17] debug: 	zh:zigate:driver: <-- ZDO send command 0102104c0210021cb75696a4c1381d13758a9b0210021003
[2024-09-24 10:55:17] debug: 	zh:zigate:frame: {"msgCodeBytes":{"type":"Buffer","data":[128,0]},"msgLengthBytes":{"type":"Buffer","data":[0,9]},"checksumBytes":{"type":"Buffer","data":[13]},"msgPayloadBytes":{"type":"Buffer","data":[200,0,0,76,0,0,0,0]},"rssiBytes":{"type":"Buffer","data":[0]},"msgLengthOffset":-1}
[2024-09-24 10:55:17] debug: 	zh:zigate:driver: --> parsed frame >>>> Status 0x8000 <<<<
[2024-09-24 10:55:17] debug: 	zh:zigate:object: Last bytes of data were not parsed c8 00 00 4c 00 00 00 00
[2024-09-24 10:55:17] debug: 	zh:zigate:driver: {"status":200,"sequence":0,"packetType":76}
[2024-09-24 10:55:27] error: 	z2m: Request 'zigbee2mqtt/bridge/request/device/remove' failed with error: 'Failed to remove device '0xa4c1381d13758a9b' (block: false, force: false) (Error: {"clusterId":32820,"target":22166} after 10000ms)'
[2024-09-24 10:55:27] debug: 	z2m: Error: Failed to remove device '0xa4c1381d13758a9b' (block: false, force: false) (Error: {"clusterId":32820,"target":22166} after 10000ms)
    at Bridge.removeEntity (/projects/zigbee2mqtt/lib/extension/bridge.ts:718:19)
    at Bridge.deviceRemove (/projects/zigbee2mqtt/lib/extension/bridge.ts:259:16)
    at Bridge.onMQTTMessage (/projects/zigbee2mqtt/lib/extension/bridge.ts:200:34)
    at EventEmitter.wrappedCallback (/projects/zigbee2mqtt/lib/eventBus.ts:206:17)

Additionally, I found out that clusters for the device are read in wrong endian:

(from database.json)

      "inClusterList": [
        0,
        768,
        256,
        1284,
        516,
        10756,
        12292,
        12548
      ],
      "outClusterList": [
        6400
      ],

6400 = 0x1900 should be 0x0019 = OTA (and others are reversed as well). I suppose, reading from the endpoint is parsed incorrectly

@Nerivec Nerivec mentioned this pull request Sep 24, 2024
@Nerivec Nerivec deleted the zdo-move branch October 2, 2024 19:15
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.

4 participants