Refactorization of ot repos #7139

doru91 · 2021-05-26T19:06:39Z

This addresses #7058

Part 1:
[K32W] Use the new ot-nxp repo

OpenThread maintainers decided to split the big OT repo (core OT stack + vendor example)
in several repos: one repo containing the OT stack, then each vendor has a separate repo
with its example (the vendor repo contains the OT stack repo as a submodule).

This commit allows using the ot-nxp repo inside PCHIP. Only the platform files from ot-nxp
are used, the OT stack repo used is still the one from third_party/openthread/repo.

Part 2: Use the new ot-{efr32/qorvo} repos

This PR also touches Silabs and Qorvo files so help from @jmartinez-silabs and someone from Qorvo is needed.

todo · 2021-05-26T19:06:41Z

*/

connectedhomeip/third_party/k32w_sdk/sdk_fixes/openthread-core-k32w061-config.h

Lines 102 to 112 in 1a9ce7e

    
           /* TODO */ 
        
           /** 
        
            * @def OPENTHREAD_CONFIG_ENABLE_SOFTWARE_CSMA_BACKOFF 
        
            * 
        
            * Define to 1 if you want to enable software CSMA-CA backoff logic. 
        
            * 
        
            */ 
        
           /* TODO */ 
        
           /**

This comment was generated by todo based on a `TODO` comment in `1a9ce7e` in #7139. cc @doru91.

todo · 2021-05-26T19:06:44Z

*/

connectedhomeip/third_party/k32w_sdk/sdk_fixes/openthread-core-k32w061-config.h

Lines 110 to 120 in 1a9ce7e

    
           /* TODO */ 
        
           /** 
        
            * @def OPENTHREAD_CONFIG_NCP_HDLC_ENABLE 
        
            * 
        
            * Define to 1 to enable NCP HDLC support. 
        
            * 
        
            */ 
        
           #ifndef OPENTHREAD_CONFIG_NCP_HDLC_ENABLE 
        
           #define OPENTHREAD_CONFIG_NCP_HDLC_ENABLE 1 
        
           #endif

This comment was generated by todo based on a `TODO` comment in `1a9ce7e` in #7139. cc @doru91.

todo · 2021-05-26T19:06:47Z

*/

connectedhomeip/third_party/k32w_sdk/sdk_fixes/system.c

Lines 79 to 89 in 1a9ce7e

    
               /* TODO */ 
        
               return false; 
        
           } 
        
           void otSysDeinit(void) 
        
           { 
        
               /* TODO */ 
        
           } 
        
           void otSysProcessDrivers(otInstance *aInstance) 
        
           {

This comment was generated by todo based on a `TODO` comment in `1a9ce7e` in #7139. cc @doru91.

todo · 2021-05-26T19:06:50Z

*/

connectedhomeip/third_party/k32w_sdk/sdk_fixes/system.c

Lines 85 to 95 in 1a9ce7e

    
               /* TODO */ 
        
           } 
        
           void otSysProcessDrivers(otInstance *aInstance) 
        
           { 
        
               K32WRadioProcess(aInstance); 
        
               K32WUartProcess(); 
        
               K32WAlarmProcess(aInstance); 
        
           } 
        
           WEAK void otSysEventSignalPending(void)

This comment was generated by todo based on a `TODO` comment in `1a9ce7e` in #7139. cc @doru91.

OpenThread maintainers decided to split the big OT repo (core OT stack + vendor example) in several repos: one repo containing the OT stack, then each vendor has a separate repo with its example (the vendor repo contains the OT stack repo as a submodule). This commit allows using the ot-nxp repo inside PCHIP. Only the platform files from ot-nxp are used, the OT stack repo used is still the one from third_party/openthread/repo. Signed-off-by: Doru Gucea <doru-cristian.gucea@nxp.com>

Signed-off-by: Doru Gucea <doru-cristian.gucea@nxp.com>

tima-q · 2021-05-27T09:49:32Z

src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp

@@ -813,10 +813,6 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::DoInit(otInstanc
        VerifyOrExit(otInst != NULL, err = MapOpenThreadError(OT_ERROR_FAILED));
    }

-#if !defined(__ZEPHYR__) && !defined(ENABLE_CHIP_SHELL) && !defined(PW_RPC_ENABLED)


The path towards OT CLI interface get's removed here. What will be the substitution to access the OT infrastructure for debugging purposes ?
Noted removing the CLI frees up quite some RAM/flash if no longer required

I removed these lines because otCliUartInit API is not available anymore in openthread since openthread/openthread#6243.
I will push a change such that the user will be able to use a GN build argument for enabling/disabling the OT CLI.

OT CLI takes around 25K of flash with everything compiled with size optimizations - I wonder why it requires so much space.

tima-q · 2021-05-27T09:53:34Z

examples/platform/k32w/app/project_include/OpenThreadConfig.h

@@ -44,6 +44,15 @@
 // Use smaller maximum interval to speed up reattaching.
 #define OPENTHREAD_CONFIG_MLE_ATTACH_BACKOFF_MAXIMUM_INTERVAL (60 * 10 * 1000) // default 1200000 ms

+// disable unused features


Agree to remove these features as they are indeed unused within Matter - we would leave a lot of unused dead code in the Matter stack otherwise.
But are they not required for Thread stack certification ? Which triggers the question how the supported set of Thread features relates to Matter certification.

To be honest, I don't know how Matter certification will work in conjunction with Thread certification. In my opinion. we shouldn't loose Thread certification if some features which are not used by Matter are disabled. Maybe someone experienced with the certification process can comment.

AFAIK at least Joiner functionality (from the below list) is now required to pass the official Thread certification for End Device / Router.

Update QPG6100 OpenThreadConfig.h (UART->HDLC)

Signed-off-by: Doru Gucea <doru-cristian.gucea@nxp.com>

github-actions · 2021-05-27T12:36:55Z

Size increase report for "gn_qpg6100-example-build" from d96d8a3

File	Section	File	VM
chip-qpg6100-lighting-example.out	.heap	0	2128
chip-qpg6100-lighting-example.out	.bss	0	-2128
chip-qpg6100-lighting-example.out	.text	-22036	-22036

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-qpg6100-lighting-example.out.map and ./pull_artifact/chip-qpg6100-lighting-example.out.map:

BLOAT EXECUTION FAILED WITH CODE 1:
bloaty: unknown file type for file './pull_artifact/chip-qpg6100-lighting-example.out.map'

Comparing ./master_artifact/chip-qpg6100-lighting-example.out and ./pull_artifact/chip-qpg6100-lighting-example.out:

sections,vmsize,filesize
.heap,2128,0
.debug_aranges,0,-1824
.bss,-2128,0
.debug_ranges,0,-5096
.debug_frame,0,-6280
.strtab,0,-12460
.symtab,0,-13792
.debug_abbrev,0,-15929
.text,-22036,-22036
.debug_str,0,-26477
.debug_line,0,-42867
[Unmapped],0,-43500
.debug_loc,0,-50388
.debug_info,0,-892651

Signed-off-by: Doru Gucea <doru-cristian.gucea@nxp.com>

doru91 · 2021-05-27T14:17:06Z

CHIP_DEVICE_CONFIG_THREAD_ENABLE_CLI

Done.

doru91 · 2021-05-27T14:18:11Z

could this logic be moved to third_party/openthread/platforms/BUILD.gn to be generic for all ?

I wouldn't add this change to third_party/openthread/platforms/BUILD.gn as that BUILD.gn file contains rules for building core OT stack. The CLI implementation should be platform dependant.

build_overrides/ot_efr32.gni

* Re-enabled CLI on QPG6100 - introducing CLI flag in DeviceConfig.h

Fix to efr buld to support openthread repo update

doru91 · 2021-05-28T15:49:52Z

third_party/openthread/platforms/efr32/BUILD.gn

-    "${openthread_root}/examples/platforms/efr32/src/openthread-core-efr32-config.h",
+    "${openthread_efr32_root}/src/src/openthread-core-efr32-config-check.h",
+    "${openthread_efr32_root}/src/src/openthread-core-efr32-config.h",
+    "${openthread_root}/src/cli/cli_config.h",


@mspang, could you please help us removing this ugly hack?

We had to do it in order to bypass this error:

ERROR at //third_party/connectedhomeip/third_party/openthread/repo/examples/apps/cli/cli_uart.cpp:37:11: Include not allowed.
#include "cli/cli_config.h"
^---------------
It is not in any dependency of
//third_party/connectedhomeip/third_party/openthread/platforms:libopenthread-cli-uart
The include file is in the target(s):
//third_party/connectedhomeip/third_party/openthread/repo/src/cli:libopenthread-cli-ftd
//third_party/connectedhomeip/third_party/openthread/repo/src/cli:libopenthread-cli-mtd

LuDuda · 2021-05-28T18:53:56Z

build_overrides/ot_nxp.gni

@@ -0,0 +1,17 @@
+# Copyright (c) 2020 Project CHIP Authors


LuDuda · 2021-05-28T19:01:23Z

examples/platform/k32w/app/project_include/OpenThreadConfig.h

@@ -44,6 +44,15 @@
 // Use smaller maximum interval to speed up reattaching.
 #define OPENTHREAD_CONFIG_MLE_ATTACH_BACKOFF_MAXIMUM_INTERVAL (60 * 10 * 1000) // default 1200000 ms

+// disable unused features


AFAIK at least Joiner functionality (from the below list) is now required to pass the official Thread certification for End Device / Router.

Suggestion cleanup + re-enabling CLI on QPG6100

* [K32W] Use the new ot-nxp repo OpenThread maintainers decided to split the big OT repo (core OT stack + vendor example) in several repos: one repo containing the OT stack, then each vendor has a separate repo with its example (the vendor repo contains the OT stack repo as a submodule). This commit allows using the ot-nxp repo inside PCHIP. Only the platform files from ot-nxp are used, the OT stack repo used is still the one from third_party/openthread/repo. Signed-off-by: Doru Gucea <doru-cristian.gucea@nxp.com> * Use the new ot-{efr32/qorvo} repos Signed-off-by: Doru Gucea <doru-cristian.gucea@nxp.com> * Restyled by whitespace * Restyled by clang-format * Restyled by gn * Restyled by shfmt * Fix ot submodules Signed-off-by: Doru Gucea <doru-cristian.gucea@nxp.com> * Remove TODOs Signed-off-by: Doru Gucea <doru-cristian.gucea@nxp.com> * Restyled by clang-format * Remove ot-efr from .gitsubmodules Signed-off-by: Doru Gucea <doru-cristian.gucea@nxp.com> * Fix ot-efr submodule Signed-off-by: Doru Gucea <doru-cristian.gucea@nxp.com> * * Update QPG6100 OpenThreadConfig.h (UART->HDLC) * [K32W] Add a GN build arg for enabling/disabling the OT CLI Signed-off-by: Doru Gucea <doru-cristian.gucea@nxp.com> * Restyled by clang-format * Restyled by gn * Fix define naming Signed-off-by: Doru Gucea <doru-cristian.gucea@nxp.com> * * Removing ot_<platform>.gni files - moved ot root var to platforms * Re-enabled CLI on QPG6100 - introducing CLI flag in DeviceConfig.h * Fix to efr buld to support openthread repo update Co-authored-by: Restyled.io <commits@restyled.io> Co-authored-by: Timothy Maes <timothy.maes@qorvo.com> Co-authored-by: jmartinez-silabs <junior.martinez@silabs.com>

boring-cyborg bot added the examples label May 26, 2021

boring-cyborg bot added k32w openthread platform labels May 26, 2021

doru91 mentioned this pull request May 26, 2021

Openthread submodule needs to be updated #7058

Closed

restyled-io bot mentioned this pull request May 26, 2021

Restyle Feature/ot repo refactoring stash #7140

Closed

doru91 changed the title ~~Feature/ot repo refactoring stash~~ Feature/ot repo refactoring May 26, 2021

pullapprove bot requested review from chrisdecenzo, jelderton, jmartinez-silabs, LuDuda, mspang, saurabhst and woody-apple May 26, 2021 19:31

pullapprove bot added the review - pending label May 26, 2021

doru91 changed the title ~~Feature/ot repo refactoring~~ Refactorization of ot repos May 26, 2021

doru91 force-pushed the feature/ot_repo_refactoring_stash branch from 990906f to cb3bd36 Compare May 26, 2021 19:37

restyled-io bot mentioned this pull request May 27, 2021

Restyle Refactorization of ot repos #7178

Closed

doru91 and others added 8 commits May 27, 2021 00:36

Use the new ot-{efr32/qorvo} repos

bb9f767

Signed-off-by: Doru Gucea <doru-cristian.gucea@nxp.com>

Restyled by whitespace

64ca3ae

Restyled by clang-format

89dfcf5

Restyled by gn

dca8657

Restyled by shfmt

c778e57

Fix ot submodules

aae7886

Signed-off-by: Doru Gucea <doru-cristian.gucea@nxp.com>

Remove TODOs

6b16197

Signed-off-by: Doru Gucea <doru-cristian.gucea@nxp.com>

doru91 force-pushed the feature/ot_repo_refactoring_stash branch from abfcd3b to a27add0 Compare May 27, 2021 07:36

doru91 added 2 commits May 27, 2021 01:02

Remove ot-efr from .gitsubmodules

4c8e905

Signed-off-by: Doru Gucea <doru-cristian.gucea@nxp.com>

Fix ot-efr submodule

202bb42

Signed-off-by: Doru Gucea <doru-cristian.gucea@nxp.com>

tima-q reviewed May 27, 2021

View reviewed changes

