-
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
[ESP32] Added support of door-lock for esp32 #24169
[ESP32] Added support of door-lock for esp32 #24169
Conversation
PR #24169: Size comparison from e101730 to f21434d Increases (6 builds for bl602, cc13x2_26x2, nrfconnect, psoc6, qpg, telink)
Decreases (5 builds for cc13x2_26x2, esp32, telink)
Full report (40 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, mbed, nrfconnect, psoc6, qpg, telink)
|
I remember that STL is used in Linux door-lock code and STL is not acceptable in embedded code. Could you find a better way to add the door-lock support for ESP32? Also ESP32 all-clusters-app uses the Linux door-lock code here. |
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.
@jadhavrohit924 Requesting changes to validate the comment from @wqx6
f21434d
to
910cd17
Compare
PR #24169: Size comparison from 19c8ea4 to 910cd17 Increases (12 builds for bl602, bl702, cc13x2_26x2, esp32, nrfconnect, psoc6, telink)
Full report (53 builds for bl602, bl702, cc13x2_26x2, 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.
Can the ESP32 all-clusters-app example use the lock manager here instead of the lock manager in Linux lock example?
166baea
to
24f88d7
Compare
PR #24169: Size comparison from e766102 to 24f88d7 Increases above 0.2%:
Increases (5 builds for bl702, esp32, telink)
Decreases (7 builds for bl602, esp32, k32w, psoc6)
Full report (53 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
door-lock code to common
24f88d7
to
058244b
Compare
@bzbarsky-apple @wqx6 @dhrishi Please review. |
PR #24169: Size comparison from b4c2827 to 058244b Increases above 0.2%:
Increases (3 builds for esp32, telink)
Decreases (13 builds for bl602, bl702, esp32, nrfconnect, psoc6, telink)
Full report (54 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
typedef void (*Callback_fn_completed)(Action_t); | ||
void SetCallbacks(Callback_fn_initiated aActionInitiated_CB, Callback_fn_completed aActionCompleted_CB); | ||
|
||
bool Lock(chip::EndpointId endpointId, const Optional<chip::ByteSpan> & pin, DlOperationError & err); |
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.
How does this build, given that it was renamed earlier in #24358 ?
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.
It's a merge conflict. The last time CI ran on this PR was before #24358 merged (3 days ago vs yesterday)...
We need to fix this new code ASAP to work with the right names; right now this is going to block all PRs from landing.
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.
It's blocking me from merging master into my feature branch. Let me see about reverting this one.
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.
Revert: #24531
This reverts commit df07c87.
* [ESP32/Linux] Added support of door-lock for esp32 and moved linux door-lock code to common * Restored linux implementation, implemented door lock support for esp32 * Restyled by clang-format * esp32/all-clusters-app's uses linux door-lock replace it with esp32 door-lock support * Restyled by whitespace * Restyled by clang-format * Removed redundent handling of lock/unlock Co-authored-by: Restyled.io <[email protected]>
)" (project-chip#24531) This reverts commit df07c87.
Problem
Changes
Testing