Skip to content
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

Support for ESP32 #5

Open
elliotwoods opened this issue Jul 8, 2020 · 39 comments
Open

Support for ESP32 #5

elliotwoods opened this issue Jul 8, 2020 · 39 comments

Comments

@elliotwoods
Copy link

Hey there

I noticed issue #3 , however that issue is closed and yet I don't think that support for ESP32 is resolved, so I'm adding a new issue here. If indeed this does work on ESP32, then this might become an instructive log of how to do that. Otherwise, I hope we can find the issues :).

When simply trying to call make MICROPYTHON_PORT_DIR=../micropython/ports/esp32 we get:

make -C ../micropython/ports/esp32 BUILD=build MICROPY_PY_BTREE=0 MICROPY_PY_FFI=0 MICROPY_PY_USSL=0 MICROPY_PY_AXTLS=0 MICROPY_FATFS=0 MICROPY_PY_THREAD=0 USER_C_MODULES=/mnt/c/dev/micropython-wrap CFLAGS_EXTRA="-DMODULE_UPYWRAPTEST_ENABLED=1 -DMICROPY_MODULE_BUILTIN_INIT=1" build/genhdr/qstrdefs.generated.h
make[1]: Entering directory '/mnt/c/dev/micropython/ports/esp32'
Use make V=1 or set BUILD_VERBOSE in your environment to increase build verbosity.
Including User C Module from /mnt/c/dev/micropython-wrap/tests
Building with ESP IDF v4
fatal: No names found, cannot describe anything.
../../py/mkrules.mk:80: *** mixed implicit and normal rules: deprecated syntax
../../py/mkrules.mk:116: *** mixed implicit and normal rules: deprecated syntax
make[1]: *** No rule to make target '%.c,', needed by 'build/genhdr/qstr.i.last'.  Stop.
make[1]: Leaving directory '/mnt/c/dev/micropython/ports/esp32'
Makefile:49: recipe for target 'staticlib' failed
make: *** [staticlib] Error 2

when trying with sharedlib it will start to complain about missing includes.
It's easy enough to add these to the include path.
After that we then receive many many many errors:
https://gist.github.com/elliotwoods/c2378981fdb1b154b37d111f58eaaa4c
(This may be what you're referring to with but the problem is dynruntime.h does not yet implement the complete API and as such micropython-wrap won't compile with it.)

In my case I'm just trying to build the tests first and then will try with my own code.

I've also made an issue over at:
micropython/micropython#6233

Thank you
Elliot

@stinos
Copy link
Owner

stinos commented Jul 8, 2020

I have close to zero experience with ESP32 and it's build, but at first sight the errors in that gist seem to originate from the ESP headers not being valid C++. But I'm not sure if it makes any sense trying to build a shared library for ESP: does it support dynamic loading? I.e. does it have a working implementation of dlfcn.h?

That doesn't explain the problem with the staticlib target though, but I also don't know a lot about about Makefiles so cannot decipher that error. If you're genuinely interested in getting this to work, I can have a look into it though. But you'll have to help me out a bit then: how do I setup a build environment for ESP32?

@elliotwoods
Copy link
Author

elliotwoods commented Jul 8, 2020

@stinos - thank you so much for offering to help

I'm unsure whether ESP32 supports dynamic linking
There is no dlfcn.h file within the esp32 port folder

I'd be very happy to help you get set up with ESP-IDF for testing micropython-wrap with it
I don't want to take up your time though

Which platform are you building on? I'm testing here on Ubuntu (WSL)

@stinos
Copy link
Owner

stinos commented Jul 8, 2020

dlfcn.h is a system header i.e. comes with compiler and/or OS, it's not part of MicroPython: https://linux.die.net/man/3/dlopen

I can build on WSL or standard Ubuntu, anything which works. I missed your pevious comment wrt SRC_USERMOD_CPP though; so please lay out as detailed as possible what your build environment looks like and what you utlimately want to do (I assume wrapping some C++ library? Which one?). I'd be happy to spend some time on this because I have a feeling that if it works for ESP32 most problems for other microcontroller ports will also be solved. However, I don't exactly have a lot of time so cannot make promises when that will happen :)

@elliotwoods
Copy link
Author

