-
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]: We should not setup DeviceInfoProvider via PlatfromMgr on ESP32. #21039
[esp32]: We should not setup DeviceInfoProvider via PlatfromMgr on ESP32. #21039
Conversation
PR #21039: Size comparison from d5fd062 to 5977fef Increases (2 builds for bl602, telink)
Decreases (1 build for bl602)
Full report (41 builds for bl602, cc13x2_26x2, cyw30739, efr32, k32w, linux, mbed, nrfconnect, p6, 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.
See #18236 (comment)
I don't think this PR is conflict with #18236 (comment), it should use the existing common implementation of DeviceInfoProvider instead of making a copy of it and put it into platform/esp in #18236 The goal of this PR is to decouple the DeviceInfoProvider and PlatfromMgr on ESP32. |
PR #21039: Size comparison from 5665d38 to 46b9d81 Increases (3 builds for bl602, cc13x2_26x2, k32w)
Decreases (3 builds for cc13x2_26x2, esp32)
Full report (32 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
Problem
What is being fixed? Examples:
Currently, we call SetDiagnosticDataProvider within PaltformMgr separately on ESP32 platform. And this will cause call DiagnosticDataProvider get set up on both client and server side.
Change overview
Use common DeviceInfoProvider implementation instead of platform specific version
Please note, currently, some functions of DeviceInfoProvider does not work since storage delegate has not been setup for DeviceInfoProvider. This PR does not solve this issue either, it will be addressed in a separate PR.
Testing
How was this tested? (at least one bullet point required)