[BUG] [zephyr] Use connectedhomeip as a zephyr module

QuentinCaldeira-eaton commented 4 months ago

Reproduction steps

Hello, I'm trying to use connectedhomeip as a zephyr OS module. To do so, connectedhomeip is pulled from zephyr's west.yaml like this:

- name: connectedhomeip
      remote : connectedhomeip
      revision: 648d7bf3d2e52ded740c94989965cd8485c85832
      path: modules/lib/matter
      submodules:
        - name: nlio
          path: third_party/nlio/repo
        - name: nlassert
          path: third_party/nlassert/repo
        - name: nlunit-test
          path: third_party/nlunit-test/repo
        - name: pigweed
          path: third_party/pigweed/repo
        - name: jsoncpp
          path: third_party/jsoncpp/repo

Next, I moved the zephyr folder in config/zephyr/chip-module to the root, and changed the paths to Kconfig and CMakeList.

My goal is to include the Kconfig CONFIG_CHIP=y in a standalone zephyr application, and see it compiled. My final goal is to compile a Matter zephyr application on an STM32 target. For now, I've added the Kconfig CONFIG_CHIP to the hello_world sample in zephyr/samples/cpp.

When I west build, I first get an error about chip_build_pw_rpc_lib = false, like this one. Then if I bypass this error, I get hundreds of other errors.

I had understood that the zephyr platform could be used standalone. Is there another way to use connectedhomeip as a zephyr module?

Bug prevalence

Whenever I do this

GitHub hash of the SDK that was being used

648d7bf3d2e52ded740c94989965cd8485c85832

Platform

other

Platform Version(s)

No response

Anything else?

log.txt

Damian-Nordic commented 4 months ago

The standalone Zephyr platform in Matter was added by one developer who abandoned the platform maintenance shortly after :), so I'm afraid, you will have to update the platform yourself if you want to use it.

Note that there are 3 other Zephyr-based platforms in Matter - nrfconnect, telink and NXP. Because of that, we don't keep the Zephyr module manifest in the root directory (I mean not in this repo, but we do in our forks) but rather use ZEPHYR_EXTRA_MODULES CMake variable to add the selected Zephyr module (belonging to the selected Zephyr-based platform) to the build system.

Also, some files in src/platform/Zephyr are shared between all these platforms, so beware of that when modifying the code in there.

QuentinCaldeira-eaton commented 4 months ago

All right, in the end, the Zephyr platform can't be used as such for pure Zephyr projects (without a manufacturer's SDK). The platform only works thanks to nRF, NXP and Telink implementations? So, is the best approach to develop an STM32 Zephyr platform, or to make the Zephyr platform standalone?

Damian-Nordic commented 4 months ago

I mean if you use vanilla Zephyr, it should be OK with you to use the standalone Zephyr platform in Matter. I just wanted to let you know that it's currently lacking a maintainer so it's not surprising that it doesn't work at the moment :). But if you can resurrect it, it may be the right choice. We as a Nordic use a separate platform because our SDK is a superset of Zephyr so we want to have a chance to use some extra features of our SDK in Matter.

Damian-Nordic commented 4 months ago

That being said, if you need to modify shared Zephyr files in Matter, feel free to ping me to review the changes.

QuentinCaldeira-eaton commented 3 months ago

Hi @Damian-Nordic ! I'm currently working on the Zephyr standalone app for STM32. I'm working with the stm32wb5mm_dk. Here is my prj.conf for the app :

CONFIG_CHIP=y
CONFIG_CHIP_DEVICE_PRODUCT_ID=32773
CONFIG_STD_CPP17=y
CONFIG_CHIP_ENABLE_PAIRING_AUTOSTART=y
CONFIG_BT=y
CONFIG_NETWORKING=y
CONFIG_NET_L2_OPENTHREAD=y
CONFIG_IEEE802154=y
CONFIG_ENTROPY_STM32_RNG=y

When I build the app, I have this error

/home/quentin/github/workspace/zephyr/include/zephyr/device.h:91:41: error: '__device_dts_ord_DT_CHOSEN_zephyr_ieee802154_ORD' undeclared here (not in a function)
   91 | #define DEVICE_NAME_GET(dev_id) _CONCAT(__device_, dev_id)
      |                                         ^~~~~~~~~
