Skip to content

[WIP] esp32/Makefile: Move to v4.1 #6413

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Conversation

osctobe
Copy link

@osctobe osctobe commented Sep 5, 2020

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 .

Copy link
Contributor

@tve tve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this is a great start! I did have a couple of questions, though...

@@ -260,7 +265,7 @@ CFLAGS_COMMON = -Os -ffunction-sections -fdata-sections -fstrict-volatile-bitfie
-Wno-error=unused-variable -Wno-error=deprecated-declarations \
-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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please explain the need for this change?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$(ECHO) "GEN $@"
$(Q)$(MKDIR) -p $(dir $@)
$(Q)$(PYTHON) $(ESPIDF)/tools/kconfig_new/confgen.py \
$(Q)$(PYTHON2) $(ESPIDF)/tools/kconfig_new/confgen.py \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change, seems something more related to your build env as opposed to esp-idf v4.0.1 vs. v4.1?

@@ -562,7 +577,7 @@ ESPIDF_BT_NIMBLE_O = $(patsubst %.c,%.o,\
endif

$(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)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this have to be excluded?

@@ -89,7 +91,7 @@ else ifeq ($(ESPIDF_CURHASH),$(ESPIDF_SUPHASH_V4))
$(info Building with ESP IDF v4)

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)),)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@dpgeorge
Copy link
Member

@osctobe thanks for the contribution here. Are you able to keep working on this, or shall someone else pick it up?

@pumelo
Copy link

pumelo commented Sep 18, 2020

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?
If built as a component, this would lift the burden to maintain linker scripts and compile flags in micropython for esp-idf components.

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.

@dpgeorge
Copy link
Member

Is there a specific reason that micropython does not build as a component to esp-idf?

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.

If built as a component, this would lift the burden to maintain linker scripts and compile flags in micropython for esp-idf components.

I agree.

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.

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.

@dpgeorge
Copy link
Member

See #6906 .

@dpgeorge
Copy link
Member

ESP IDF v4.1.1 is now supported, as of d191d88

@dpgeorge dpgeorge closed this Feb 15, 2021
tannewt pushed a commit to tannewt/circuitpython that referenced this pull request May 23, 2022
…n-main

Translations update from Hosted Weblate
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants