Skip to content

esp32/Makefile: Include all driver/*.c in build. Fixes #4869#4874

Closed
nevercast wants to merge 1 commit intomicropython:masterfrom
nevercast:feature/ports-esp32-all-drivers
Closed

esp32/Makefile: Include all driver/*.c in build. Fixes #4869#4874
nevercast wants to merge 1 commit intomicropython:masterfrom
nevercast:feature/ports-esp32-all-drivers

Conversation

@nevercast
Copy link
Copy Markdown
Contributor

@nevercast nevercast commented Jun 25, 2019 •
edited
Loading

Small change to import all drivers in the ESP32 port. This allows writing modules, internally or externally with the cmodules functionality that can use these ESP32 drivers.

For me personally, this allows me to write a cmodule that uses the I2S peripheral without modifying the Makefile.

A snippet of the make process showing the files included:

CC /builder/esp-idf/components/driver/spi_master.c
CC /builder/esp-idf/components/driver/sigmadelta.c
CC /builder/esp-idf/components/driver/uart.c
CC /builder/esp-idf/components/driver/sdspi_transaction.c
CC /builder/esp-idf/components/driver/sdio_slave.c
CC /builder/esp-idf/components/driver/i2c.c
CC /builder/esp-idf/components/driver/periph_ctrl.c
CC /builder/esp-idf/components/driver/ledc.c
CC /builder/esp-idf/components/driver/sdmmc_transaction.c
CC /builder/esp-idf/components/driver/sdspi_host.c
CC /builder/esp-idf/components/driver/pcnt.c
CC /builder/esp-idf/components/driver/sdspi_crc.c
CC /builder/esp-idf/components/driver/timer.c
CC /builder/esp-idf/components/driver/rmt.c
CC /builder/esp-idf/components/driver/mcpwm.c
CC /builder/esp-idf/components/driver/spi_common.c
CC /builder/esp-idf/components/driver/can.c
CC /builder/esp-idf/components/driver/spi_slave.c
CC /builder/esp-idf/components/driver/gpio.c
CC /builder/esp-idf/components/driver/sdmmc_host.c
CC /builder/esp-idf/components/driver/i2s.c
CC /builder/esp-idf/components/driver/rtc_module.c
AR build/esp-idf/driver/libdriver.a

Here is a screenshot of me initializing the I2S hardware via MicroPython, possible due to this change. (Python & CModule library not provided)
Screenshot from 2019-06-25 13-37-25

Discussion and issue initially raised here: #4869

@nevercast nevercast changed the title ports/esp32: Include all driver/*.c in build. Fixes #4869 esp32/Makefile: Include all driver/*.c in build. Fixes #4869 Jun 25, 2019
@nevercast
Copy link
Copy Markdown
Contributor Author

nevercast commented Jun 25, 2019

It could be an improvement that more directories automatically include their .c/.o files. Not just ESPIDF_DRIVER, but I felt that was out of scope for this change. I want to keep this PR small since its blocking me downstream from being synced to #master.

We can discuss making this change more generic in a follow up issue, if that is of interest.

dpgeorge
dpgeorge reviewed Jun 25, 2019
View reviewed changes
Comment thread ports/esp32/Makefile Outdated
Copy link
Copy Markdown
Member

@dpgeorge dpgeorge Jun 25, 2019

Choose a reason for hiding this comment

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

How about combining this into one declaration, to avoid the need to define ESPIDF_DRIVER_C?

Copy link
Copy Markdown
Contributor Author

@nevercast nevercast Jun 25, 2019

Choose a reason for hiding this comment

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

Honestly, I don't know how to get make to do that in one line. It seems to evaluate the replace before the wildcard, and just breaks.

If you know how, shoot. Otherwise, I'll look in to it further.

Copy link
Copy Markdown
Member

@dpgeorge dpgeorge Jun 25, 2019

Choose a reason for hiding this comment

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

Try using patsubst, see py/py.mk for examples.

Copy link
Copy Markdown
Contributor Author

@nevercast nevercast Jun 25, 2019

Choose a reason for hiding this comment

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

Its one line now.

Copy link
Copy Markdown
Contributor Author

@nevercast nevercast Jun 25, 2019

Choose a reason for hiding this comment

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

patsubst needs the whole path to be satisfied which elongates the line by a substantial amount, even with the $(ESPCOMP) variable. subst, though not as safe, seems perfectly sufficient.

Copy link
Copy Markdown
Member

@dpgeorge dpgeorge Jun 25, 2019

Choose a reason for hiding this comment

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

Yes it looks ok with subst

@nevercast nevercast force-pushed the feature/ports-esp32-all-drivers branch 2 times, most recently from e2610c5 to 49f2b09 Compare June 25, 2019 03:32
@nevercast nevercast force-pushed the feature/ports-esp32-all-drivers branch from 49f2b09 to 3a6fc47 Compare June 25, 2019 03:36
@nevercast
Copy link
Copy Markdown
Contributor Author

nevercast commented Jun 25, 2019

Whoops, misunderstanding regarding real name and real email. This PR should be all good.

@nevercast
Copy link
Copy Markdown
Contributor Author

nevercast commented Jun 25, 2019

A last minute thought, do you want me to rebase this on top of your travis changes so that we get an ESP32 build confirmation?

@dpgeorge
Copy link
Copy Markdown
Member

dpgeorge commented Jun 25, 2019

do you want me to rebase this on top of your travis changes so that we get an ESP32 build confirmation?

No it's ok, I'll test and rebase during merging.

@nevercast
Copy link
Copy Markdown
Contributor Author

nevercast commented Jun 25, 2019

No it's ok, I'll test and rebase during merging.

Great, thanks.

@dpgeorge
Copy link
Copy Markdown
Member

dpgeorge commented Jun 25, 2019

Thanks for the work, merged in 2f262d5

@dpgeorge dpgeorge closed this Jun 25, 2019
@nevercast nevercast deleted the feature/ports-esp32-all-drivers branch June 25, 2019 05:28
@nevercast
Copy link
Copy Markdown
Contributor Author

nevercast commented Jun 25, 2019

Thanks for reviewing and merging, I'm happy to say we are now tracking micropython#master again 👍

@dpgeorge dpgeorge mentioned this pull request Jul 11, 2019
Closed
tannewt added a commit to tannewt/circuitpython that referenced this pull request Jul 21, 2021
@tannewt
Fix Arduino RP2040 flash size
109bd28 
For RP2040 boards, we now change the default flash size based on
the configured flash. We will still try to read the size from the
flash first.

Fixes micropython#4874
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dpgeorge dpgeorge dpgeorge left review comments

Assignees

No one assigned

Labels

port-esp32

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@nevercast @dpgeorge