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

[TC-WNCV-3.1] CurrentPositionLiftPercent100ths attribute read value mismatch error #22265

Closed
manjunath-grl opened this issue Aug 30, 2022 · 7 comments · Fixed by #22497
Closed
Assignees

Comments

@manjunath-grl
Copy link
Contributor

manjunath-grl commented Aug 30, 2022

Summary:
As per testplan of WNCV-3.1 in Step 3b: TH reads CurrentPositionLiftPercent100ths and the expected outcome value must not be 10000 ( 0x2710 ) but got 10000 when executed.
Steps needs review 3b , 3c, 3d, 3e
Client app: All clusters app
Server app: Chip-tool

Diff link: master...manjunath-grl:connectedhomeip:CurrentPositionLiftPercent100ths_Value_Mismatch

Testplan screenshot:
image

Execution logs:
WNCV_3_1.txt

@bzbarsky-apple
Copy link
Contributor

Ah, I guess this is moving toward 0, so the assertion is that it has moved at least some?

Is this failing reliably, or just some of the time @manjunath-grl ?

@manjunath-grl
Copy link
Contributor Author

@bzbarsky-apple this is failing every time when executed.

@bzbarsky-apple
Copy link
Contributor

bzbarsky-apple commented Aug 30, 2022

So the log shows that we send the UpOrOpen command (at a point when LiftPercent100ths is 10000) then we do our reads, etc, and then after all that LiftPercent100ths is set to 0. I don't see it being set to any intermediate values.

@jmeg-sfy any idea why that's happening? Note that this is all-clusters-app, not window-app.

@jmeg-sfy
Copy link
Contributor

jmeg-sfy commented Aug 31, 2022

There is only one single shot timer on all-Cluster-app to fake a motion update (w/o intermediate values)
#define FAKE_MOTION_DELAY_MS 5000 here src/app/clusters/window-covering-server/window-covering-server.cpp
it is defined to update to the final value after 5s . this is not the best way to simulate it for sure but it is easy to circumvent w/ YAML, real life product took between 30s up to > 1 minutes to fully move

@manjunath-grl keep step "3a2: DUT updates its attributes" to 2000 ms or increase to 3000 ms

@jmeg-sfy
Copy link
Contributor

@manjunath-grl #22284 i made some comments to solve this issue

@bzbarsky-apple
Copy link
Contributor

There is only one single shot timer on all-Cluster-app to fake a motion update (w/o intermediate values)

It's not clear to me that that's a legal implementation of the SHALL bits of "5.3.4.3. Position Aware" in appclusters...

@jmeg-sfy
Copy link
Contributor

Position aware just require to have at the end (current == target) position
but a real device will update its current along the way.
We can improve all-cluster-app for CI or better create a dedicated window-app later post v1.0

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

Successfully merging a pull request may close this issue.

5 participants