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

When user does not exist, week day and year day schedules return NOT_FOUND, which is not allowed per spec. #19622

Closed
Ashwinigrl opened this issue Jun 15, 2022 · 17 comments · Fixed by #19813
Assignees

Comments

@Ashwinigrl
Copy link

Problem -when we try to send Set Week Day Schedule, Get Week Day Schedule, Clear Week Day Schedule command and Set Year Day Schedule, Get Year Day Schedule, Clear Year Day Schedule command we are getting (NOT_FOUND)

sequence of steps

Set Week Day Schedule =./chip-tool doorlock set-week-day-schedule 2 1 2 15 45 16 55 1 1 (Response=NOT_FOUND)
Get Week Day Schedule =./chip-tool doorlock get-week-day-schedule 2 1 1 1 (Response=NOT_FOUND)
Clear Week Day Schedule =./chip-tool doorlock clear-week-day-schedule 2 1 1 1 (Response=NOT_FOUND)

Set Year Day Schedule =./chip-tool doorlock set-year-day-schedule 1 1 960 1980 1 1 (Response=NOT_FOUND)
Get Year Day Schedule =./chip-tool doorlock get-year-day-schedule 1 1 1 1 (Response=NOT_FOUND)
Clear Year Day Schedule =./chip-tool doorlock clear-year-day-schedule 1 1 1 1 ( (Response=NOT_FOUND)

Expected Behavior -
DOOR LOCK Logs.txt
status should be success 0x00 (Success)

Additional information

1654673540.052851][3218:3223] CHIP:TOO: AcceptedCommandList: 18 entries
[1654673540.052894][3218:3223] CHIP:TOO: [1]: 0
[1654673540.052929][3218:3223] CHIP:TOO: [2]: 1
[1654673540.052963][3218:3223] CHIP:TOO: [3]: 3
[1654673540.052997][3218:3223] CHIP:TOO: [4]: 11
[1654673540.053031][3218:3223] CHIP:TOO: [5]: 12
[1654673540.053064][3218:3223] CHIP:TOO: [6]: 13

[1654673540.053097][3218:3223] CHIP:TOO: [7]: 14
[1654673540.053129][3218:3223] CHIP:TOO: [8]: 15
[1654673540.053162][3218:3223] CHIP:TOO: [9]: 16
[1654673540.053195][3218:3223] CHIP:TOO: [10]: 17
[1654673540.053228][3218:3223] CHIP:TOO: [11]: 18
[1654673540.053262][3218:3223] CHIP:TOO: [12]: 19
[1654673540.053294][3218:3223] CHIP:TOO: [13]: 26
[1654673540.053327][3218:3223] CHIP:TOO: [14]: 27
[1654673540.053360][3218:3223] CHIP:TOO: [15]: 29
[1654673540.053392][3218:3223] CHIP:TOO: [16]: 34
[1654673540.053425][3218:3223] CHIP:TOO: [17]: 36
[1654673540.053458][3218:3223] CHIP:TOO: [18]: 38

App used - lock app
Platform - Chip-tool - RPI-4, 8GB RAM
DUT - RPI-4, 8GB RAM
Network - Ble-wifi
Commit - 9493d7b

@bzbarsky-apple
Copy link
Contributor

Set Week Day Schedule =./chip-tool doorlock set-week-day-schedule 2 1 2 15 45 16 55 1 1 (Response=NOT_FOUND)

@Ashwinigrl Is there an actual user at user index 1? What commands were run to set up the user before running these schedule commands?

@bzbarsky-apple
Copy link
Contributor

Filed https://github.com/CHIP-Specifications/connectedhomeip-spec/issues/5364 on the lack of spec clarity here, because NOT_FOUND is definitely not a valid response to "Set Week Day Schedule" per spec as things stand.

@sumaky
Copy link
Contributor

sumaky commented Jun 16, 2022

Set Week Day Schedule =./chip-tool doorlock set-week-day-schedule 2 1 2 15 45 16 55 1 1 (Response=NOT_FOUND)

@Ashwinigrl Is there an actual user at user index 1? What commands were run to set up the user before running these schedule commands?

@bzbarsky-apple This is an issue with test plan as well.
https://github.com/CHIP-Specifications/chip-test-plans/issues/1537

@bzbarsky-apple
Copy link
Contributor

@Morozov-5F It looks like the spec will not change here, so we can't return NOT_FOUND in the SDK. Need to return one of the spec-allowed values.

@Morozov-5F
Copy link
Contributor

@bzbarsky-apple but this case is not covered by the spec. Should I return generic error in that case?

@bzbarsky-apple
Copy link
Contributor

I think generic FAILURE may be our best option here, given what the spec says right now.... Worth checking with Dan Matosian how he expects this to work?

@woody-apple
Copy link
Contributor

Cert Blocker Review: Making this a cert blocker, we should do something reasonable here in the SDK.

@cjandhyala
Copy link
Contributor

This test is Passing with SDK commit ID c60233d. closing the issue

@bzbarsky-apple
Copy link
Contributor

I don't see how this can possibly be passing, given that the SDK does not implement the spec as far as I know...

@Survensa
Copy link

Survensa commented Jun 29, 2022

@bzbarsky-apple

Re-verified with latest SHA

  • Commit used : 81c7f2a

  • Chip-tool : RPI

  • DUT : Door lock app

  • Network : IP (ethernet)

Execution LOG :

  1. Week day shedule belongs to TC-DRLK-2.5 test case : TC-DRLK-2.5 LOG
  2. Year day shedule belongs to TC-DRLK-2.7 test case : TC-DRLK-2.7 LOG

@bzbarsky-apple
Copy link
Contributor

@Survensa Is that running the updated test plan, or the old one? The old one was broken, as far as I can tell, but highlighted an SDK issue. This is tracking that SDK issue.

The log show a set-user command, so that looks like the updated test plan.

@bzbarsky-apple bzbarsky-apple changed the title Doorlock Set Week Day Schedule, Get Week Day Schedule,Clear Week Day Schedule command and Set Year Day Schedule, Get Year Day Schedule, Clear Year Day Schedule command return (NOT_FOUND) When used does not exist, Doorlock Set Week Day Schedule, Get Week Day Schedule,Clear Week Day Schedule command and Set Year Day Schedule, Get Year Day Schedule, Clear Year Day Schedule command return (NOT_FOUND) Jun 29, 2022
@bzbarsky-apple bzbarsky-apple changed the title When used does not exist, Doorlock Set Week Day Schedule, Get Week Day Schedule,Clear Week Day Schedule command and Set Year Day Schedule, Get Year Day Schedule, Clear Year Day Schedule command return (NOT_FOUND) When user does not exist, Doorlock Set Week Day Schedule, Get Week Day Schedule,Clear Week Day Schedule command and Set Year Day Schedule, Get Year Day Schedule, Clear Year Day Schedule command return (NOT_FOUND) Jun 29, 2022
@bzbarsky-apple
Copy link
Contributor

@Survensa I updated the summary to make it clear what the issue is about.

@mykrupp
Copy link
Contributor

mykrupp commented Jun 30, 2022

The test spec is not very clear. Anything that involves setting credentials, or holidays requires setting a user first.

Some test cases state something like: "Assume everything is set up properly to be able to set credentials and schedules". But these should explicitly mention that a user needs to be first, and setting a user needs to be added to the test plan in some cases.

@sumaky
Copy link
Contributor

sumaky commented Jun 30, 2022

@mykrupp This issue is mentioning that the steps are executed when there is no user setup
The command returns NOT_FOUND. It means user not found. So this is not an issue
@bzbarsky-apple Please close this issue

@mykrupp
Copy link
Contributor

mykrupp commented Jun 30, 2022

Ah I understand, thanks for clarifying

@cjandhyala
Copy link
Contributor

closing the issue per above comments

@bzbarsky-apple
Copy link
Contributor

Please stop closing this issue. NOT_FOUND is not a valid return per spec here. That's what the issue is about.

@bzbarsky-apple bzbarsky-apple reopened this Jul 1, 2022
@bzbarsky-apple bzbarsky-apple changed the title When user does not exist, Doorlock Set Week Day Schedule, Get Week Day Schedule,Clear Week Day Schedule command and Set Year Day Schedule, Get Year Day Schedule, Clear Year Day Schedule command return (NOT_FOUND) When user does not exist, week day and year day schedules return NOT_FOUND, which is not allowed per spec. Jul 1, 2022
Morozov-5F added a commit to Morozov-5F/connectedhomeip that referenced this issue Jul 6, 2022
Morozov-5F added a commit to Morozov-5F/connectedhomeip that referenced this issue Jul 6, 2022
Morozov-5F added a commit to Morozov-5F/connectedhomeip that referenced this issue Jul 7, 2022
selissia pushed a commit that referenced this issue Jul 7, 2022
* [#19622] Return generic failure instead of not found for schedule operations in the door lock

* [#19563] Check enum ranges for Door Lock commands
- Refactor the cluster, so it is consistent in way of handling commands

* Refactoring of the door lock cluster: move credential error checking into a single function, fix typos

* Fix tests for Door Lock cluster and run them against the lock app

* Update auto-generated files
github-actions bot pushed a commit that referenced this issue Jul 7, 2022
* [#19622] Return generic failure instead of not found for schedule operations in the door lock

* [#19563] Check enum ranges for Door Lock commands
- Refactor the cluster, so it is consistent in way of handling commands

* Refactoring of the door lock cluster: move credential error checking into a single function, fix typos

* Fix tests for Door Lock cluster and run them against the lock app

* Update auto-generated files
andy31415 pushed a commit that referenced this issue Jul 7, 2022
* [#19622] Return generic failure instead of not found for schedule operations in the door lock

* [#19563] Check enum ranges for Door Lock commands
- Refactor the cluster, so it is consistent in way of handling commands

* Refactoring of the door lock cluster: move credential error checking into a single function, fix typos

* Fix tests for Door Lock cluster and run them against the lock app

* Update auto-generated files

Co-authored-by: Evgeniy Morozov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants