search

project-chip / connectedhomeip

Matter (formerly Project CHIP) creates more connections between more objects, simplifying development for manufacturers and increasing compatibility for consumers, guided by the Connectivity Standards Alliance.
https://buildwithmatter.com
Apache License 2.0
7.48k stars 2.01k forks source link

[BUG] Commissioning window timeout can be set to the value incompatible with the spec #35505

Closed kkasperczyk-no closed 1 month ago

kkasperczyk-no commented 1 month ago

Reproduction steps

The CommissioningWindorManager::MaxCommissioningTimeout is used to determine the maximum allowed commissioning timeout used in the CommissioningWindowManager::OpenCommissioningWindow method. This method was modified btw of adding BLE extended announcement feature: https://github.com/project-chip/connectedhomeip/commit/ca17912497a7eb8db22d42ddbe55f36370214d8c#diff-d6f2798a18ed8d25dbb88df7816ca0584e4d6869dc2e4618dc74ee494db7f314R64 and if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING is enabled, the returned value is equal to 48 hours.

This makes the implementation return timeout equal to 48 hours, even for the commissioned device, and the spec does not allow the window to exceed 900 s in such scenario.

It makes the CADMIN 1.21 i 1.22 certification tests fail.

Bug prevalence

Always

GitHub hash of the SDK that was being used

83d345e18cbff6943e4925a5936a3edc72ab076b (master)

Platform

core

Platform Version(s)

v1.3.0

Anything else?

No response

bzbarsky-apple commented 1 month ago

For the IP transport, the spec does not allow the window to exceed 900 s.

Yes, it does, for uncommissioned devices already on the IP network. The "Extended Announcement" feature in the spec is transport-independent.

kkasperczyk-no commented 1 month ago

@bzbarsky-apple thank you for the clarification. We must have had some misunderstanding of this feature in the verification area. Maybe because the spec in the past mentioned about "unnecessary pollution of the crowded 2.4 GHz wireless spectrum, especially with BLE" and in our heads it turned out into BLE related :)

kkasperczyk-no commented 1 month ago

Or it's the CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING config name in the SDK that suggests it's related to BLE only.