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

samples: matter: common: Cleaned up samples common directory #13485

Merged
merged 2 commits into from
Jan 5, 2024
Merged

samples: matter: common: Cleaned up samples common directory #13485

merged 2 commits into from
Jan 5, 2024

Conversation

LipinskiPNordicSemi
Copy link
Contributor

@LipinskiPNordicSemi LipinskiPNordicSemi commented Dec 20, 2023 •
edited
Loading

This commit:

  • creates directories for commons in the samples matter.
  • fix issue for matter bridge binding handler (one common BH used for every sample)

@github-actions github-actions bot added changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. manifest labels Dec 20, 2023
@NordicBuilder
Copy link
Contributor

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff
matter nrfconnect/sdk-connectedhomeip@d95eb1d (master) nrfconnect/sdk-connectedhomeip#370 nrfconnect/sdk-connectedhomeip#370/files

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@NordicBuilder
Copy link
Contributor

NordicBuilder commented Dec 20, 2023 •
edited
Loading

Test specification

CI/Jenkins/NRF

  • Integration Platforms

CI/Jenkins/integration

Test Module File based changes Manually selected West overwrite
test-fw-nrfconnect-chip X

Detailed information of selected test modules

Note: This message is automatically posted and updated by the CI

Damian-Nordic
Damian-Nordic approved these changes Dec 20, 2023
View reviewed changes
west.yml Outdated Show resolved Hide resolved
@github-actions github-actions bot removed the manifest label Dec 20, 2023
@NordicBuilder
Copy link
Contributor

You can find the documentation preview for this PR at this link. It will be updated about 10 minutes after the documentation build succeeds.

Note: This comment is automatically posted by the Documentation Publishing GitHub Action.

@markaj-nordic markaj-nordic changed the title Common cleanup samples: matter: common: Cleaned up samples common directory Dec 21, 2023
markaj-nordic
markaj-nordic requested changes Dec 21, 2023
View reviewed changes
Copy link
Contributor

@markaj-nordic markaj-nordic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please align Matter WS and Bridge apps.

samples/matter/lock/src/app_task.cpp Outdated Show resolved Hide resolved
@LipinskiPNordicSemi LipinskiPNordicSemi force-pushed the common_cleanup branch 5 times, most recently from db00b3f to 9686604 Compare January 4, 2024 08:56
ArekBalysNordic
ArekBalysNordic reviewed Jan 4, 2024
View reviewed changes
applications/matter_bridge/CMakeLists.txt Show resolved Hide resolved
samples/matter/common/src/bridge/bridge_storage_manager.h
@@ -7,7 +7,7 @@
#pragma once

#include "matter_bridged_device.h"
#include "persistent_storage_util.h"
#include "ps_storage/persistent_storage_util.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding the commit description, I can see the commit doesn't implement anything new. I guess, you've used the previously created common binding manager/handler in the Matter Bridge, right? :)

@LipinskiPNordicSemi @markaj-nordic
samples: matter: common: Cleaned up samples common directory
9b84889 
This commit creates directories for commons in the samples.

Signed-off-by: Patryk Lipinski <patryk.lipinski@nordicsemi.no>
@LipinskiPNordicSemi @markaj-nordic
applications: Matter Bridge: Use common binding handler
17ad801 
This commit removes Matter Bridge specific binding handler,
and re-uses the common one.

Signed-off-by: Patryk Lipinski <patryk.lipinski@nordicsemi.no>
@markaj-nordic markaj-nordic self-requested a review January 5, 2024 13:07
@markaj-nordic markaj-nordic removed the DNM label Jan 5, 2024
@nordicjm nordicjm merged commit 2790dbd into nrfconnect:main Jan 5, 2024
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kkasperczyk-no kkasperczyk-no kkasperczyk-no approved these changes

@markaj-nordic markaj-nordic markaj-nordic approved these changes

@ArekBalysNordic ArekBalysNordic ArekBalysNordic approved these changes

@Damian-Nordic Damian-Nordic Damian-Nordic approved these changes

@LuDuda LuDuda LuDuda approved these changes

@carlescufi carlescufi Awaiting requested review from carlescufi

@tejlmand tejlmand Awaiting requested review from tejlmand

Assignees
No one assigned
Labels
changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. manifest-matter
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@LipinskiPNordicSemi @NordicBuilder @LuDuda @kkasperczyk-no @Damian-Nordic @ArekBalysNordic @markaj-nordic @nordicjm