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

platforms/esp32-idf: make the code compile, add build test, cleanup #27

Merged
merged 5 commits into from
Dec 30, 2019

Conversation

igrr
Copy link
Contributor

@igrr igrr commented Dec 30, 2019

  • esp32-idf: Remove unneeded files
  • Simplify main.c, remove leftovers from IDF hello-world example
  • Sync ESP32 examples for IDF and PIO, check in CI that the source is kept the same
  • Add a build job for esp32-idf
  • Run the test in QEMU for esp32

@igrr igrr changed the title platforms/esp32-idf: make the code compile, add build test, cleanup WIP: platforms/esp32-idf: make the code compile, add build test, cleanup Dec 30, 2019
@igrr igrr force-pushed the esp32-idf-ci branch 4 times, most recently from ce72bde to 2888d54 Compare December 30, 2019 16:12
@igrr igrr changed the title WIP: platforms/esp32-idf: make the code compile, add build test, cleanup platforms/esp32-idf: make the code compile, add build test, cleanup Dec 30, 2019
@vshymanskyy
Copy link
Member

@igrr thanks. I've been working on this already and I got the examples running again.
Unfortunately it's not yet ready to be comitted, as it relies on fix of #24 (which is big)

@igrr
Copy link
Contributor Author

igrr commented Dec 30, 2019

No problem, please ping me when the master branch is fixed, I'll rebase.

@vshymanskyy
Copy link
Member

@igrr I decided to submit it partially (it's already on the master).
Things to note:

  • I have only updated examples for pio
  • d_m3AllocateLinearMemory flag was removed (now always on)
  • d_m3SkipStackCheck was enabled for esp devices, will investigate later

@igrr
Copy link
Contributor Author

igrr commented Dec 30, 2019

Thanks, so I understand I need to add -Dd_m3FixedHeap=8192 -Dd_m3LogOutput=false -Dd_m3SkipStackCheck to the IDF build as well, right?

@igrr
Copy link
Contributor Author

igrr commented Dec 30, 2019

Okay, have rebased, now this includes the QEMU test.

@vshymanskyy
Copy link
Member

@igrr esp32 should have enough memory (and proper heap implementation), so should work without d_m3FixedHeap

@vshymanskyy
Copy link
Member

@igrr this is cool, thanks for this contribution. It looks like it's almost ready to be merged, but currently the test timeouts for some reason.
One more thing, when d_m3LogOutput is defined, there's no default Result: ... output.
Currently you can prin the result like this:

long value = *(uint64_t*)(runtime->stack);
printf("Result: %ld\n", value);

@igrr
Copy link
Contributor Author

igrr commented Dec 30, 2019

Yep, the test times out because the app is not finishing (not calling exit or esp_restart). I'll push the fix in a few hours.

About printing the result, i have tried this snippet, but I'm getting an error about dereferencing an incomplete type. Not sure how it compiles in Platformio...

@vshymanskyy
Copy link
Member

@igrr yeagh, in this case you need:

#include "m3/m3_env.h"

@igrr igrr force-pushed the esp32-idf-ci branch 2 times, most recently from 0d41e84 to 1c4a972 Compare December 30, 2019 21:13
@igrr
Copy link
Contributor Author

igrr commented Dec 30, 2019

@vshymanskyy Have fixed the test, and also synced IDF and PIO versions of the ESP32 example. The test job will now check that they stay in sync.

@vshymanskyy vshymanskyy merged commit dbb90e3 into wasm3:master Dec 30, 2019
@vshymanskyy
Copy link
Member

Merged 🔥 🔥 🔥

@igrr igrr deleted the esp32-idf-ci branch December 30, 2019 21:48
@vshymanskyy
Copy link
Member

@igrr does standard input work with the ESP32 qemu emulator?
I'm thinking to add repl mode to ESP32, then we can try running spec tests suite!

@igrr
Copy link
Contributor Author

igrr commented Jan 3, 2020

It does, sort of. I may need to implement UART FIFO timeout for the IDF UART driver to work properly, but that shouldn't be a big issue.

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

Successfully merging this pull request may close these issues.

2 participants