-
-
Notifications
You must be signed in to change notification settings - Fork 638
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
Add support for ESP32-H2 and ESP32-C2 #1347
base: develop
Are you sure you want to change the base?
Conversation
I just found out there is already a PR for the C2 variant. As my PR is also based on the work from Jason2866 these two are very similar, just with the added H2 variant here. So maybe first merge #1203 and then I can rebase? |
@valeros should this PR maybe be changed to only include ESP32-H2 or close this PR and create a new PR for only ESP32-H2? |
For me it is fine if my PR is closed in favour of this one. Let's hope something gets merged. |
Could you please validate this? |
Could you please tell me what is missing or validate this request? |
@valeros is there anything the community could do to help merging this request? |
Could you validate this request? |
Hi everyone!! Why are pull requests about ESP32-C2 not being merged? What is the problem exactly? Because the only problem I see in all the pull requests is the lack of words. No one answers but no one does anything. We have been waiting for support for this board for over a year and it is still not available. There are people like @Jason2866 who have coded the component but it is totally ignored and we don't even know why. Please, can you accept this pull request or at least describe why it is not being merged? Thank you all very much. |
As they require Arduino v3.0 based on ESP-IDF v5.1 or later so you can find explaination and very long discussion here -> #1225 FYI, easiest workaround today is to use pioarduino's community fork of platform-espressif32 -> espressif/arduino-esp32#10039 |
@@ -236,7 +236,7 @@ def __fetch_fs_size(target, source, env): | |||
mcu = board.get("build.mcu", "esp32") | |||
toolchain_arch = "xtensa-%s" % mcu | |||
filesystem = board.get("build.filesystem", "spiffs") | |||
if mcu in ("esp32c3", "esp32c6"): | |||
if mcu in ("esp32c2", "esp32c3", "esp32c6", "esp32h2"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not much of a python guy, but I program(med) a lot.
Espressif has said that all new devices will have RISC-V cores. Should code like this reflect that? Could this code set a global (I know, globals are bad. But we have mcu...) for "mcu_is_riscv" using approximately this test and then use "mcu_is_riscv" (I don't care how it's spelled) in all the places that are making this exact test, such as setting the GDB name? Otherwise, when something like p4 gets added, there's going to be the same bulky hunt-and-destroy exercise to add a model name to a test that's essentially an eternally growing list of post-2020 models?
Here we set
if mcu in ("esp32c2", "esp32c3", "esp32c6", "esp32h2", "esp32p4", ...):
mcu_family_riscv = true; // whatever
Then the special cases for GDB and ULP and esp-builtin and the dozen others changed here can be
"tool-riscv32-esp-elf-gdb"
if mcu_family_riscv
else "tool-xtensa-esp-elf-gdb"
...and importantly, those dozen hunks don't have to change when p4 and p5 and esp32z3 and whatever else gets added.
Otherwise, the bulk of this CL gets repeated for every new incoming model.
Just a review fly-by...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not that easy. There are checks not only based on the MCU architecture. As example ULP check...
So the whole code needs always checked when a new MCU is added. I knew this a bit, added the P4 and LP core support ;-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As long as you've looked at it, minimized the amount of work to do, and concluded this is as good as it gets, my work here is done. 😀. I'll take my not quite common subexpression optimization and crawl back under my rock. I was just trying to help make future revs not need the entire catalog added in a bunch of places.
Thanx for being awesome.
This PR adds support for ESP32 H2 and C2 variants on esp-idf only.