Ah I'm sorry that comment was on the wrong issue. I'll remove it here

The c++ library which I'm trying to wrap is Tensorflow Lite Micro for inference locally on the ESP32
My intention is to implement:

  • Load a model from a binary string in python (tensorflow can load from an unsigned char array)
  • Pass an array of ints into the model and return an array of ints

There are notes on how this code looks at:
https://www.tensorflow.org/lite/microcontrollers/get_started

So I presume something like

module tensorflow {
    class model {
        void load(binary string)
        list_of_floats invoke(list_of_floats)
        list_of_ints invoke(list_of_ints)
    }
}

To build for esp32 get the correct version of esp-idf running:

git clone https://github.com/espressif/esp-idf.git --depth 1
cd esp-idf
git pull origin 4c81978a3e2220674a432a588292a4c860eef27b
git checkout 4c81978a3e2220674a432a588292a4c860eef27b
git submodule init
git submodule update --recursive
./install.sh
source ./export.sh

esp-idf paths will now be correctly set (just repeat the final step there next time you start a new bash session)

build the mpy-cross compiler (can't remember where you do this)

make -C mpy-cross

now if you go to the micropython/ports/esp32 folder, make should work

@elliotwoods
Copy link
Author

btw .. dlfcn.h is not a part of the esp-idf repo or any of its submodules

@stinos
Copy link
Owner

stinos commented Aug 25, 2020

@elliotwoods was looking into this, but as you figured it does require changes to the core makefiles to get C++ code compiled (for the unix port I'm circumventing that because the compilation is fairly simple and requires only a couple of extra flags/include directories - in other words, the Makefile here is really only for the unix port or something similar enough). Moreover I cannot even build the example user C module per the instructions in cmodules.rst, so that makes it pretty hard to do much for this issue.

However I did build the test code (module.cpp) into an ESP32 application by adding the file manually, so my initial idea ('if you have a C++ compiler it should work') still seems to hold. Didn't test it as I don't have an ESP32 though. Diff against current master:

diff --git a/ports/esp32/Makefile b/ports/esp32/Makefile
index 2cbe9f6be..ad8942770 100644
--- a/ports/esp32/Makefile
+++ b/ports/esp32/Makefile
@@ -256,7 +256,7 @@ endif
 # these flags are common to C and C++ compilation
 CFLAGS_COMMON = -Os -ffunction-sections -fdata-sections -fstrict-volatile-bitfields \
        -mlongcalls -nostdlib \
-       -Wall -Werror -Wno-error=unused-function -Wno-error=unused-but-set-variable \
+       -Wall -Wno-error=unused-function -Wno-error=unused-but-set-variable \
        -Wno-error=unused-variable -Wno-error=deprecated-declarations \
        -DESP_PLATFORM

@@ -271,7 +271,7 @@ CFLAGS += -DMICROPY_ESP_IDF_4=1
 endif

 # this is what ESPIDF uses for c++ compilation
-CXXFLAGS = -std=gnu++11 $(CFLAGS_COMMON) $(INC) $(INC_ESPCOMP)
+CXXFLAGS = -std=gnu++11 $(CFLAGS_COMMON) $(INC) $(INC_ESPCOMP) -I$(BOARD_DIR)

 LDFLAGS = -nostdlib -Map=$(@:.elf=.map) --cref
 LDFLAGS += --gc-sections -static -EL
@@ -379,6 +379,7 @@ DRIVERS_SRC_C = $(addprefix drivers/,\
 OBJ_MP =
 OBJ_MP += $(PY_O)
 OBJ_MP += $(addprefix $(BUILD)/, $(SRC_C:.c=.o))
+OBJ_MP += $(addprefix $(BUILD)/, $(SRC_CPP:.cpp=.o))
 OBJ_MP += $(addprefix $(BUILD)/, $(EXTMOD_SRC_C:.c=.o))
 OBJ_MP += $(addprefix $(BUILD)/, $(LIB_SRC_C:.c=.o))
 OBJ_MP += $(addprefix $(BUILD)/, $(DRIVERS_SRC_C:.c=.o))
@@ -387,7 +388,7 @@ OBJ_MP += $(addprefix $(BUILD)/, $(DRIVERS_SRC_C:.c=.o))
 $(OBJ_MP): CFLAGS += -Wdouble-promotion -Wfloat-conversion

 # List of sources for qstr extraction
-SRC_QSTR += $(SRC_C) $(EXTMOD_SRC_C) $(LIB_SRC_C) $(DRIVERS_SRC_C)
+SRC_QSTR += $(SRC_C) $(SRC_CPP) $(EXTMOD_SRC_C) $(LIB_SRC_C) $(DRIVERS_SRC_C)
 # Append any auto-generated sources that are needed by sources listed in SRC_QSTR
 SRC_QSTR_AUTO_DEPS +=

diff --git a/ports/esp32/main.c b/ports/esp32/main.c
index 6f9ab82d0..5bf8aa830 100644
--- a/ports/esp32/main.c
+++ b/ports/esp32/main.c
@@ -118,6 +118,9 @@ soft_reset:
     // initialise peripherals
     machine_pins_init();

+    mp_obj_module_t* mod = init_upywraptest();
+    mp_module_register(qstr_from_str("upywraptest"), mod);
+
     // run boot-up scripts
     pyexec_frozen_module("_boot.py");
diff --git a/py/makeqstrdefs.py b/py/makeqstrdefs.py
index 9449a46ee..fad9083cf 100644
--- a/py/makeqstrdefs.py
+++ b/py/makeqstrdefs.py
@@ -44,7 +44,7 @@ def process_file(f):
             m = re_line.match(line)
             assert m is not None
             fname = m.group(1)
-            if not fname.endswith(".c"):
+            if not fname.endswith(".c") and not fname.endswith(".cpp"):
                 continue

Then build with

make SRC_CPP=../../../micropython-wrap/tests/module.cpp

Let me know if this is useful.

@stinos
Copy link
Owner

stinos commented Aug 26, 2020

@elliotwoods ok today, out of nowhere, I could build 'user C modules' again so here's an alternative solution:

  • move module.c to cmodule.c (because the build otherwise creates module.o for both of them - that's not an uncommon problem so I'll rename the file and commit that change)
  • adjust micropython.mk to:
EXAMPLE_MOD_DIR := $(USERMOD_DIR)
SRC_USERMOD += $(EXAMPLE_MOD_DIR)/cmodule.c
SRC_USERMOD_CPP += $(EXAMPLE_MOD_DIR)/module.cpp
LDFLAGS_USERMOD += -lstdc++
  • change the ESP32 Makefile so it builds C++ files:
diff --git a/ports/esp32/Makefile b/ports/esp32/Makefile
index 2cbe9f6be..b0c00e7b3 100644
--- a/ports/esp32/Makefile
+++ b/ports/esp32/Makefile
@@ -256,7 +256,7 @@ endif
 # these flags are common to C and C++ compilation
 CFLAGS_COMMON = -Os -ffunction-sections -fdata-sections -fstrict-volatile-bitfields \
        -mlongcalls -nostdlib \
-       -Wall -Werror -Wno-error=unused-function -Wno-error=unused-but-set-variable \
+       -Wall -Wno-error=unused-function -Wno-error=unused-but-set-variable \
        -Wno-error=unused-variable -Wno-error=deprecated-declarations \
        -DESP_PLATFORM

@@ -271,7 +271,7 @@ CFLAGS += -DMICROPY_ESP_IDF_4=1
 endif

 # this is what ESPIDF uses for c++ compilation
-CXXFLAGS = -std=gnu++11 $(CFLAGS_COMMON) $(INC) $(INC_ESPCOMP)
+CXXFLAGS = -std=gnu++11 $(CFLAGS_COMMON) $(INC) $(INC_ESPCOMP) -I$(BOARD_DIR)

 LDFLAGS = -nostdlib -Map=$(@:.elf=.map) --cref
 LDFLAGS += --gc-sections -static -EL
@@ -357,6 +357,9 @@ SRC_C = \
        $(wildcard $(BOARD_DIR)/*.c) \
        $(SRC_MOD)

+SRC_CPP += \
+       $(SRC_MOD_CPP)
+
 EXTMOD_SRC_C += $(addprefix extmod/,\
        modonewire.c \
        )
@@ -379,6 +382,7 @@ DRIVERS_SRC_C = $(addprefix drivers/,\
 OBJ_MP =
 OBJ_MP += $(PY_O)
 OBJ_MP += $(addprefix $(BUILD)/, $(SRC_C:.c=.o))
+OBJ_MP += $(addprefix $(BUILD)/, $(SRC_CPP:.cpp=.o))
 OBJ_MP += $(addprefix $(BUILD)/, $(EXTMOD_SRC_C:.c=.o))
 OBJ_MP += $(addprefix $(BUILD)/, $(LIB_SRC_C:.c=.o))
 OBJ_MP += $(addprefix $(BUILD)/, $(DRIVERS_SRC_C:.c=.o))
@@ -387,7 +391,7 @@ OBJ_MP += $(addprefix $(BUILD)/, $(DRIVERS_SRC_C:.c=.o))
 $(OBJ_MP): CFLAGS += -Wdouble-promotion -Wfloat-conversion

 # List of sources for qstr extraction
-SRC_QSTR += $(SRC_C) $(EXTMOD_SRC_C) $(LIB_SRC_C) $(DRIVERS_SRC_C)
+SRC_QSTR += $(SRC_C) $(SRC_CPP) $(EXTMOD_SRC_C) $(LIB_SRC_C) $(DRIVERS_SRC_C)
 # Append any auto-generated sources that are needed by sources listed in SRC_QSTR
 SRC_QSTR_AUTO_DEPS +=

@@ -796,7 +800,7 @@ $(Q)$(CXX) $(CXXFLAGS) -c -MD -o $@ $<
   $(RM) -f $(@:.o=.d)
 endef

-vpath %.cpp . $(TOP)
+vpath %.cpp . $(TOP) $(USER_C_MODULES)
 $(BUILD)/%.o: %.cpp
        $(call compile_cxx)

diff --git a/py/makeqstrdefs.py b/py/makeqstrdefs.py
index 9449a46ee..fad9083cf 100644
--- a/py/makeqstrdefs.py
+++ b/py/makeqstrdefs.py
@@ -44,7 +44,7 @@ def process_file(f):
             m = re_line.match(line)
             assert m is not None
             fname = m.group(1)
-            if not fname.endswith(".c"):
+            if not fname.endswith(".c") and not fname.endswith(".cpp"):
                 continue
             if fname != last_fname:
                 write_out(last_fname, output)
diff --git a/py/py.mk b/py/py.mk
index d864a7ed3..a0fa7a482 100644
--- a/py/py.mk
+++ b/py/py.mk
@@ -42,6 +42,7 @@ $(foreach module, $(wildcard $(USER_C_MODULES)/*/micropython.mk), \
 )

 SRC_MOD += $(patsubst $(USER_C_MODULES)/%.c,%.c,$(SRC_USERMOD))
+SRC_MOD_CPP += $(patsubst $(USER_C_MODULES)/%.cpp,%.cpp,$(SRC_USERMOD_CPP))
 CFLAGS_MOD += $(CFLAGS_USERMOD)
  • note I removed -Werror but that's solely to get rid of the #warning rom/ets_sys.h is deprecated, please use esp32/rom/ets_sys.h instead which I don't immediately know how to do otherwise
  • build with make USER_C_MODULES=../../../micropython-wrap CFLAGS_EXTRA="-DMODULE_UPYWRAPTEST_ENABLED=1"

@elliotwoods
Copy link
Author

I had some success building CPP also, but using custom build scripts to make the cpp files and some hacks of the existing micropython makefiles. Howeveer I didn't get it working with your build process / library

Sadly since i got this working I've moved onto something else so don't have much time right now to check your solution (reading through it here, i think i need to spend more time to fully understand it)

however i can test your firmware on my esp32 if you want me to test your module here runs correctly
do you have a binary i can try?

@elliotwoods
Copy link
Author

And thank you again for looking into this! I really like your library and hope to use it for future projects when I'm wrapping for micropython

@stinos
Copy link
Owner

stinos commented Aug 26, 2020

@elliotwoods ok, no problem. See files here. If you can run something like

import upywraptest
a = upywraptest.Simple(1)

it's good.

@elliotwoods
Copy link
Author

it fails to boot and keeps restarting
it seems the partitions are not correct

I'm trying with firmware.bin which is the standard thing i usually upload

E (679) esp_image: Image length 1669984 doesn't fit in partition length 1572864
E (679) boot: Factory app partition is not bootable
E (681) boot: No bootable app partitions in the partition table
ets Jun  8 2016 00:22:57

rst:0x3 (SW_RESET),boot:0x17 (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:0x3fff0018,len:4
load:0x3fff001c,len:5096
load:0x40078000,len:12856
load:0x40080400,len:3480
entry 0x40080638
E (679) esp_image: Image length 1669984 doesn't fit in partition length 1572864
E (679) boot: Factory app partition is not bootable
E (681) boot: No bootable app partitions in the partition table
ets Jun  8 2016 00:22:57

rst:0x3 (SW_RESET),boot:0x17 (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:0x3fff0018,len:4
load:0x3fff001c,len:5096
load:0x40078000,len:12856
load:0x40080400,len:3480
entry 0x40080638
E (679) esp_image: Image length 1669984 doesn't fit in partition length 1572864
E (679) boot: Factory app partition is not bootable
E (681) boot: No bootable app partitions in the partition table
ets Jun  8 2016 00:22:57

rst:0x3 (SW_RESET),boot:0x17 (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:0x3fff0018,len:4
load:0x3fff001c,len:5096
load:0x40078000,len:12856
load:0x40080400,len:3480
entry 0x40080638
E (679) esp_image: Image length 1669984 doesn't fit in partition length 1572864
E (679) boot: Factory app partition is not bootable
E (681) boot: No bootable app partitions in the partition table
ets Jun  8 2016 00:22:57

rst:0x3 (SW_RESET),boot:0x17 (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:0x3fff0018,len:4
load:0x3fff001c,len:5096
load:0x40078000,len:12856
load:0x40080400,len:3480
entry 0x40080638

@elliotwoods
Copy link
Author

using command esptool write_flash -z 0x1000 firmware.bin

@elliotwoods
Copy link
Author

I've never seen that happen before
perhaps the length of the image is wrong in the partitions table?

my partitions.csv looks like this:

# Notes: the offset of the partition table itself is set in
# $ESPIDF/components/partition_table/Kconfig.projbuild and the
# offset of the factory/ota_0 partition is set in makeimg.py
# Name,   Type, SubType, Offset,  Size, Flags
nvs,      data, nvs,     0x9000,  0x6000,
phy_init, data, phy,     0xf000,  0x1000,
factory,  app,  factory, 0x10000, 0x180000,
vfs,      data, fat,     0x200000, 0x200000,

@stinos
Copy link
Owner

stinos commented Aug 26, 2020

@elliotwoods thanks for trying this out; as I don't have any experience with such issues nor ESP32 I don't really know what to do here; keep in mind this is a build for the 'GENERIC' board definition, maybe that is an issue?

Anyway might be easier if you build locally: pull esp32-c++ branch of this repository, and esp32-c++ branch from my MicroPython fork https://github.com/stinos/micropython which (a least for me) work out of the box after having followed your instructions to get the esp SDK as above. Then with a directory structure like

projects/
   | micropython
   | micropython-wrap

cd to the ports/esp32 directory and build with (possibly add a board definition etc)

make USER_C_MODULES=../../../micropython-wrap CFLAGS_EXTRA="-DMODULE_UPYWRAPTEST_ENABLED=1" all V=1

@elliotwoods
Copy link
Author

Alright! I’ll check that tomorrow. Out of office now (Seoul GMT+8)
Thank you for giving direct instructions on how to test

@elliotwoods
Copy link
Author

getting the same issue building here
Very strange, since it seems to be related to partitions, but your commits don't seem to affect partitions at all
investigating...

@elliotwoods
Copy link
Author

removing the USER_C_MODULES and CFLAGS_EXTRA doesn't fix it
there must be something weird within the microython repo itself..

@stinos
Copy link
Owner

stinos commented Aug 31, 2020

removing the USER_C_MODULES and CFLAGS_EXTRA doesn't fix it

Hmm, that's strange, as I'm only a couple of commits away from master I'd also think it's a MicrpPython problem then. Can you create an issue for it?

@elliotwoods
Copy link
Author

i'm trying current master now to check... (old master does work here)

@elliotwoods
Copy link
Author

yeah weird..
that works
image

@elliotwoods
Copy link
Author

elliotwoods commented Aug 31, 2020

and your branch of micropython works when i pull the latest micropython/micropython master
(so potentially they'd temporarily broken something for esp32 around the time you forked)
trying now with the modules also...

@elliotwoods
Copy link
Author

seems related..
micropython/micropython-esp32#235

trying this

@IhorNehrutsa IhorNehrutsa mentioned this issue Sep 8, 2020
Closed
@elliotwoods
Copy link
Author

Revisiting this now (setting up from scratch on a new computer)...

Ending today on this error when building (when it gets to the qstr):

GEN build-GENERIC/genhdr/qstr.i.last
cc1plus: warning: command line option '-std=gnu99' is valid for C/ObjC but not for C++
In file included from /home/kimchips/.espressif/tools/xtensa-esp32-elf/1.22.0-80-g6c4433a-5.2.0/xtensa-esp32-elf/xtensa-esp32-elf/include/c++/5.2.0/cstdint:35:0,
                 from ../../../micropython-wrap/tests/../detail/micropython.h:25,
                 from ../../../micropython-wrap/tests/../detail/frompyobj.h:4,
                 from ../../../micropython-wrap/tests/../detail/callreturn.h:4,
                 from ../../../micropython-wrap/tests/../classwrapper.h:36,
                 from ../../../micropython-wrap/tests/module.cpp:1:
/home/kimchips/.espressif/tools/xtensa-esp32-elf/1.22.0-80-g6c4433a-5.2.0/xtensa-esp32-elf/xtensa-esp32-elf/include/c++/5.2.0/bits/c++0x_warning.h:32:2: error: #error This file requires compiler and library support for the ISO C++ 2011 standard. This support is currently experimental, and must be enabled with the -std=c++11 or -std=gnu++11 compiler options.
 #error This file requires compiler and library support for the \
  ^
make: *** [../../py/mkrules.mk:89: build-GENERIC/genhdr/qstr.i.last] Error 1
make: *** Deleting file 'build-GENERIC/genhdr/qstr.i.last'

I notice that on line 274 of the ports/esp32/Makefile we have:
CXXFLAGS = -std=gnu++11 $(CFLAGS_COMMON) $(INC) $(INC_ESPCOMP) -I$(BOARD_DIR)

However, perhaps this is not being applied at the QSTR build stage?

@elliotwoods
Copy link
Author

forcing the flag works (as in it builds)
i.e. in mkrules.mk add it to the CFLAGS on line 14:

ifeq ($(MICROPY_ROM_TEXT_COMPRESSION),1)
# If compression is enabled, trigger the build of compressed.data.h...
OBJ_EXTRA_ORDER_DEPS += $(HEADER_BUILD)/compressed.data.h
# ...and enable the MP_COMPRESSED_ROM_TEXT macro (used by MP_ERROR_TEXT).
CFLAGS += -DMICROPY_ROM_TEXT_COMPRESSION=1 -std=c++11
endif

This causes many warnings in the output of course every c file compiled throws a warning

@stinos
Copy link
Owner

stinos commented Oct 6, 2020

Revisiting this now (setting up from scratch on a new computer)...

So you cloned everything from scratch, checked out esp32-C++ branches, used same SHA for ESP sdk as you mentioned above? Just so I can try to reproduce this - could be that I never did a clean in between trying things and QTR generation didn't kick in or so.

Adding c++ flags to CFLAGS is expected to fail.

@elliotwoods
Copy link
Author

elliotwoods commented Oct 6, 2020

Yes you're correct. I:

It does build and succeed with the -std=c++11 hack, but when I add your USER_C_MODULES then i get issues with multiple definitions of std::__throw_bad_function_call() defined in esp-idf/cxx/libcxx.a
(which sounds like perhaps there's 2 different standard libraries being mixed). This occurs with or without CFLAGS_EXTRA="-DMODULE_UPYWRAPTEST_ENABLED=1", but disappears when i don't add USER_C_MODULES

@stinos
Copy link
Owner

stinos commented Oct 6, 2020

Ok thanks for the info. I'll try to sort this out as soon as time permits, might be a couple of days. Note in the gist you linked to you're using a different SHA than the one mentioned above, not sure if that is the problem though.

@elliotwoods
Copy link
Author

oh waiit - i need a specific SHA for ESPIDF.
let me check...

@elliotwoods
Copy link
Author

elliotwoods commented Oct 6, 2020

i presume that's the SHA for v4.0 of ESP-IDF in my comment above (whilst i've been trying on v3.x today)
checking with that SHA above now...

@stinos
Copy link
Owner

stinos commented Oct 6, 2020

Tried this again here after cleaning sources (with the v4.0 SHA) and the compiler also emits the warning:

> make USER_C_MODULES=../../../micropython-wrap CFLAGS_EXTRA="-DMODULE_UPYWRAPTEST_ENABLED=1" all -j4
...
GEN build-GENERIC/genhdr/qstr.i.last
cc1plus: warning: command line option '-std=gnu99' is valid for C/ObjC but not for C++
GEN build-GENERIC/genhdr/qstr.split
...

but it doesn't lead to any errors and the build continues until completion.

@elliotwoods
Copy link
Author

ok! that builds and links
when i run the firmware I go back to having the issue described above about the partition table
i think i've tracked that down

@elliotwoods
Copy link
Author

YES!!!
we are in businesss!!
image

I had to change the partitions.csv in the micropython/ports/esp32 folder to:

# Notes: the offset of the partition table itself is set in
# $ESPIDF/components/partition_table/Kconfig.projbuild and the
# offset of the factory/ota_0 partition is set in makeimg.py
# Name,   Type, SubType, Offset,  Size, Flags
nvs,      data, nvs,     0x9000,  0x6000,
phy_init, data, phy,     0xf000,  0x1000,
factory,  app,  factory, 0x10000, 0x200000,
vfs,      data, fat,     0x210000, 0x1F0000,

The factory partition doesn't have to be quite this large, but this leaves a little extra for the user's C++ code+libraries

@elliotwoods
Copy link
Author

re: the warning before about the std library
the warnings which come from the C file are fine
it's that there was a file in the ESP-IDF (v3) which required c++11, and hence you needed the flag somewhere to build with c++11. It seems with this SHA (which i presume is v4), the flag isn't needed

@stinos
Copy link
Owner

stinos commented Oct 6, 2020

Aha, nice!
I'll leave this open for reference now since it has all explanation.
However in the end it almost solely comes down to modifications to MicroPython itself to have it support C++ for user modules so there's not a whole lot I can do on that front.

@elliotwoods
Copy link
Author

Thank you so much!

I guess some things to share this goodness further could be:

  • pull the edited partitions.csv into your ESP32-C++ branch of MicroPython
  • pull any changes (which can be) into the main branch of micropython-wrap
  • potentially PR your ESP32-C++ branch of MicroPython to the upstream master? Perhaps just to create a discussion with examples of what needs to change

@stinos
Copy link
Owner

stinos commented Oct 6, 2020

Makes sense; first point is easy enough. Second point is doable with a make flag to remain compatible with the current situation. Third point: I already have an open PR agains Microython for the QSTR generation part. The rest is rather specific to ESP32 but indeed doesn't hurt to start discussion from that.

@stinos
Copy link
Owner

stinos commented Oct 15, 2020

@elliotwoods I've worked on this a bit and there's now a PR for MicroPython for C++ support, and I changed the esp32-c++ branch here to work against that. Can you test this if you have some time? So that's the usercmodule-c++ bracnh from my MicroPython repository, and the esp32-c++ branch from this one (needs pull --rebase because I overwrote the previsuo commits). Then make usercmodule MICROPYTHON_PORT_DIR=../micropython/ports/esp32 from the micropython-wrap directory should work, and do the same as before, difference being that after the PR gets merged it will work with the master MicroPython branch i.e. no special customizations needed anymore.

I didn't include the partitions.csv changes though; would there be a way to be able to customize that from the command line? Or does everyone really has to manually edit the file? Seems worth raising an issue for that.

@elliotwoods
Copy link
Author

Hi @stinos
I'm sorry but i'm crammed on a deadline at the moment
I will try to check in on this later

@stinos
Copy link
Owner

stinos commented Oct 17, 2020

No worries, not urgent at all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants