-
-
Notifications
You must be signed in to change notification settings - Fork 507
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
added support for seeed xiao esp32 c3 #664
Conversation
See: #663 |
Somehow my module started behaving weird. My log output looks like this now:
I had to reformat the littlefs, but not sure if i did that correctly. I'm quite new to the esp32 platform. |
I've got the gpio_set_level problem solved by changing in InverterSettings.cpp: |
I can confirm that the error messages do not appear anymore. Now i only need an inverter to test and in fact i can still not access the web ui. |
78a919c
to
67fadf1
Compare
9f8a45f
to
4c68d51
Compare
@tbnobody mind taking a look? I'm a first time contributor, please let me know if there is any docs to adhere to in terms of code style etc. |
4c68d51
to
63b4d7e
Compare
fixes #680 |
@math322 can you check whether this one works for you? |
Choosing FSPI for SPI will not work for legacy ESP32. You need to distinguish like I did here for Ahoy. Same applies to the S3 ESP32. So I recommend you directly enhance this also for the S3. EDIT: I just saw you had a source code comment there already. So yes, I can confirm, this applies to the S3 as well. |
I will try to find some time in the next 72 hours to add the S3 as well. |
Sure, sounds good. Als linked in the code example from ahoy just 'oring' the second define is enough. The devkit-s3 target will tell you if it compiles properly. |
i wonder how there can be this board definition already with 2 contributors at least if that issue generally applies to all esp32-s3 chips. EDIT: ref: https://github.com/tbnobody/OpenDTU/blame/master/platformio.ini#L131 EDIT 2: I'm well aware that just because it builds, doesn't mean it runs ;-) |
9d96f0d
to
601232d
Compare
There is no issue with S3 so far as it has complete flexibility. But it makes sense to move to the SPI bus to a persistent location. For S3 the FSPI and HSPI are both hardware SPI PHys, so which one you pick does not matter. So the old approach to not select one explicitly just worked. This only becomes problematic if the second one is to be used for a display or a CMT2300A chip for HMS/HMT series comms. |
I see multiple problems with this approach, however i could have an error in my thoughts. When we change the default, it might break existing setups. This is especially true for the mentioned displays (which i see a lot in facebook groups where people proudly post their current stats). What do you think @markusdd , should this be configurable in the platformio.ini settings instead? I have a feeling that hard setting this for a whole set of boards might make some people unhappy. |
a) there is no official SPI display support for OpenDTU yet as far as I am aware. If there is something soon to be supported, which SPI PHY does it use? (in ahoy it is HSPI, that is why we shift NRF24 to VSPI on ESP32 and FSPI on ESP32-S3/C3) b) The Phy being used internally is a pure driver thing. This is not noticeable to the end user, as there is no connection from the pin config to the phy config. (only in the actual code obviously) I would refrain, at least for now, to make this selectable as no one will understand what this actually does, as I would assume like only 5% or less have read the actual datasheet of all relevant ESP32 generations. (I only did because I designed a whole integrated PCB) Also, as far as I know people are working on HMS/HMT support which comes with the addition that a second SPI based Radiomodule (CMT2300A, 868MHz) must be supported, which makes integrating an SPI Display even harder. |
Thanks for clearing all of that up. In that case i will keep my change-set as-is, unless someone complains that something doesn't work. let me know if this works for you. |
if all builds compile from my view that looks fine, of course @tbnobody has the final call. |
The two added builds are compiling without issues. |
Not 100% sure. I just realized today, that the patch with VSPI/FSPI breaks the current implementation of the Nokia display. The HW_SPI driver of the display library uses the |
The problem is once multiple SPIs get used it is basically mandatory to explicitly select the SPI PHY to be used. In ahoy the SPI display implementations default to HSPI hardcoded and the NRF24 takes the other one based on whatever is availabel per generation. That is usually VSPI for ESP32 and FSPI should do for S3/C3 |
601232d
to
2245905
Compare
As far as I see it, support for esp32-c3 should be already added. feel free to open again if something is not working as expected |
I will check sometime in the upcoming weeks. |
Thanks, it works with my devkitc-02. |
Finally had some time to try it out and i can confirm that after compiling the latest master and flashing the image to the board, i finally saw the access point, could connect to it and use the web ui. |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new discussion or issue for related concerns. |
This commit changes the esp webserver dependency to a fork that i did with the pr suggested by @tbnobody.
I build the code and uploaded it to my "Seeed Studio XIAO ESP32C3" (https://wiki.seeedstudio.com/XIAO_ESP32C3_Getting_Started) and could connect to the web ui nicely.
The spi wifi module did also work after some struggles with the spi bus setting (thanks @hardstyler-hh for the fix).
Since i do not own a hoymiles micro inverter i can not test the wifi modules functionality, so i can only confirm data that is visible in the web ui.
The web ui did NOT show the error message about the wiring of the wifi module anymore after it fixed the spi bus setting.
I added a .tool_versions file for asdf (https://asdf-vm.com/) which makes managing tooling a bit easier.
There is now a check whether we are building for esp32-c3, which then sets the spi bus to FSPI instead of HSPI.
It might be good to check against esp32-s3, as it could face the same issue. However i don't own one, so maybe any code contributor could help here?
Also i added a sleep of 1 second, just for convenience when using the upload and monitor functionality of vscode.