fix(modify): Repair the issue of channel updating

[CubeZ2mDeveloper]

When I update channels in zigbee2mqtt and restart, I find that some devices fail to be controlled. I discovered that when changing channels, the value of the nwkUpdateId field remains at 0.

I referred to the ZigBee specification and found the instructions regarding the modification of the nwkUpdateId field when changing channels:

"The network manager should broadcast a Mgmt_NWK_Update_req notifying devices of the new channel. The broadcast shall be to all devices with RxOnWhenIdle equal to TRUE. The network manager is responsible for incrementing the nwkUpdateId parameter from the NIB and including it in the Mgmt_NWK_Update_req. The network manager shall set a timer based on the value of apsChannelTimer upon issue of a Mgmt_NWK_Update_req that changes channels and shall not issue another such command until this timer expires. However, during this period, the network manager can complete the above analysis. However, instead of changing channels, the network manager would report to the local application using Mgmt_NWK_Update_notify and the application can force a channel change using the Mgmt_NWK_Update_req."

[Koenkk]

I don't think making a new backup is necessary here? Can't we get the value from the existing backup?

[Nerivec]

Agreed.
Probably the property should be added to the adapter type NetworkParameters, since it's part of that, and then can be used throughout controller without issue (cached on start).
Remains to see if all adapters provide it though.

[CubeZ2mDeveloper]

Okay, I will try to change it to the implementation method you suggested.

[Nerivec]

Was this tested against the described problem?
Some devices are expected to not switch (bad design), some devices require activation to wake them up, then they switch after a few seconds (as described in docs).
I'd expect if this was required, all devices would fail to switch, which they do not (I've not found a device, even crappy Tuya, that doesn't switch with current implementation, on my end, several users also tested it and reported no issue).

From what I can tell, that quoted paragraph of the spec is for Upon receipt of a Mgmt_NWK_Unsolicited_Enhanced_Update_notify message, which this is not.

In the paragraph about the reception of nwk update req by a device, that value doesn't seem to matter.

3) If the ScanDuration parameter is equal to 0xfe, the message is a command to change channels. The device SHALL do the following.
  a) If the nwkNextChannelChange value in the NIB is non-zero, do the following. Compare the channel to change received over the air to the value in the NIB. If the values do not match, do the following:
    i) Follow the Error Response procedure setting the status to NOT_AUTHORIZED.
    ii) The request SHALL be dropped and no more processing SHALL take place.
  b) If there is more than 1 channel indicated in the ScanChannels bitmap (if the message is Mgmt_NWK_Update_req) or in the ScanChannelsListStructure (if the message is Mgmt_NWK_Enhanced_Update_req), then this is an invalid request. Do the following:
    i) Follow the Error Response procedure setting the status to INV_REQUESTTYPE.
    ii) The request SHALL be dropped and no more processing SHALL take place.
  c) The receiving device SHALL determine if the channel is one within the range of all supported channels.
    i) Examine the SupportedChannels element for each entry in the nwkMacInterfaceTable, and determine if there is a match within the received ScanChannels bitmap or ScanChannelsListStructure.
    ii) If no match is found, do the following:
      (1) Follow the Error Response procedure setting the status to INV_REQUESTTYPE.
      (2) The request SHALL be dropped and no more processing SHALL take place.
    iii) If a match is found, perform a channel change.
      (1) Execute a MLME-SET.request for the PIB value phyCurrentPage.
      (2) Execute a MLME-SET.request for the PIB value phyCurrentChannel.
      (3) No further processing SHALL be done.

As far as I can tell, the case where the nwkUpdateId would be used would be if a router decides to rejoin (for some reason), and it finds the same network on at least two different nodes, then it should pick the node with the higher nwkUpdateId (if any). But that scenario should not matter for a channel change, since routers receive the broadcast, and switch immediately. Leaving any offline router eventually powered back on, with only the new network available to rejoin.

The spec is a bit lacking on that nwkUpdateId, which I noted in the implementation a while back:

zigbee-herdsman/src/zspec/zdo/buffaloZdo.ts

Lines 1199 to 1204 in 1c8d886

	// TODO: What does "This value is set by the Network Channel Manager prior to sending the message." mean exactly??
	// (isn't used/mentioned in EmberZNet, confirmed working if not set at all for channel change)
	// for now, allow to bypass with undefined, otherwise should throw if undefined and duration passes below conditions (see NwkEnhancedUpdateRequest)
	if (nwkUpdateId !== undefined && (duration === 0xfe || duration === 0xff)) {
	this.writeUInt8(nwkUpdateId);
	}

Short version: I don't expect it would solve the problem at hand?
I'm also a little worried about changing this considering the various stacks involved, because it was tested rather extensively during implementation, and after by several users and no particular issue was found that couldn't be attributed to devices being "too cheap". Hopefully, at worse, it does nothing... but I half expect that most devices just don't use the value at all...

[Nerivec]

const mockAdapterBackup = vi.fn(() => Promise.resolve(mockDummyBackup));

should provide more accurate typing/testing (and fix typo).
(A lot of others also need refactoring, but might as well add new ones properly.)

[CubeZ2mDeveloper]

I added the same device to Home Assistant, then changed the channel. I found that it can be controlled normally in Home Assistant, and I also discovered that Home Assistant will increase the value of nwkUpdateID.

[CubeZ2mDeveloper]

Hi, @Koenkk.
I would like to ask about the progress of this PR.
We currently have a project that needs to use this feature in zigbee2mqtt.
I can send the problematic device to you if necessary.

[Nerivec]

Was this tested in Z2M against the initial problem?
And, if it does fix it, we need to implement this for all adapters (state management for nwkUpdateId, make sure it's properly reported by adapters, etc.).

[CubeZ2mDeveloper]

I have already tested the code on z2m, my device can be controlled normally after the fix.
Currently, the NwkUpdateId state has been maintained on both the Ember and EZSP adapters, but I am not familiar with the protocol of other adapters, so it has not been implemented yet.

[CubeZ2mDeveloper]

After pulling the latest zigbee2mqtt code from the master branch, I started z2m using a Dongle that supports the zstack protocol stack and modified the channel. After changing the channel, the device could be controlled normally on zstack, and the nwkUpdateId incremented by 1. However, when using an Ember Dongle, the device control failed after changing the channel, and the nwkUpdateId remained unchanged, still at 0.

Since our users are currently encountering related issues, this PR might help resolve the problem. Could you let me know about your plans for this PR? For example, are there any further changes needed, or do you have an estimated timeline for merging it? If there’s anything we can do to assist, such as providing devices for testing, please feel free to let us know. Looking forward to your reply, thank you!

[Nerivec]

The increment should be done in changeChannel so it only happens when the channel is actually changed (re-get getNetworkParameters in the function instead of passing a param, it's cached anyway).
Also might need to deal with sendZdo potentially failing (I think that will stop Z2M startup, so should be fine, but needs to be verified).

[Nerivec]

This should not be optional, and support should added be in all drivers.

---

The change should also be tested at least in zstack and ember (the two officially supporting change channel) with at least a few devices that worked before, to make sure they still work after this change. Since it doesn't appear to be required for a change channel (based on spec & previous testing with many devices), it might have negative impact on other aspects (like some devices not expecting a changed nwkUpdateID when receiving a change channel request, and ignoring the request entirely).
In that regard, can you confirm which devices (models) are having the issue in the first place?