Moving OVMS to latest ESP-IDF framework

[markwj]

This issue will be used to track the possibility / progress of moving OVMS to the latest ESP IDF framework.

[markwj]

Original (now closed, as unresolvable) issue was here:

Problems when trying to update esp-idf and toolchain to version 8.2 (the latest) #263

[leres]

(After a lot of work) I have built the espressif crosstool-NG esp-2020r1 release version of the toolchain for my chosen dev environment (FreeBSD).

Is it time to create branches of openvehicles/Open-Vehicle-Monitoring-System-3 and openvehicles/esp-idf to give us a place to work? I'd assume we want to target the esp-idf v4.0 release?

[leres]

I took a run at this today. I used the esp-idf v4.0 branch.

I created a pull request for two easy problems. One is that esp32wifi::SetPowerMode() is using WIFI_PS_MODEM which is deprecated. In the current esp-idf-espressif version this is a define to WIFI_PS_MIN_MODEM. The define is gone by v4.0.

The other problem was a conflict with the spi_signal_conn_t struct definition in components/spinodma/spi_master_nodma.c. The versions of spi_signal_conn_t in the esp-idf-espressif and v4.0 of soc/spi_periph.h look the same and are similar to what's in spi_master_nodma.c with members renamed (e.g. spiclk_native becomes spiclk_iomux_pin).

There is a lot of churn resulting in code ifdef'ed off of ESP_DNS in components/lwip/port/esp32/include/lwipopts.h. This changes dns_getserver() from returning a pointer to a const ip_addr_t to returning a ip_addr_t. You can't just turn off ESP_DNS because it breaks some ESP_LWIP code (that really should be tied to ESP_DNS). I decided against including these changes in my pull request because it involves changing the esp-idf and I wanted my pull to be conservative.

strverscmp() is in the standard libraries with gcc8 and conflicts with components/strverscmp.

There are a bunch of gcc8 errors in the version of wolfssl we're using related to the len parameter to xstrncpy() and friends. This is supposed to indicate available space in the dst buffer. But the code is not using it that way (and it looks like their are many opportunities for buffer overflow!) For example:

XSTRNCPY(header, BEGIN_CERT, headerLen);
XSTRNCAT(header, "\n", 1);
[...]
XSTRNCAT(header, "Proc-Type: 4,ENCRYPTED\n", 23);
XSTRNCAT(header, "DEK-Info: ", 10);

where headerLen is set to the size of the buffer initially I dunno what is up with the rest of the calls.

I looked at the current version of wolfssl and the code looks better but I suspect gcc8 will still flag remaining problems.

Boot::Boot() in main/ovms_boot.cpp sets up an error handler that v4.0 doesn't have:

// install error handler:
xt_set_error_handler_callback(ErrorCallback);

It looks like Fabrizio ran into this and opened an issue with espressif.

When I stopped I was getting linker errors for undefined symbols, e.g. mbuf_init, mbuf_free, etc. I didn't find these in the v4.0 esp-idf.

[dexterbg]

Regarding the panic callback, see my post:
espressif/esp-idf#5163 (comment)

The mbuf_init etc. calls are mongoose related. See:
https://github.com/openvehicles/mongoose/blob/master/mongoose.c#L1547
Note these also are also different in our fork, as my change to these regarding thread safety hasn't been merged upstream yet.

gcc8 does a good job at detecting possible buffer issues, it's definitely a good idea not to ignore the warnings.

[leres]

I was going to do some more work on this; should I be targeting v4.0.1 (released three weeks ago) or v4.1 (beta2 pre-released a week ago).

[fowi4hjte]

There are many CVE issues that have been fixed since the unsupported IDF 3.3 the OVMS is using currently: https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=esp
Its really important to have latest security fixes to a device that have 24/7 access to the CAN bus of the car and can open the doors.