Repository navigation
Conversation
tve
left a comment
There was a problem hiding this comment.
Thanks, this is a great start! I did have a couple of questions, though...
| -DESP_PLATFORM | ||
|
|
||
| CFLAGS_BASE = -std=gnu99 $(CFLAGS_COMMON) -DMBEDTLS_CONFIG_FILE='"mbedtls/esp_config.h"' -DHAVE_CONFIG_H | ||
| CFLAGS_BASE = -std=gnu11 $(CFLAGS_COMMON) -DMBEDTLS_CONFIG_FILE='"mbedtls/esp_config.h"' -DHAVE_CONFIG_H |
There was a problem hiding this comment.
Can you please explain the need for this change?
There was a problem hiding this comment.
I think this compile flag is only required for one file:
| $(ECHO) "GEN $@" | ||
| $(Q)$(MKDIR) -p $(dir $@) | ||
| $(Q)$(PYTHON) $(ESPIDF)/tools/kconfig_new/confgen.py \ | ||
| $(Q)$(PYTHON2) $(ESPIDF)/tools/kconfig_new/confgen.py \ |
There was a problem hiding this comment.
Why this change, seems something more related to your build env as opposed to esp-idf v4.0.1 vs. v4.1?
|
|
||
| $(BUILD)/$(ESPCOMP)/esp_eth/src/esp_eth_mac_dm9051.o: CFLAGS += -fno-strict-aliasing | ||
| ESPIDF_ESP_ETH_O = $(patsubst %.c,%.o,$(wildcard $(ESPCOMP)/esp_eth/src/*.c)) | ||
| ESPIDF_ESP_ETH_O = $(patsubst %.c,%.o,$(filter-out %_openeth.c,$(wildcard $(ESPCOMP)/esp_eth/src/*.c))) |
There was a problem hiding this comment.
Why does this have to be excluded?
|
|
||
| PYPARSING_VERSION = $(shell python3 -c 'import pyparsing; print(pyparsing.__version__)') | ||
| ifneq ($(PYPARSING_VERSION),2.3.1) | ||
| ifeq ($(filter 2.3% 2.2% 2.1%,$(PYPARSING_VERSION)),) |
There was a problem hiding this comment.
Is it really a good idea to allow all these older versions? Did you test against each one? Do we really want to test against each one?
|
@osctobe thanks for the contribution here. Are you able to keep working on this, or shall someone else pick it up? |
|
This is more of a general question than related to the upgrade to v4.1: Is there a specific reason that micropython does not build as a component to esp-idf? See e.g. that the compile flags are changed for only one file https://github.com/espressif/esp-idf/blob/94cb7e8b8f6da8abf2ca5c6117c81eaac92d90ee/components/driver/component.mk#L11. I know that there have been discussions about moving the esp32 port to cmake, but that's not what I'm talking about. As long as esp-idf does support the "legacy make" build system this should work quite well. I have a working poc which builds micropython as a component to esp-idf with quite minimal change to the micropython code base. It uses the approch described here https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-guides/build-system-legacy.html#fully-overriding-the-component-makefile. I'll be happy to share this poc if interest is available. |
Because it was done that way from the beginning, to have full control over the build, which is mainly to get the qstr generation done correctly. It also makes it more like other ports. But these are not strong reasons to do it that way.
I agree.
The legacy make support in the IDF may disappear in the future, so I'd say it's better to just go straight to cmake if anything. See #6473 for a PR to do that. |
|
See #6906 . |
|
ESP IDF v4.1.1 is now supported, as of d191d88 |
…n-main Translations update from Hosted Weblate
Fixup build system for ESP-IDF v4.1. This only makes SDK parts build, there are API changes that need adjusting to in the code.
Issue #6412 .