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

[Proposal] Removal of "forever" Permit Join #940

Open
Nerivec opened this issue Feb 27, 2024 · 11 comments
Open

[Proposal] Removal of "forever" Permit Join #940

Nerivec opened this issue Feb 27, 2024 · 11 comments

Comments

@Nerivec
Copy link
Collaborator

Nerivec commented Feb 27, 2024

After observing behaviors while testing ember and seeing reports here and there about this, I think removing the possibility to open the network "forever" via internal workarounds would make sense.

  1. It doesn't seem to work well in general from Discord reports (compared to triggering as needed with the Permit Join button).
  2. Some stacks now enforce no "forever", which messes with the interval refresh triggered by Z2M, basically breaking the "assumed state".
  3. It prevents state validation, because of the interval refresh that won't actually do anything if the network is already open (at least in EmberZNet)
  4. It's bad for security...

The TODOs on this would appear to be:

  • Remove the config
  • Cleanup the config validations
  • Remove the interval refresh logic to only have:
    • Open for 254 seconds (0xFF can be a special value) on button click.
    • Close on button click if within 254 seconds of opening.
    • Optionally use stack-level function/prop to determine if network is currently opened instead of assuming or using an internal timer (not sure if all stacks support this)
  • Remove/fix related tests

Related code (probably not all...):

public async permitJoinInternal(

https://github.com/Koenkk/zigbee2mqtt/blob/f04ade6d764518bd8228eb32a15e96cd4e3fbde6/data/configuration.yaml#L5
https://github.com/Koenkk/zigbee2mqtt/blob/f04ade6d764518bd8228eb32a15e96cd4e3fbde6/lib/types/types.d.ts#L122
https://github.com/Koenkk/zigbee2mqtt/blob/f04ade6d764518bd8228eb32a15e96cd4e3fbde6/lib/util/settings.schema.json#L48
https://github.com/Koenkk/zigbee2mqtt/blob/f04ade6d764518bd8228eb32a15e96cd4e3fbde6/lib/util/settings.ts#L39
https://github.com/Koenkk/zigbee2mqtt/blob/f04ade6d764518bd8228eb32a15e96cd4e3fbde6/lib/controller.ts#L139
https://github.com/Koenkk/zigbee2mqtt/blob/f04ade6d764518bd8228eb32a15e96cd4e3fbde6/lib/extension/bridge.ts#L161

@Koenkk
Copy link
Owner

Koenkk commented Feb 27, 2024

There are some special cases where permit join is required to be permanently enabled, if I remember correctly for some green power devices. As a simple first step, let's just remove the permit_join option from the Z2M configuration.yaml. Then we can see if any setups break.

@Nerivec
Copy link
Collaborator Author

Nerivec commented Feb 27, 2024

Just putting false as override would be a good first step for sure (just ignore the config at first). Better off waiting until after the 1st to implement any of this; just thought I'd get the ball rolling on the topic 😉

There are some special cases where permit join is required to be permanently enabled, if I remember correctly for some green power devices.

Is it the network that needs to be open, or the commissioning mode that needs refreshing?
I guess that refresh logic could be moved to GP if it's the second case (I suppose a separate button would be ideal in that scenario, kind of like Touchlink... make it clear what's what).
@dduransseau @chris-1243 Sorry for the ping, again, but I guess you've gained the GP-go-to status 😁

@chris-1243
Copy link

For the PTM215Z/216Z, there is no need to have the permit join always on, on my side at least. I do open the network for the pairing and close it when done.

In configuration.yaml permit join is set to false for me.

@dduransseau
Copy link
Contributor

That's an interesting case, I have a big doubt that permit_joint status (true/false) make any difference to allow GP device to join or not. Sometimes I figure out that GP device joined the "wrong" instance of Z2M when I performed some test. For example during ember test I figure out that GP switch joined the "main" instance and not the "lab" even if "main" was not configured to allow join.
I'll perform some tests with only one instance up and running to confirm this.

@Nerivec
Copy link
Collaborator Author

Nerivec commented Feb 27, 2024

@chris-1243 Can you confirm this is the case in zStack too? I believe you have access to a network using it.

@dduransseau If that's confirmed, my guess would be that the coordinator is always in commissioning mode for GP (no matter what Z2M says), but that would likely be EmberZNet-specific, and coordinator-only.

@dduransseau
Copy link
Contributor

dduransseau commented Feb 28, 2024

Nope, I figure out this behavior on deconz with conbee II for the first time. I'm not an expert of herdsman, since GP provisioning do not use same type of frame, would it be possible that GP frame bypass the allow_join filter ?

EDIT: I just performed the test to remove a GP device from the test instance (ember) and the main one (that use conbee II), when I execute the the commissioning sequence on the GP switch, it appear on both instance and actions are correctly handled by both instance.

@chris-1243
Copy link

chris-1243 commented Feb 28, 2024

@Nerivec I need to check if this behaviour occurs between my main network (zstack) and test (ember).

As I have both PTM215/216, I will try with both of them. As far as I remember, I have not seen such behaviour.

My two networks are not on the same channel and do not share the same network keys, pan id and extended id. All those values are different.

As you have to choose your network channel when pairing, it sounds strange for me to see the same device connected to two differents networks.

EDIT: If adapters are not on the same channel, nothing happens. That make sense. If I pair a ZGP device in zstack (channel 20), nothing is shown in ember (channel 25). The other way around is the same.

Then if both adapters are on the same channel, zstack is not impacted as it needs an open router (Hue for PTM216Z, Hue or any brand able to translate like IKEA, Sunricher for PTM215Z). ember, the device pairs even if permit join is not active. Then, if you completely reset the device (all buttons pressed for ~10s) and you try to pair it again:

  • if permit join : false (via the frontend) : nothing happens (I tried 3 times in a row)
  • if permit join : true via the frontend) : the PTM216Z pairs following the procedure.

It seems ember always reacts to the commissioning frame sent by the PTM215Z/216Z even if the permit join option is turned off

@dduransseau
Copy link
Contributor

In my scenario both network are on the same channel but different network and pan key.

@Nerivec
Copy link
Collaborator Author

Nerivec commented Feb 28, 2024

Since the current "Permit join" executes 2 actions (1- open the network, 2- turn on GP commissioning mode), the steps are a bit blurred as to what GP actually needs to pair/pair after reset.
I think there is a need for separation here. If only to ensure proper logic, because it seems devices are currently not following open/close as directed/expected by Z2M for GP stuff.

@dduransseau
Copy link
Contributor

I just found this issue that seems related

@MattWestb
Copy link

In my scenario both network are on the same channel but different network and pan key.

Make sure also have different Extended Pan-ID or you can getting strange problems then paring and network managements commands in both networks.

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

No branches or pull requests

5 participants