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.52k stars 2.02k forks source link

[pre-SVE] TC-DRLK-2.9 Credential Response command with responses don't match #19990

Closed AlvinHsiao closed 2 years ago

AlvinHsiao commented 2 years ago

Problem

expected behavior vs actual behavior

Step 6 expects response as OCCUPIED, but CHIP Tool Verification Steps shows status: 133. Step 7 expects response as OCCUPIED, but CHIP Tool Verification Steps shows status: 2. Step 8 expects response as OCCUPIED, but CHIP Tool Verification Steps shows status: 133. Step 13 expects response as SUCCESS, but CHIP Tool Verification Steps shows status: 2.

system configuration : Chip-Tool: RPI DUT used : Infineon CYW30739 app use: Lock-app chip-tool and DUT commit id : https://github.com/project-chip/connectedhomeip/commit/1e3f12041013dd71a53d035dbf2f6d142fc06c4e (pre-sve SHA)

woody-apple commented 2 years ago

SDK Review: Are you able to take a look @Morozov-5F ?

Morozov-5F commented 2 years ago

How up-to-date are the YAML tests in the SDK? I can see that the all the enabled DRLK tests (namely, Test_TC_DRLK_2_2, Test_TC_DRLK_2_3, Test_TC_DRLK_2_4, Test_TC_DRLK_2_5, Test_TC_DRLK_2_7 and Test_TC_DRLK_2_9) are passing against the Linux lock-app.

woody-apple commented 2 years ago

Cert Blocker Review: Thanks @Morozov-5F, unassigning you. Ian to follow up from Infineon.

woody-apple commented 2 years ago

Door Lock Review: We need to fix the YAML here. The main issue here is resolved: https://github.com/project-chip/connectedhomeip/pull/21319

Filing a new issue to fix the YAML.

bzbarsky-apple commented 2 years ago

Step 7 expects response as OCCUPIED, but CHIP Tool Verification Steps shows status: 2.

I wish the steps were numbered in the YAML, but looking at the comments that mention this issue:

  1. Test adds credential of type 1 at index 1, sets to 123456.
  2. Test adds credential of type 1 at index 2, sets to 4321
  3. Test tries to add credential of type 1 at index 2 with value 123456
  4. Test tries to add credential of type 1 at index 1 with value 123456

I believe steps 3 and 4 are the ones at issue here? The comments in the YAML seems to be mis-placed...

Anyway, "5.2.4.41.1. Status" in the spec does not define which of the error codes to pick if more than one applies; it just says it SHALL be one of them, as far as I can tell. @DanM-AssaAbloy

bzbarsky-apple commented 2 years ago

Reopening for now so we don't lose track of this.

sethunk commented 2 years ago

In TC-DRLK-2.9, 1.While executing Set credential Command, User Index is provided as 1 and in the SetCredentialResponse the User Index is null but the expected value is 1. 2.While executing Set credential Command, User Index is provided as null and in the SetCredentialResponse the User Index is 2 but the expected value is null. When executing the Set Credential Command, User Index is provided as null by changing the Credential Index as 2, the SetCredentialResponse the User Index is null.

Steps to reproduce ./chip-tool doorlock set-user 0 1 xxx 6452 1 0 0 1 1 --timedInteractionTimeoutMs 1000

[1659979602.648964][2424:2429] CHIP:DMG: StatusIB = [1659979602.649067][2424:2429] CHIP:DMG: { [1659979602.649153][2424:2429] CHIP:DMG: status = 0x00 (SUCCESS), [1659979602.649257][2424:2429] CHIP:DMG: }

./chip-tool doorlock set-credential 0 '{ "credentialType" : 1 , "credentialIndex" : 1 }' 123456 null null null 1 1 --timedInteractionTimeoutMs 1000

[1659979614.974351][2431:2436] CHIP:DMG: Received Command Response Data, Endpoint=1 Cluster=0x0000_0101 Command=0x0000_0023 [1659979614.974450][2431:2436] CHIP:TOO: Endpoint: 1 Cluster: 0x0000_0101 Command 0x0000_0023 [1659979614.974566][2431:2436] CHIP:TOO: SetCredentialResponse: { [1659979614.974643][2431:2436] CHIP:TOO: status: 0 [1659979614.974700][2431:2436] CHIP:TOO: userIndex: 2 [1659979614.974757][2431:2436] CHIP:TOO: nextCredentialIndex: 2 [1659979614.974812][2431:2436] CHIP:TOO: }

./chip-tool doorlock set-credential 0 '{ "credentialType" : 1 , "credentialIndex" : 2 }' 123456 null null null 1 1 --timedInteractionTimeoutMs 1000

[1659979635.153480][2439:2444] CHIP:DMG: Received Command Response Data, Endpoint=1 Cluster=0x0000_0101 Command=0x0000_0023 [1659979635.153580][2439:2444] CHIP:TOO: Endpoint: 1 Cluster: 0x0000_0101 Command 0x0000_0023 [1659979635.153699][2439:2444] CHIP:TOO: SetCredentialResponse: { [1659979635.153776][2439:2444] CHIP:TOO: status: 2 [1659979635.153834][2439:2444] CHIP:TOO: userIndex: null [1659979635.153891][2439:2444] CHIP:TOO: nextCredentialIndex: 3 [1659979635.153948][2439:2444] CHIP:TOO: }

Logs doorlock.txt

@bzbarsky-apple Please confirm whether it is a SDK issue or test plan issue?

bzbarsky-apple commented 2 years ago

1.While executing Set credential Command, User Index is provided as 1 and in the SetCredentialResponse the User Index is null but the expected value is 1.

Why is the expected value here 1? Note https://github.com/CHIP-Specifications/connectedhomeip-spec/issues/5507 and https://github.com/CHIP-Specifications/connectedhomeip-spec/pull/5528 addressing that; after that the expected value here is null, I believe.

2.While executing Set credential Command, User Index is provided as null and in the SetCredentialResponse the User Index is 2 but the expected value is null.

Given the commands being run above, that's an Add operation on the credential. For an Add, this is just wrong in the test plan: the spec has been very clear that for an Add if the provided user index is null in the response the user index is the index of the user that was created.

sethunk commented 2 years ago

@bzbarsky-apple Thank you, I accept that it is a test plan issue and based on the spec issue https://github.com/CHIP-Specifications/connectedhomeip-spec/issues/5507 and https://github.com/CHIP-Specifications/connectedhomeip-spec/pull/5528, the test plan should be updated.

franck-apple commented 2 years ago

Per the above comment, closing this issue as invalid test, no SDK work needed.

franck-apple commented 2 years ago

Test plan issue tracked here: https://github.com/CHIP-Specifications/chip-test-plans/issues/2033

bzbarsky-apple commented 2 years ago

@franck-apple That's a different issue....