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.

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

[DNM] Update matter bridge to NCS 2.4.0 #1

Conversation

markus-becker-tridonic-com

No description provided.

@CLAassistant
Copy link

CLAassistant commented Jan 9, 2023

CLA assistant check
All committers have signed the CLA.

@markus-becker-tridonic-com markus-becker-tridonic-com changed the title Update matter bridge to NCS 2.0.2 [DNM] Update matter bridge to NCS 2.0.2 Jan 12, 2023
@markus-becker-tridonic-com
Copy link
Author

The signature change to the weak functions emberAfExternalAttributeReadCallback and emberAfExternalAttributeWriteCallback is not yet included, thus always the weak functions are called.

Copy link

@kkasperczyk-no kkasperczyk-no left a comment

Choose a reason for hiding this comment

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

Thank you a lot for the contribution! I have several initial comments.

@@ -3,6 +3,13 @@
* SPDX-License-Identifier: LicenseRef-Nordic-5-Clause
*/

/ {
chosen {

Choose a reason for hiding this comment

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

If the sample is going to support OTA DFU on the nRF5340 DK, it needs the below code in its overlay as well.

Choose a reason for hiding this comment

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

I have not looked at OTA at all.

src/app_task.cpp Show resolved Hide resolved
prj.conf Show resolved Hide resolved
prj.conf Show resolved Hide resolved
prj.conf Show resolved Hide resolved
src/app_task.cpp Show resolved Hide resolved
@@ -0,0 +1,15 @@
#
# Copyright (c) 2022 Nordic Semiconductor ASA

Choose a reason for hiding this comment

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

prj_no_dfu.conf and prj_release.conf files are not needed in the mcuboot and multiprotocol_rpmsg directories, as long, as we have only one prj.conf file in the main sample directory.

@lats1980
Copy link
Collaborator

I got lots of compile error like:

C:\Users\lats\ncs\v2.0.2\modules\lib\matter\src\app\OperationalDeviceProxy.h:200:51: error: 'const struct chip::Dnssd::CommonResolutionData' has no member named 'interfaceId'
200 | interfaceId = nodeData.resolutionData.interfaceId;
| ^~~~~~~~~~~
ninja: build stopped: subcommand failed.

Is there some missing code not included in the PR?

@markus-becker-tridonic-com
Copy link
Author

I got lots of compile error like:

C:\Users\lats\ncs\v2.0.2\modules\lib\matter\src\app\OperationalDeviceProxy.h:200:51: error: 'const struct chip::Dnssd::CommonResolutionData' has no member named 'interfaceId' 200 | interfaceId = nodeData.resolutionData.interfaceId; | ^~~~~~~~~~~ ninja: build stopped: subcommand failed.

Is there some missing code not included in the PR?

Yes, indeed. E.g. see #1 (comment) and I did some more changes (regen zap). Did not have the time yet to update the PR.

Since I do not have ZigBee devices I modified this sample app and added a shell for adding new devices. This makes it harder now to add the relevant changes back into this PR.

@markus-becker-tridonic-com markus-becker-tridonic-com changed the title [DNM] Update matter bridge to NCS 2.0.2 [DNM] Update matter bridge to NCS 2.4.0 Sep 28, 2023
@markus-becker-tridonic-com
Copy link
Author

I got lots of compile error like:

C:\Users\lats\ncs\v2.0.2\modules\lib\matter\src\app\OperationalDeviceProxy.h:200:51: error: 'const struct chip::Dnssd::CommonResolutionData' has no member named 'interfaceId' 200 | interfaceId = nodeData.resolutionData.interfaceId; | ^~~~~~~~~~~ ninja: build stopped: subcommand failed.

Is there some missing code not included in the PR?

I have updated the PR to NCS 2.4.0. Would you be able to check?

@markus-becker-tridonic-com
Copy link
Author

As a side-effect this might have changed the pins to the default pins for UART1 (1.00 and 1.01).

@markus-becker-tridonic-com
Copy link
Author

The changes were done in #2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants