ESP32 - Upgrade CHIP to ESP-IDF release v4.1

[dhrishi]

Problem

Upgrade CHIP to latest ESP-IDF release v4.1 from v4.0 for ESP32 platform

ESP-IDF release v4.1 is released a few days back which has updates over release v4.0. More details about the new features, critical bug fixes etc. can be found here

Documentation for ESP-IDF release v4.1 is here

Summary of Changes

Major changes:

1. Replace tcpip_adapter (now deprecated) with new esp_netif API to manage network interfaces
2. Modifications in event loop as it is now based on esp_event library

ToDo

• Submit a PR to https://github.com/jeremyjh/ESP32_TFT_library.git repository for changes in tft_demo example and update the corresponding submodule pointer in this or a subsequent PR

[CLAassistant]

All committers have signed the CLA.

[rwalker-apple]

is your TODO a prerequisite for this landing? doesn't appear to be...

[dhrishi]

is your TODO a prerequisite for this landing? doesn't appear to be...

@rwalker-apple Not really. The example is present in this repository and hence thought of updating it as well. I already have a PR ready and will submit it there.

[rwalker-apple]

is your TODO a prerequisite for this landing? doesn't appear to be...

@rwalker-apple Not really. The example is present in this repository and hence thought of updating it as well. I already have a PR ready and will submit it there.

ok, super. I think we just need to address the CI failures, then.

[sagar-apple]

So CI seems to be failing because this header can't be found. I think the Docker Images CI uses need to be updated.

[dhrishi]

@sagar-apple That's right. esp-netif.h header is added in ESP-IDF release v4.1 and was not present in v4.0

[sagar-apple]

@rwalker-apple whats the right order to fix this in? Does he need to update the dockerfile in this PR and then publish the images?

[rwalker-apple]

yeah, Dockerfile, Docker push, rebase :-(

@dhrishi, if you're ok with me pushing to your PR, I can effect the changes

[dhrishi]

@rwalker-apple Sure, no worries

[rwalker-apple]

@dhrishi we're close. here are some of the test failures from the QEMU tests:

/Users/rwalker/dev/chip/src/inet/tests/TestInetAddress.cpp:1023: assertion failed: "!memcmp(&ip_addr_1, &ip_addr_2, sizeof(ip_addr_1))"
/Users/rwalker/dev/chip/src/inet/tests/TestInetAddress.cpp:1023: assertion failed: "!memcmp(&ip_addr_1, &ip_addr_2, sizeof(ip_addr_1))"
/Users/rwalker/dev/chip/src/inet/tests/TestInetAddress.cpp:1023: assertion failed: "!memcmp(&ip_addr_1, &ip_addr_2, sizeof(ip_addr_1))"
/Users/rwalker/dev/chip/src/inet/tests/TestInetAddress.cpp:1023: assertion failed: "!memcmp(&ip_addr_1, &ip_addr_2, sizeof(ip_addr_1))"
/Users/rwalker/dev/chip/src/inet/tests/TestInetAddress.cpp:1023: assertion failed: "!memcmp(&ip_addr_1, &ip_addr_2, sizeof(ip_addr_1))"
/Users/rwalker/dev/chip/src/inet/tests/TestInetAddress.cpp:1023: assertion failed: "!memcmp(&ip_addr_1, &ip_addr_2, sizeof(ip_addr_1))"
[ inet-address : Convert IPAddress to IPv6                  ] : FAILED

[rwalker-apple]

more digging reveals that esp-if's version of lwip has an ip6_addr_t looks like this:

struct ip6_addr {
  u32_t addr[4];
#if LWIP_IPV6_SCOPES
  u8_t zone;
#endif /* LWIP_IPV6_SCOPES */
};

CHIP isn't expecting that. Looks to be written to expect:

struct ip6_addr {
  u32_t addr[4];
};

@gerickson you might find this interesting

I've turned off that feature of lwip for the esp32 builds

[dhrishi]

@rwalker-apple Thanks for debugging and identifying the actual issue. I was not able to see the detailed failure logs in CI (like the ones you pasted above). Are the failure logs uploaded somewhere? If yes, can you please point me to them, for any subsequent issues?

[sagar-apple]

@kedars Is there a way to perform this configuration via the sdkconfig files? Seems very not esp-idf like to do it this way.

[dhrishi]

@sagar-apple IMO, the option LWIP_IPV6_SCOPES should not be disabled, neither through CFLAGS nor sdkconfig. The reasons being:

1. LWIP mentions that this option is to be disabled only for single-interface configurations, see here
2. ESP-IDF esp-netif layer, the newly added one in v4.1, assumes that zone (introduced when LWIP_IPV6_SCOPES is enabled) is present in the IPv6 address

With the current delta in LWIP for ESP-IDF and CHIP, I think we can modify the test memcmp statement to just compare member addr of ip6_addr_t instead. Thoughts?

[sagar-apple]

Yeah I guess we could update the test instead. Since @rwalker-apple has looked at this test already maybe there's a reason he opted to not do it that way?

[rwalker-apple]

Modifying the memcmp() is insufficient because CHIP thinks ip6_addr_t is 16 bytes, allocates 16 bytes for it (as Addr[] in IPAddress) and routinely moves the memory as *(ip6_addr_t *)&Addr, which moves 20 bytes. This can lead to memory corruption or a crash.

My change is a hack, needs to be addressed with a more complete fix if we are planning to use LWIP_IPV6_SCOPES, which looks to me like a breaking change on LWIP's part...

[rwalker-apple]

@rwalker-apple Thanks for debugging and identifying the actual issue. I was not able to see the detailed failure logs in CI (like the ones you pasted above). Are the failure logs uploaded somewhere? If yes, can you please point me to them, for any subsequent issues?

Logs aren't available from CI, I had to diagnose locally. #2585

[rwalker-apple]

@pan-apple @kedars @dhrishi I think your help is needed in order to get qemu building. The new tooling seems to confuse our qemu test builder. We are ending up with something with no boot partitions:

The below is from running transport tests

+ /Users/rwalker/dev/chip/scripts/tools/esp32_qemu_run.sh /tmp/tmp.ysq7GSAevS
Adding SPI flash device
==256==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
ets Jun  8 2016 00:22:57

rst:0x1 (POWERON_RESET),boot:0x12 (SPI_FAST_FLASH_BOOT)
configsip: 0, SPIWP:0xee
clk_drv:0x00,q_drv:0x00,d_drv:0x00,cs0_drv:0x00,hd_drv:0x00,wp_drv:0x00
mode:DIO, clock div:2
load:0x3fff0030,len:4
load:0x3fff0034,len:6908
load:0x40078000,len:13720
load:0x40080400,len:3880
entry 0x40080680
�[0;32mI (433) boot: ESP-IDF v4.1 2nd stage bootloader�[0m
�[0;32mI (449) boot: compile time 19:41:31�[0m
�[0;32mI (464) boot: chip revision: 0�[0m
�[0;32mI (478) boot.esp32: SPI Speed      : 40MHz�[0m
�[0;32mI (482) boot.esp32: SPI Mode       : DIO�[0m
�[0;32mI (485) boot.esp32: SPI Flash Size : 2MB�[0m
�[0;32mI (507) boot: Enabling RNG early entropy source...�[0m
�[0;32mI (539) boot: Partition Table:�[0m
�[0;32mI (541) boot: ## Label            Usage          Type ST Offset   Length�[0m
�[0;32mI (548) boot:  0 nvs              WiFi data        01 02 00009000 00006000�[0m
�[0;32mI (554) boot:  1 phy_init         RF data          01 01 0000f000 00001000�[0m
�[0;32mI (557) boot:  2 factory          factory app      00 00 00010000 00100000�[0m
�[0;32mI (567) boot: End of partition table�[0m
�[0;32mI (586) esp_image: segment 0: paddr=0x00010020 vaddr=0x3f400020 size=0x31430 (201776) map�[0m
�[0;32mI (1232) esp_image: segment 1: paddr=0x00041458 vaddr=0x3ffbdb60 size=0x03e1c ( 15900) load�[0m
�[0;32mI (1309) esp_image: segment 2: paddr=0x0004527c vaddr=0x40080000 size=0x00400 (  1024) load�[0m
�[0;32mI (1342) esp_image: segment 3: paddr=0x00045684 vaddr=0x40080400 size=0x0a994 ( 43412) load�[0m
�[0;32mI (1517) esp_image: segment 4: paddr=0x00050020 vaddr=0x400d0020 size=0xc338c (799628) map�[0m
�[0;32mI (4016) esp_image: segment 5: paddr=0x001133b4 vaddr=0x4008ad94 size=0x0e138 ( 57656) load�[0m
�[0;31mE (4218) esp_image: Image length 1119504 doesn't fit in partition length 1048576�[0m
�[0;31mE (4226) boot: Factory app partition is not bootable�[0m
�[0;31mE (4230) boot: No bootable app partitions in the partition table�[0m

[dhrishi]

@rwalker-apple Looks like you fixed the Qemu related issue? Thanks!