-
Notifications
You must be signed in to change notification settings - Fork 356
Refactor of deconz adapter to solve various problems #1417
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
Conversation
|
Thanks for the PR! The Deconz adapter really can use some love ❤️ It would be nice if the missing features can be implemented, then it can be moved back to Recommended.
Currently APS ack is always enabled, if not acked the message will just time-out. Note that on a higher level (above the adapter) a
Therefore I would not do this (to keep adapter implementations consistent).
In the ideal world yes, but in reality vendors have implemented different levels of abstraction. E.g. the zStack adapters don't propagate the raw ZDO messages but only the parsed ones. |
|
Hi, thanks yes the refresh is much needed, I hope the most urgent issues can be resolved.
The PR made a bit progress on this:
Ah alright I'll enable the ACKs then, from observation in the sniffer there isn't much polling towards devices so it should be ok in usual operation. I might add a little quirk later on that tracks how many parallel APS unicast requests are running and ensure that at least 2 request slots are always usable without ACKs so that the queue never blocks for too long (iirc there are max. 12 parallel APS requests in current firmware). |
Putting the |
Much todo, focus on async problems first.
Only forward valid packages which: - Have correct CRC - Correct stored_length field - Minimum size of 5 bytes
As the "rxFrame" event only forwards valid packages with a minimum length of 5, parseFrame() doesn't have to deal with invalid packets anymore.
Doesn't make sense to increase the call stack for no reason.
- Fix async handling of errors, before the function did wait for data even if enqueueSendDataRequest() did run into errors and called reject(). - Fix returning correct `wasBroadcast` field, as there is no srcAddressMode 0xF. There can be group casts or various NWK broadcast addresses. - Improve (?) handling of `disableResponse` argument. If it is false always expect a response, which could be specific response or ZCL default response. Match by clusterId and ZCL sequence number.
Implementations quals that of deCONZ now.
Work in progress to have strict driver state and remove losely defined code paths. A few more enum definitions to provide better debugging.
If the configuration has a different channel, during reconfiguration update te NwkUpdateID and broadcast a ZDP channel change request to the network.
The confirm.status wasn't always parsed at the right position. Also improve logging output by providing human readable status code text.
Currently only for ConBee III debug firmware.
|
Update: The PR is now in shape and the topics above are implemented and ready for review. I've tested in my (small) setup with two routers and 3 sleeping end-devices also letting it run over a few days. Tests where done with ConBee I, II and III with latest stable firmware versions. Also made some tests to ensure that a setup handled by the former adapter doesn't get messed up by the PR. |
|
Many thanks for this! Would you also mind updating the Attention parts of https://www.zigbee2mqtt.io/guide/adapters/deconz.html ? |
Yes I'll have a look and make a PR 👍 |
commit da4f831 Author: Koen Kanters <[email protected]> Date: Sat Jun 28 10:21:08 2025 +0200 chore(master): release 4.2.1 (Koenkk#1429) commit 8a134cc Author: Manuel Pietschmann <[email protected]> Date: Sat Jun 28 10:18:47 2025 +0200 fix: deCONZ: handle race condition between APS confirm/indication timeouts (Koenkk#1432) commit 33e708d Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue Jun 24 21:47:54 2025 +0200 fix(ignore): bump the minor-patch group with 2 updates (Koenkk#1430) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> commit a4d7b19 Author: Koen Kanters <[email protected]> Date: Mon Jun 23 20:48:46 2025 +0200 fix(ignore): Migrate to Biome 2 (Koenkk#1427) commit 8668dee Author: Koen Kanters <[email protected]> Date: Sun Jun 22 22:11:33 2025 +0200 chore(master): release 4.2.0 (Koenkk#1426) commit 73a2548 Author: Nerivec <[email protected]> Date: Sun Jun 22 12:21:46 2025 +0200 feat: Support for EmberZNet 8.2.0 (EZSP v17 / v2025.6.0) (Koenkk#1428) commit c719ee4 Author: Manuel Pietschmann <[email protected]> Date: Thu Jun 19 20:17:13 2025 +0200 feat: Refactor and improve of Deconz adapter (Koenkk#1417) Co-authored-by: Koen Kanters <[email protected]> commit 99b6c97 Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue Jun 17 14:42:07 2025 +0200 fix(ignore): bump @types/node from 22.15.30 to 24.0.3 (Koenkk#1425) commit 7beb12c Author: Koen Kanters <[email protected]> Date: Sun Jun 15 13:44:18 2025 +0200 chore(master): release 4.1.2 (Koenkk#1420) commit 172d543 Author: jomders <[email protected]> Date: Sun Jun 15 19:42:40 2025 +0800 fix: Fix interview failing for HOBEIAN devices (Koenkk#1422) Co-authored-by: Koen Kanters <[email protected]> commit 29815bd Author: Koen Kanters <[email protected]> Date: Tue Jun 10 20:25:06 2025 +0200 chore: bump pnpm commit 0a9a1c7 Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue Jun 10 20:11:57 2025 +0200 fix(ignore): bump the minor-patch group with 3 updates (Koenkk#1419) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> commit 2da206d Author: Koen Kanters <[email protected]> Date: Fri Jun 6 22:10:28 2025 +0200 chore(master): release 4.1.1 (Koenkk#1415) commit f465823 Author: puddly <[email protected]> Date: Fri Jun 6 21:08:10 2025 +0100 fix: Enable serial port locking by default for ZiGate (Koenkk#1380) commit 949a08c Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon Jun 2 19:51:32 2025 +0200 fix(ignore): bump the minor-patch group with 2 updates (Koenkk#1416) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> commit 534cff5 Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Date: Sat May 31 12:46:54 2025 +0200 fix(ignore): bump @types/node from 22.15.21 to 22.15.29 in the minor-patch group (Koenkk#1414) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> commit 6ea8348 Author: Koen Kanters <[email protected]> Date: Sat May 31 12:34:25 2025 +0200 chore: update dependabot config commit 56d010a Author: Nerivec <[email protected]> Date: Thu May 29 12:45:30 2025 +0200 chore: update ZoH dep (Koenkk#1413) commit 204b7db Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon May 26 20:31:36 2025 +0200 chore(deps-dev): bump @types/node from 22.15.19 to 22.15.21 in the minor-patch group (Koenkk#1412) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> commit 73b8efa Author: Koen Kanters <[email protected]> Date: Tue May 20 21:18:46 2025 +0200 chore(master): release 4.1.0 (Koenkk#1409) commit cc889b0 Author: Koen Kanters <[email protected]> Date: Tue May 20 17:09:23 2025 +0200 feat: Add conditional fieldControl fields to genOta commands (Koenkk#1408) commit c316b58 Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon May 19 20:37:09 2025 +0200 chore(deps-dev): bump the minor-patch group with 3 updates (Koenkk#1407) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Koen Kanters <[email protected]> commit 797aeca Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon May 12 19:31:19 2025 +0200 chore(deps-dev): bump @types/node from 22.15.3 to 22.15.17 in the minor-patch group (Koenkk#1406) commit c937bb7 Author: Koen Kanters <[email protected]> Date: Sun May 11 21:28:35 2025 +0200 chore(master): release 4.0.2 (Koenkk#1404) commit bd1be3f Author: Koen Kanters <[email protected]> Date: Sun May 11 21:25:13 2025 +0200 fix(ignore): fix tests commit 35c34b2 Author: Koen Kanters <[email protected]> Date: Sun May 11 15:32:41 2025 +0200 chore(master): release 4.0.1 (Koenkk#1398) commit a82b79f Author: Koen Kanters <[email protected]> Date: Sun May 11 15:31:58 2025 +0200 fix: ZStack: add additional logging when comparing adapter state with config Koenkk#1403 commit da33fbc Author: Koen Kanters <[email protected]> Date: Wed May 7 21:18:13 2025 +0200 chore: bump node in CI to 24 commit 2205715 Author: Koen Kanters <[email protected]> Date: Mon May 5 19:52:07 2025 +0200 chore: fully switch to Dependabot commit 988f788 Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon May 5 19:41:34 2025 +0200 chore(deps-dev): bump the minor-patch group with 2 updates (Koenkk#1401) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> commit 45f6da4 Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon Apr 28 21:06:30 2025 +0200 chore(deps-dev): bump @types/node from 22.14.1 to 22.15.3 in the minor-patch group (Koenkk#1400) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> commit 0ddc009 Author: Nerivec <[email protected]> Date: Sat Apr 26 20:21:21 2025 +0200 fix: Add support for generic science-related clusters (Koenkk#1396) commit a56a776 Author: Koen Kanters <[email protected]> Date: Tue Apr 22 21:15:15 2025 +0200 chore(master): release 4.0.0 (Koenkk#1393) commit b9e86b0 Author: Koen Kanters <[email protected]> Date: Tue Apr 22 21:14:25 2025 +0200 feat!: Expose interviewState (Koenkk#1391) commit b09f8e9 Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon Apr 21 21:23:28 2025 +0200 chore(deps-dev): bump the minor-patch group with 2 updates (Koenkk#1395) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> commit f4f8535 Author: Koen Kanters <[email protected]> Date: Sat Apr 19 21:06:33 2025 +0200 chore: Biome ignore `package.json` commit bc30e84 Author: Koen Kanters <[email protected]> Date: Sat Apr 19 20:56:51 2025 +0200 fix!: Migrate to Biome (Koenkk#1387) Co-authored-by: Nerivec <[email protected]> commit d4b3eea Author: Koen Kanters <[email protected]> Date: Fri Apr 18 09:50:40 2025 +0200 chore(master): release 3.5.2 (Koenkk#1389) commit e9b6bc9 Author: Nerivec <[email protected]> Date: Fri Apr 18 00:52:06 2025 +0200 fix(ignore): Update ZoH to v0.1.11 (Koenkk#1392) commit caa3231 Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon Apr 14 22:02:01 2025 +0200 chore(deps-dev): bump typescript-eslint from 8.29.1 to 8.30.0 in the minor-patch group (Koenkk#1390) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> commit 0dd3554 Author: Koen Kanters <[email protected]> Date: Sun Apr 13 21:33:43 2025 +0200 chore: lint commit b288ab9 Author: Koen Kanters <[email protected]> Date: Sun Apr 13 21:13:02 2025 +0200 chore: enable dependabot commit 8d2da3a Author: Koen Kanters <[email protected]> Date: Sun Apr 13 20:53:26 2025 +0200 fix(ignore): update dependencies (Koenkk#1388) commit 4911b7e Author: Koen Kanters <[email protected]> Date: Sun Apr 6 21:16:38 2025 +0200 chore(master): release 3.5.1 (Koenkk#1385) commit 573641b Author: Nerivec <[email protected]> Date: Sun Apr 6 21:05:30 2025 +0200 fix: Improve Green Power processing (Koenkk#1386) commit 9e92522 Author: Koen Kanters <[email protected]> Date: Sun Apr 6 15:37:16 2025 +0200 fix(ignore): update dependencies (Koenkk#1384) commit 8f15950 Author: Koen Kanters <[email protected]> Date: Sat Apr 5 13:57:22 2025 +0200 chore(master): release 3.5.0 (Koenkk#1383) commit 5bf6b40 Author: avzasorin-sd <[email protected]> Date: Sat Apr 5 14:41:58 2025 +0300 feat: add Tunneling cluster commands (Koenkk#1381) Co-authored-by: Koen Kanters <[email protected]> commit 236da5b Author: avzasorin-sd <[email protected]> Date: Thu Apr 3 21:04:46 2025 +0300 feat: add SberDevices manufacturer code (Koenkk#1382) Co-authored-by: Koen Kanters <[email protected]> commit 38d2885 Author: CubeZ2mDeveloper <[email protected]> Date: Thu Apr 3 01:47:10 2025 +0800 fix: Support for Sonoff Dongle Max auto-discovery (Koenkk#1378)
|
@manup I was looking at the deconz code earlier. Seems there are a few unused private in the driver class. It led me to the event emitting that's only used within the same class. Would be much better for perf to remove this and just trigger the functions directly. This is generating countless unnecessary emits. zigbee-herdsman/src/adapter/deconz/driver/driver.ts Lines 182 to 184 in a7fbacf
zigbee-herdsman/src/adapter/deconz/driver/driver.ts Lines 200 to 202 in a7fbacf
zigbee-herdsman/src/adapter/deconz/driver/driver.ts Lines 257 to 259 in a7fbacf
|
|
@Nerivec Thanks for looking into this. I only have a limited understanding of how nodejs works. My goal was to use emit because the event should be processed async. Meaning that they are queued but not pile up into larger call stacks. It's a bit difficult to describe, the following toy example In C shows what I tried to do via static int tasks = 0;
void task1() { tasks |= 2; }
void task2() { tasks |= 1; }
int main() {
tasks = 1; // dummy queue event 1
for (;;) // main loop
{
if (tasks & 1) {
tasks &= ~1; // clear event 1
task1();
}
if (tasks & 2) {
tasks &= ~2; // clear event 2
task2();
}
}
}Note that neither task1 nor task2 call each other directly, they only "queue" up the events (an event here is simply a bit in |
|
When you're doing this:
might as well just do this directly: this.handleStateEvent(DriverEvent.Connected);At the very least, that would remove the whole "emit to itself". Here's a start: master...Nerivec:zigbee-herdsman:sample-deconz Also a lot of stuff appears to not be zigbee-herdsman/src/adapter/deconz/driver/driver.ts Lines 1189 to 1190 in a7fbacf
You should use the parser/writer instead of dealing with serial vs socket in-place (like in Any reason you used an interval-based You should be able to use the ASH layer of the |
This PR is a rewrite of the adapter and driver for ConBee I, II, III and RaspBee I/II and addresses following topics:
The old driver is rather optimistic and didn't handle errors in a defined manner. The rewrite is in par with the deCONZ reference implementation and handles error paths explicitly by tracking timeouts and transaction sequence numbers. Especially here the queue handling is streamlined now to never do "fire and forget" commands and having untracked Promises.
The old driver is only re-configuring some of the network parameters and doesn't send ZDP channel change commands to move a network to a new channel if the configuration was changed. The PR handles this fully within the new state machine. Further any misconfiguration gets handled same as in deCONZ reference.
Not part of this PR is Zigbee Install Code support. Although at least the ConBee II firmware supports these with a hack, we would like to add proper firmware support first before adapting this in the driver in a future PR.
Some clarification is needed on APS level ACKs, sice these can block queues of any coordinator (not just ConBee) for offline and sleeping devices. If I understand the Adapter interface right, decision of using APS ACKs is left to the adapter? But imho that should only be decided by the z2m controller since the adapter has no knowledge of wherever end-devices are sleeping or certain devices are tricky to reach in the network, or simply offline. Currently I plan to maintain a simple in memory list for devices which don't yield responses or often timeout to turn APS ACKs on for these. But it would be better if not every adapter has to implement such things. @Koenkk I'd appreciate your thoughts on this.As remark in deCONZ a device integration is made via a JSON file "Device Description File (DDF)" which also specifies when a sleeping device isawake, for example the Xiaomi devices which are "never" reachable without a button press, do in fact respond to ZCL commands once every hour then their periodic report is send to the coordinator and shortly after they do a MAC poll request to their parent to receive commands. In deCONZ we use this to schedule reconfiguration if needed, for example to configure a new sensitivity value for the vibration sensor.DONE APS ACKs are now enabled for all unicast requests.
Personal wishlist for a future Adapter interface:
(to simplify adapter development a lot 😄 )
An adapter should only provide an interface for `APS Request, Confirmation and Indication. And handling of these moved into z2m controller. The adapters shouldn't know anything about ZCL, ZDO requests or tracking their responses. With that only z2m controller needs robust handling implemented in one place which works the same for every coordinator. Imho this would reduce the code size of adapters a lot and bugs could be ironed out more broadly in one place.