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.

Sign up for GitHub

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

Jump to bottom

[NXP] Layering inversion in ConfigurationmanagerImpl.cpp #30599

Closed
andy31415 opened this issue Nov 21, 2023 · 2 comments · Fixed by #30891
Closed

[NXP] Layering inversion in ConfigurationmanagerImpl.cpp #30599

andy31415 opened this issue Nov 21, 2023 · 2 comments · Fixed by #30891

Comments

@andy31415
Copy link
Contributor

We have:

src/platform/nxp/common/ConfigurationManagerImpl.cpp
30:#include <app/server/Server.h>

It is not ok for something inside Platform to depend On app - this inverts layering as platform is supposed to be the lowest level and app is one of the highest levels. Generally GN would prevent this however app is excluded from layering checks because of legacy code. We should fix these layering inversions.

Found while checking #30596
@andy31415
Copy link
Contributor Author

It looks like the reason this is done is for Server::GetInstance().GetFabricTable().DeleteAllFabrics();.
We should figure out some eventing system and pass this into app rather than having platform calling directly into server.

@marius-alex-tache
Copy link
Contributor

@dinabenamar

@doru91 doru91 removed their assignment Nov 29, 2023
@dinabenamar dinabenamar mentioned this issue Dec 8, 2023
Merged
@mergify mergify bot closed this as completed in #30891 Dec 11, 2023
GabrielCouturier pushed a commit to NXP/matter that referenced this issue Dec 20, 2023
@dinabenamar
[COMMON] Remove circular dependency with application code in platform…
74df6d2 
… file

	* Github issue project-chip#30599 : [NXP] Layering inversion in ConfigurationmanagerImpl.cpp
	project-chip#30599
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@andy31415 @doru91 @marius-alex-tache