tima-q and others added 5 commits May 27, 2021 10:03

* Update QPG6100 OpenThreadConfig.h (UART->HDLC)

f7f96a1

Merge pull request #1 from tima-q/doru/ot_repo_refactoring

71968c7

Update QPG6100 OpenThreadConfig.h (UART->HDLC)

[K32W] Add a GN build arg for enabling/disabling the OT CLI

ea95394

Signed-off-by: Doru Gucea <doru-cristian.gucea@nxp.com>

Restyled by clang-format

cc84e36

Restyled by gn

c5d9686

restyled-io bot mentioned this pull request May 27, 2021

Restyle Refactorization of ot repos #7184

Closed

Fix define naming

b69d72c

Signed-off-by: Doru Gucea <doru-cristian.gucea@nxp.com>

jmartinez-silabs reviewed May 27, 2021

View reviewed changes

build_overrides/ot_efr32.gni Outdated Show resolved Hide resolved

tima-q and others added 3 commits May 28, 2021 14:39

* Removing ot_<platform>.gni files - moved ot root var to platforms

e8239c8

* Re-enabled CLI on QPG6100 - introducing CLI flag in DeviceConfig.h

Fix to efr buld to support openthread repo update

20fb5b6

Merge pull request #3 from jmartinez-silabs/update_ot_repo

63ba16f

Fix to efr buld to support openthread repo update

doru91 commented May 28, 2021

View reviewed changes

jmartinez-silabs approved these changes May 28, 2021

View reviewed changes

LuDuda approved these changes May 28, 2021

View reviewed changes

Merge pull request #2 from tima-q/doru/ot_repo_refactoring

ae93bc1

Suggestion cleanup + re-enabling CLI on QPG6100

tima-q approved these changes May 29, 2021

View reviewed changes

jmartinez-silabs mentioned this pull request May 31, 2021

Correction SRP service removal from local cache #7236

Closed

andy31415 approved these changes May 31, 2021

View reviewed changes

pullapprove bot added review - approved and removed review - pending labels May 31, 2021

andy31415 merged commit dcc1558 into project-chip:master May 31, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactorization of ot repos #7139

Refactorization of ot repos #7139

doru91 commented May 26, 2021 •

edited

Loading

todo bot commented May 26, 2021

todo bot commented May 26, 2021

todo bot commented May 26, 2021

todo bot commented May 26, 2021

tima-q May 27, 2021

doru91 May 27, 2021 •

edited

Loading

tima-q May 27, 2021

doru91 May 27, 2021

LuDuda May 28, 2021

github-actions bot commented May 27, 2021

doru91 commented May 27, 2021

doru91 commented May 27, 2021

doru91 May 28, 2021

LuDuda May 28, 2021

LuDuda May 28, 2021

Refactorization of ot repos #7139

Refactorization of ot repos #7139

Conversation

doru91 commented May 26, 2021 • edited Loading

todo bot commented May 26, 2021

*/

This comment was generated by todo based on a TODO comment in 1a9ce7e in #7139. cc @doru91.

todo bot commented May 26, 2021

*/

This comment was generated by todo based on a TODO comment in 1a9ce7e in #7139. cc @doru91.

todo bot commented May 26, 2021

*/

This comment was generated by todo based on a TODO comment in 1a9ce7e in #7139. cc @doru91.

todo bot commented May 26, 2021

*/

This comment was generated by todo based on a TODO comment in 1a9ce7e in #7139. cc @doru91.

tima-q May 27, 2021

Choose a reason for hiding this comment

doru91 May 27, 2021 • edited Loading

Choose a reason for hiding this comment

tima-q May 27, 2021

Choose a reason for hiding this comment

doru91 May 27, 2021

Choose a reason for hiding this comment

LuDuda May 28, 2021

Choose a reason for hiding this comment

github-actions bot commented May 27, 2021

doru91 commented May 27, 2021

doru91 commented May 27, 2021

doru91 May 28, 2021

Choose a reason for hiding this comment

LuDuda May 28, 2021

Choose a reason for hiding this comment

LuDuda May 28, 2021

Choose a reason for hiding this comment

doru91 commented May 26, 2021 •

edited

Loading

This comment was generated by todo based on a `TODO` comment in `1a9ce7e` in #7139. cc @doru91.

This comment was generated by todo based on a `TODO` comment in `1a9ce7e` in #7139. cc @doru91.

This comment was generated by todo based on a `TODO` comment in `1a9ce7e` in #7139. cc @doru91.

This comment was generated by todo based on a `TODO` comment in `1a9ce7e` in #7139. cc @doru91.

doru91 May 27, 2021 •

edited

Loading