-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
app: Fix clusters delegates table check #26922
app: Fix clusters delegates table check #26922
Conversation
PR #26922: Size comparison from cfb9994 to e7c9d26 Increases (3 builds for bl602, bl702)
Decreases (1 build for bl702)
Full report (6 builds for bl602, bl702, linux)
|
e7c9d26
to
3cfd9e5
Compare
PR #26922: Size comparison from 22de5a0 to 3cfd9e5 Increases above 0.2%:
Increases (17 builds for bl702, cc32xx, linux, psoc6, telink)
Decreases (12 builds for bl602, bl702, cc13x4_26x4, cc32xx, efr32, esp32, k32w, psoc6, telink)
Full report (64 builds for bl602, bl702, cc13x2_26x2, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, telink)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not going to help on its own, because for dynamic endpoints emberAfFindClusterServerEndpointIndex will return garbage.
You need to use emberAfGetClusterServerEndpointIndex
instead at all these callsites...
@bzbarsky-apple Replaced all the emberFindClusterServerEndpointIndex now |
…erverEndpointIndex
9efbcbf
to
cccb6ff
Compare
PR #26922: Size comparison from 2377844 to cccb6ff Increases above 0.2%:
Increases (17 builds for bl702, cc32xx, cyw30739, efr32, linux, psoc6, telink)
Decreases (42 builds for bl602, bl702, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
Full report (58 builds for bl602, bl702, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
PR #26922: Size comparison from 43cfe21 to 28b7c95 Increases above 0.2%:
Increases (15 builds for bl702, cc32xx, efr32, linux, psoc6, telink)
Decreases (42 builds for bl602, bl702, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
Full report (58 builds for bl602, bl702, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was actually doing this too.
Can you remove emberAfFindClusterServerEndpointIndex
altogether?
I believe the only place left that uses it is in the tv-app and in a mock for Darwin tests.
after that there is no more use for emberAfFindClusterServerEndpointIndex
Since this PR already pass CI and all, ill just merge it and I can do the deletion in another PR
We should use table size instead of the fixed cluster endpoint count for the check for the cluster servers delegate.