/home/quentin/github/workspace/zephyr/include/zephyr/toolchain/common.h:137:26: note: in definition of macro '_DO_CONCAT'
  137 | #define _DO_CONCAT(x, y) x ## y
      |                          ^
/home/quentin/github/workspace/zephyr/include/zephyr/device.h:91:33: note: in expansion of macro '_CONCAT'
   91 | #define DEVICE_NAME_GET(dev_id) _CONCAT(__device_, dev_id)
      |                                 ^~~~~~~
/home/quentin/github/workspace/zephyr/include/zephyr/device.h:228:37: note: in expansion of macro 'DEVICE_NAME_GET'
  228 | #define DEVICE_DT_NAME_GET(node_id) DEVICE_NAME_GET(Z_DEVICE_DT_DEV_ID(node_id))
      |                                     ^~~~~~~~~~~~~~~
/home/quentin/github/workspace/zephyr/include/zephyr/device.h:245:34: note: in expansion of macro 'DEVICE_DT_NAME_GET'
  245 | #define DEVICE_DT_GET(node_id) (&DEVICE_DT_NAME_GET(node_id))
      |                                  ^~~~~~~~~~~~~~~~~~
/home/quentin/github/workspace/zephyr/modules/openthread/platform/radio.c:96:9: note: in expansion of macro 'DEVICE_DT_GET'
   96 |         DEVICE_DT_GET(DT_CHOSEN(zephyr_ieee802154));

I can't find a resource linked to this __device_dts_ord_DT_CHOSEN_zephyr_ieee802154_ORD device tree function. Do you know more about that ?

Thanks in advance !

Damian-Nordic commented 3 months ago

@QuentinCaldeira-eaton sorry for late response. It looks like you don't have ieee802154 node inside chosen node in the devicetree.

QuentinCaldeira-eaton commented 3 months ago

@Damian-Nordic Yes, thanks for the response. I decided to move to nucleo h536zi with ethernet connectivity, to be independant of stm32 drivers. But again, can't compile as a Zephyr module. I should work on that. Alongside, I try to use connectedhomeip as a library, by adding all .cpp files with CMake, but I don't get better results. Where can I found some support to do that ?

QuentinCaldeira-eaton commented 3 months ago

@Damian-Nordic sorry to bother you, but could you try to build a Zephyr sample like blinky, with the CONFIG_CHIP=y in prj.conf, for an ARM (or stm32) board, using connectedhomeip as a zephyr module ? I can't figure why when I do that, I have a lot of Zephyr errors like /home/quentin/github/workspace/zephyr/include/zephyr/toolchain/gcc.h:573:2: error: #error processor architecture not supported However, if I check the .config in build directory, CONFIG_ARM is true, so I shouldn't meet this error. Also, error: 'CONFIG_ZVFS_OPEN_MAX' was not declared in this scope but this config is present in .config I've seen that something is done about the CONFIG_ARM in nRF CMakeLists but it doesn't help here. Thanks !

QuentinCaldeira-eaton commented 3 months ago

Hi @Damian-Nordic, I've worked a bit on the zephyr module, and here are my changes so far https://github.com/QuentinCaldeira-eaton/connectedhomeip Here is a quick description

For the config (cmake etc...), I used a precedent work made by someone else. For some reason the build system can't find zephyr POSIX with the previous config.
Remove inet/ZephyrSockets.h : I think this is deprecated, since zephyr seems to have an implementation of recvmsg
Use #ifdef in src/platform/Zephyr/CHIPDevicePlatformConfig, because if I put CONFIG_BT=n in my prj.conf, Zephyr seems to disable this CONFIG, instead of force it to n. The result is CONFIG_BT is not defined if I put CONFIG_BT to n.

Is it the good direction ? What do you think ? Thanks in advance for your review !

Damian-Nordic commented 3 months ago

@QuentinCaldeira-eaton Sorry, but I really don't have time for debugging build issues in vanilla Zephyr.

But the changes you've made look OK. You can delete ZephyrSocket.h entirely - I think all platforms have already switched to the Zephyr version that provides recvmsg. Feel free to post a PR with the changes so I can review them more thoroughly :)

QuentinCaldeira-eaton commented 3 months ago

@Damian-Nordic, yes, of course, sorry to ping you so much. Just a quick question, so. Do you think the Ethernet implementation should follow the Wi-Fi implementation (with a dedicated folder and driver) ? Then, why we don't do the same thing for Thread ? Thanks for your time, really !

Damian-Nordic commented 3 months ago

For Wi-Fi we need the driver for scanning and managing Wi-Fi credentials. For Thread it's shared among all OpenThread platforms - hence it's in src/platform/OpenThread. What exactly components are necessary for Ethernet given that you don't need Ethernet credentials? My understanding is that if you bring the Zephyr interface up, Matter should start using it as it uses BSD sockets in the case of the Zephyr platform.

QuentinCaldeira-eaton commented 3 months ago

My guess is, as ESP32 or Tizen, I should have an Ethenet 'object', in order to instance it in the AppTask. The Ethernet object might have some functions like Next, Count etc... I'm not very comfortable with C++ but that's what I've understood. I called it driver, but in fact Zephyr already as a good implementation of Ethernet. It's more the link between Matter and the Zephyr Ethernet

QuentinCaldeira-eaton commented 2 months ago

Hi @Damian-Nordic, sorry to bother you, but I have a quick question linked to our previous discussion. After the PR, I continue to work on my Ethernet example, but it doesn't work. I cannot figure it out, but I think it's because Matter don't find the Zephyr interface. I think I should have, in the Zephyr platform directory, a driver which perform a DHCP IPv6 attribution, at least. When I debug my application with Wireshark, I see my device, but it has no ipv6 address (appears with ::). Even with the CONFIG_CHIP_ENABLE_PAIRING_AUTOSTART=y, I can't see mdns messages in wireshark. When debugging, the sin6.address is always full of zeros.

I understand that Ethernet don't need more functions, because it inherits from BaseDriver with no functions added, but I think it should perform at least a DHCP. Because Zephyr could declare a static ipv6 address in prj.conf, I should seem like

#ifdef CONFIG_NET_CONFIG_MY_IPV6_ADDR
address = CONFIG_NET_CONFIG_MY_IPV6_ADDR
#else   
address = start_dhcpv6()....
#endif

And Matter should use this ip address (I don't know how yet). What do you think ? You could find my whole application and logs here for further information https://github.com/project-chip/connectedhomeip/issues/34317

Damian-Nordic commented 2 months ago

Things like DHCP don't need to be called from the Matter stack - normally, auto-configuration is triggered by Zephyr when the interface is brought up.

Also, in the case of IPv6, Ethernet driver should assign a link-local IPv6 address that should be enough to communicate with devices in the same network.

Apologies, but I don't have time to analyze your issue more carefully...

QuentinCaldeira-eaton commented 2 months ago

@Damian-Nordic No need to apologies, I just can't find any support or forum for connectedhomeip. I've attribute a static IP address. My application says that it publish mDNS, but can't see it in Wireshark. I thought it was because there is no CONFIG_MDNS (resolver, sd, etc.), but when I put this in my prj.conf, InitChipStack exit with an error. Is this because you (nrf) implement multicast in ConnectivityManagerImpl and not Zephyr ?

Damian-Nordic commented 2 months ago

The question is how you publish the services. Matter has the minimal mDNS implementation and if you use it, it tries to join ff02::fb multicast group using IPV6_JOIN_GROUP socket option. This option used to not be supported in Zephyr so we had to add a platform-specific handling (in ConnectivityManagerImpl). But if you use Zephyr's mDNS implementation then this shouldn't be needed.

You probably just need to read more of the Zephyr networking bits, enable verbose logging and analyze what happens under the hood. I won't be able to assist with such general challenges, especially that you use a hardware platform that I'm not familiar with.

QuentinCaldeira-eaton commented 2 months ago

@Damian-Nordic it already help me a lot understanding what was done in ConnectivityManagerImpl. Thanks a lot ! I will close this issue too

QuentinCaldeira-eaton commented 2 months ago

connectedhomeip doesn't have the pw_rpc_lib error anymore

project-chip / connectedhomeip