ESP32 gn support #2325

kedars · 2020-08-25T14:49:34Z

Problem

Add gn build for ESP32

Summary of Changes

fixes #1510

andy31415 · 2020-08-25T15:25:23Z

Shoud this be a draft while WIP or is it ready for review?

mspang · 2020-08-25T16:10:13Z

src/platform/BUILD.gn

+        "ESP32/SystemTimeSupport.cpp",
+        "FreeRTOS/SystemTimeSupport.cpp",
+      ]
+      #      public_configs += [ "${chip_root}/src/crypto:crypto_config" ]


mspang · 2020-08-25T16:10:47Z

src/system/BUILD.gn

@@ -106,8 +106,10 @@ source_set("system_config_header") {

  public_deps = [ ":system_buildconfig" ]

-  if (chip_system_config_use_lwip) {
-    public_deps += [ "${chip_root}/src/lwip" ]
+  if (target_cpu != "esp32") {


target_cpu -> current_cpu

Can you move this condition inside src/lwip/BUILD.gn ? See e.g. third_party/mbedtls/BUILD.gn

mspang · 2020-08-25T16:19:55Z

src/ble/BUILD.gn

+  if (chip_device_platform == "esp32") {
+    defines +=
+        [ "BLE_PLATFORM_CONFIG_INCLUDE=${chip_ble_platform_config_include}" ]
+  }


Is this block intended to be added? It looks like BLE_PLATFORM_CONFIG_INCLUDE is set twice here.

Ah, the upper 'if' condition had a typo, fixed.

mspang · 2020-08-25T16:22:19Z

config/esp32/toolchain/BUILD.gn

+import("//build/toolchain/gcc_toolchain.gni")
+
+declare_args() {
+  ar_ext = ""


If they're defined here, probably makes sense to name them something like

esp32_ar
esp32_cc
esp32_cxx

mspang · 2020-08-25T16:23:01Z

config/esp32/toolchain/BUILD.gn

+
+  toolchain_args = {
+    current_os = "freertos"
+    current_cpu = cpu_ext


Can this be "xtensa" ? The cpu variable is typically used to determine the architecture.

I've been going back and forth on that myself. Finally I thought there is nothing 'xtensa' specific here, the toolchain is esp32-specific, the SDK is esp32-specific, so might as well call it 'esp32'. Thoughts? What typically goes into generic 'cpu' specific files?

An important use case is writing assembly code. See e.g.
https://github.com/project-chip/connectedhomeip/blob/master/third_party/jlink/segger_rtt/RTT/BUILD.gn#L40

mspang · 2020-08-25T16:27:34Z

src/crypto/BUILD.gn

+
+    # ESP32 has its own mbedTLS
+    if (target_cpu != "esp32") {
+      public_deps += [ "${mbedtls_root}:mbedtls" ]


Remove this and set the |mbedtls_target| argument.

github-actions · 2020-08-27T07:03:45Z

Size increase report for "gn_nrf-example-build"

File	Section	File	VM

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

github-actions · 2020-08-27T07:03:52Z

Size increase report for "gn_linux-example-build"

File	Section	File	VM

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

github-actions · 2020-08-27T07:04:00Z

Size increase report for "gn_efr32-example-build"

File	Section	File	VM
chip-efr32-lock-example.out	.bss	0	1024

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-efr32-lock-example.out and ./pull_artifact/chip-efr32-lock-example.out:

sections,vmsize,filesize
.bss,1024,0
.debug_loc,0,719
.debug_info,0,335
.debug_str,0,179
.debug_abbrev,0,150
.debug_frame,0,4
.debug_line,0,-591

github-actions · 2020-08-27T07:04:07Z

Size increase report for "nrfconnect-example-build"

File	Section	File	VM
chip-nrf52840-lock-example.elf	bss	0	744
chip-nrf52840-lock-example.elf	text	134	134
chip-nrf52840-lock-example.elf	[LOAD #1 [RWX]]	-6	-6
chip-nrf52840-lock-example.elf	[LOAD #3 [RW]]	0	-8

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-nrf52840-lock-example.elf and ./pull_artifact/chip-nrf52840-lock-example.elf:

sections,vmsize,filesize
.debug_info,0,804
bss,744,0
.debug_abbrev,0,169
.debug_str,0,169
.strtab,0,151
text,134,134
.debug_line,0,88
.debug_frame,0,80
.symtab,0,64
.debug_aranges,0,16
.debug_ranges,0,16
[Unmapped],0,-1
[LOAD #1 [RWX]],-6,-6
[LOAD #3 [RW]],-8,0

github-actions · 2020-08-27T07:04:14Z

Size increase report for "esp32-example-build"

File	Section	File	VM

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-wifi-echo.elf and ./pull_artifact/chip-wifi-echo.elf:

BLOAT EXECUTION FAILED WITH CODE 1:
bloaty: integer overflow

This reverts commit 8b76957.

kedars · 2020-08-27T09:51:09Z

Approving, looks good.

@kedars, can you confirm this is no longer WIP - given you pulled it out of draft?

@woody-apple Everything is done, 2 notes:

this replaces automake build on ESP32 with GN (no CMake compatibility for CHIP, I hope that's ok?)
QEMU tests with GN may take a while. As @mspang mentioned above we could do this in a follow-on patch. Do we want to merge it as is for now? Or should I figure out if we can make both 'automake' and 'gn' build variants work for ESP32. So that gives us a breather while we take on the qemu tests?

kedars · 2020-08-27T15:36:37Z

Approving, looks good.
@kedars, can you confirm this is no longer WIP - given you pulled it out of draft?

@woody-apple Everything is done, 2 notes:

this replaces automake build on ESP32 with GN (no CMake compatibility for CHIP, I hope that's ok?)

QEMU tests with GN may take a while. As @mspang mentioned above we could do this in a follow-on patch. Do we want to merge it as is for now? Or should I figure out if we can make both 'automake' and 'gn' build variants work for ESP32. So that gives us a breather while we take on the qemu tests?

Ok, so I made both of these builds works

the default is the current automake. Pass the option CHIP_BUILD_WITH_GN=y to the build to build with GN
also added a GitHub workflow so the GN build is triggered as well (could you please help review the workflow changes)

I will flip the build to GN default in a follow-on PR (once I figure out what the documentation changes required to the example's README)

For the 'restyler' issue, I am not able to view what it is complaining about, if it is about using $1 without double-quotes in the esp_echo_app.sh, I intend it to be without double-quotes, so it nulls out

woody-apple · 2020-08-28T22:13:23Z

FYI to whomever merges, there's a known issue that for edits to .workflows, we may need to force merge changes in (like this one).

rwalker-apple · 2020-08-29T19:09:04Z

@saurabhst @hawk248 @jelderton ?

This used to work prior to project-chip#2325, which added an unconditional dependency on lwip even if we're using sockets. Make it conditional.

* Allow building CHIP without lwip checked out This used to work prior to #2325, which added an unconditional dependency on lwip even if we're using sockets. Make it conditional. * Fix typo

kedars added 2 commits August 25, 2020 12:50

Support external c/cc flags (as-is from rafal's PR)

a43a0a8

Use external mbedtls (used as-is from Rafal's PR)

ba63884

kedars requested review from andy31415, anush-apple, BroderickCarlin, chrisdecenzo, hawk248, jelderton, robszewczyk, saurabhst and woody-apple as code owners August 25, 2020 14:49

boring-cyborg bot added ble config crypto esp32 examples gn inet lwip platform system labels Aug 25, 2020

restyled-io bot mentioned this pull request Aug 25, 2020

Restyle WIP: ESP32 gn support #2326

Closed

ESP32: Enable gn support (first cut)

53d6ef3

kedars force-pushed the esp32_gn_support branch from f421f55 to 53d6ef3 Compare August 25, 2020 14:56

andy31415 requested a review from mspang August 25, 2020 15:25

mspang reviewed Aug 25, 2020

View reviewed changes

kedars marked this pull request as draft August 25, 2020 17:22

kedars force-pushed the esp32_gn_support branch from 8d5525c to bc51c1c Compare August 26, 2020 07:22

restyled-io bot mentioned this pull request Aug 26, 2020

Restyle WIP: ESP32 gn support #2333

Closed

kedars added 4 commits August 27, 2020 10:19

Fix tests - III

c3b66a4

Fix tests - IV

6464896

Fix tests - V

03022d2

restyle fixes

eef2d25

kedars added 2 commits August 27, 2020 14:06

Revert "Fix tests" based commits

f005571

Revert "Address tests"

40b537c

This reverts commit 8b76957.

kedars changed the title ~~WIP: ESP32 gn support~~ ESP32 gn support Aug 27, 2020

kedars added 2 commits August 27, 2020 17:34

Both build systems

cb14fd0

ESP32 GN build in CI

9a43217

boring-cyborg bot added github workflows labels Aug 27, 2020

rwalker-apple approved these changes Aug 29, 2020

View reviewed changes

restyle

e36e955

kedars requested a review from bzbarsky-apple as a code owner August 29, 2020 19:07

saurabhst approved these changes Aug 29, 2020

View reviewed changes

pass no args if is empty

c578d06

rwalker-apple merged commit 8617e2a into project-chip:master Aug 29, 2020

mspang added a commit to mspang/connectedhomeip that referenced this pull request Sep 16, 2020

Allow building CHIP without lwip checked out

230f012

This used to work prior to project-chip#2325, which added an unconditional dependency on lwip even if we're using sockets. Make it conditional.

mspang mentioned this pull request Sep 16, 2020

Allow building CHIP without lwip checked out #2683

Merged

mspang added a commit to mspang/connectedhomeip that referenced this pull request Sep 16, 2020

Allow building CHIP without lwip checked out

95299a6

This used to work prior to project-chip#2325, which added an unconditional dependency on lwip even if we're using sockets. Make it conditional.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ESP32 gn support #2325

ESP32 gn support #2325

kedars commented Aug 25, 2020 •

edited

Loading

andy31415 commented Aug 25, 2020

mspang Aug 25, 2020

kedars Aug 26, 2020

mspang Aug 25, 2020

mspang Aug 25, 2020

mspang Aug 25, 2020

kedars Aug 26, 2020

mspang Aug 25, 2020

kedars Aug 26, 2020

mspang Aug 25, 2020

kedars Aug 26, 2020

mspang Aug 26, 2020 •

edited

Loading

mspang Aug 25, 2020

kedars Aug 26, 2020

github-actions bot commented Aug 27, 2020

github-actions bot commented Aug 27, 2020

github-actions bot commented Aug 27, 2020

github-actions bot commented Aug 27, 2020

github-actions bot commented Aug 27, 2020

kedars commented Aug 27, 2020

kedars commented Aug 27, 2020

woody-apple commented Aug 28, 2020

rwalker-apple commented Aug 29, 2020

ESP32 gn support #2325

ESP32 gn support #2325

Conversation

kedars commented Aug 25, 2020 • edited Loading

Problem

Summary of Changes

andy31415 commented Aug 25, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mspang Aug 26, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Aug 27, 2020

github-actions bot commented Aug 27, 2020

github-actions bot commented Aug 27, 2020

github-actions bot commented Aug 27, 2020

github-actions bot commented Aug 27, 2020

kedars commented Aug 27, 2020

kedars commented Aug 27, 2020

woody-apple commented Aug 28, 2020

rwalker-apple commented Aug 29, 2020

kedars commented Aug 25, 2020 •

edited

Loading

mspang Aug 26, 2020 •

edited

Loading