EZSP cleanup & fixes

[Nerivec]

I was waiting for the new release to put out this one. It's mostly cleanup, but a few changes in behavior & fixes (below at the top).

• Fixed APS options passed to sent frames based on source routing status after init (according to silabs recommendations).
• Fixed passed address for ZLL device lookup.
• Removed wait time if ZCL multicast request fails to send.
• Fixed preJoining not awaited in permitJoin.
• Added small cooldown before retry ZCL frame to endpoint.
• Increased cooldown before retry failed sendDATA at UART level to lower chances of triggering a reset.
• Added RETRY_QUEUE_SIZE=16 config IDs to set on NCP.
• Let adapter catch errors for driver-level request functions to properly cancel any waiters (and better error reporting).
• Removed nesting of functions within layers that added no real benefit (makes debugging easier).
• Removed eslint disabling for whole ezspAdapter file (and fixed the associated problems).
• Removed emit 'event' on frame in ezspAdapter (nothing to use it above, stale code?)
• Added prefix-capability to EmberEUI64 toString function, toString(true), makes things cleaner since it's used a lot.
• Added some more logging, comments & cleanups. Elevated main logic error logging to console.log to have them in regular logs.

I think I covered all the changes...

@kirovilya Let me know if you catch anything.

PS: After this is merged & available in dev branch, please report your observations in the EZSP Testing discussion.

[Koenkk]

Great progress!

@kirovilya let me know if this is OK for you

[kirovilya]

Very good! I like. But I won’t be able to check its functionality in the next few days...

[Nerivec]

@Koenkk Is the logger passed to the Adapter class viable to log to the tab/file instead of limiting everything to debug mode (off 99.9% of the time)? I couldn't find it being used much throughout herdsman, except in z-stack manager, I was wondering if it was actually hooked up upstream and could replace a plain console.log? Could use something a bit more flexible though, I think; to be able to use it in utility classes & such without passing that logger around like crazy.

[Nerivec]

Some more...

• Transferred much of the UART frame logic inside the AshFrame class (AKA hiding all the "nasty bits" 😉 ).
  • Added length checks by frame type to reject invalid frames early.
  • Added "sugar" functions for AshFrame to make UART class cleaner/easier to read.
  • Centralized AshFrame-related logic (crc, stuffing, etc) inside AshFrame (static functions).
  • Updated Parser/Writer to use above additions.
• Added more const to avoid "magic numbers" in code (extensively documented to quickly identify changes with new EZSP updates -that's why the commit looks so long...-).
• Added basic tests for UART frame parsing logic.
• Improved EZSP frame parsing using silabs' logic to identify format and problem frames.
  • EZSP frames have control bits that let the NCP alert the host of potential issues (network congestion & the likes); this uses them.
  • It also allows to drop invalid frames that could cause trouble if deserialized upstream.
• Added basic tests for EZSP frame parsing logic.
• Tried to standardize logging some more.
• Minor typing improvements for deserializing functions.

PS: I'm by no means a Jest expert, I'm sure the tests can be improved, but at least for now it's checking a few critical things. We'll definitely need some more... 😉

[Koenkk]

@Nerivec for the logger we can do the same as in https://github.com/Koenkk/zigbee-herdsman-converters/commit/6c4475c4fd13fd4b6b069c90ecf1cfef071d5f96 + https://github.com/Koenkk/zigbee2mqtt/commit/f83709d96c4a98b2c5f34a2e06fe2ea26f9cb9d5

[Nerivec]

Sounds good. I'll let you do the base changes the way you see fit, and I'll update the EZSP part once it's done. Let's do that after this is merged though, if you end up removing the logger passed around in controller/adapter in favor of the new system, it will conflict with this. I'd prefer avoiding dealing with conflicts with such a large commit 😄

[corporategoth]

@Nerivec is this PR going to address https://github.com/Koenkk/zigbee-herdsman/issues/856 ?

[Nerivec]

Changes are all EZSP/UART-level (protocols handling), the rest is general cleanup. It doesn't change the Z2M-level logic. I'll post a reply in the original thread.

[Koenkk]

Since this PR might be a bit risky, I will wait until https://github.com/Koenkk/zigbee2mqtt/issues/21162 is fixed so that I can publish a new hotfix release.

[Nerivec]

If a hotfix is planned for release, then for sure, wait. I fully expect a couple of hotfixes to have to be done after this is merged in dev. So many setups to support...

[Nerivec]

@Koenkk I'm going to table this PR for now. I've been made aware of a couple of bugs affecting the current release version (EZSP only), that need fixing (this update doesn't solve them either... just my luck). I've identified one, a race condition (I guess it's running too fast now... 😄), but the second one remains a mystery at the moment, I don't want this pushed on top (or pushing on top of this) since it doesn't fix it and it's massive. Couldn't reproduce either in my limited test env, but definitely in the affected machine, so...

I'll come back to this if that's alright with you. I'll also walk it in smaller PRs, should be safer.

EDIT: Forgot to mention. Do you mind adding @serialport/binding-mock as a dev dep in herdsman in your next commit? I've been using it as mock serial to test EZSP locally, it avoids a lot of grunt work mocking everything by hand.

[Koenkk]

Do you mind adding @serialport/binding-mock as a dev dep in herdsman in your next commit?

No problem, would you like me to add it or will you add it in the next pr?

[Nerivec]

You can go ahead and do it. I'm still trying to fix that damn bug. 😅

[Koenkk]

Done!