search

thiagoralves / OpenPLC_Editor

OpenPLC Editor - IDE capable of creating programs for the OpenPLC Runtime
GNU General Public License v2.0
430 stars 207 forks source link

Add support for ESP32S3 #106

Closed rhysperry111 closed 6 months ago

rhysperry111 commented 8 months ago

Starting to try and add support for the ESP32S3. Main issues with this was the board does not have a DAC so PWM has to be used as a crude substitute.

While modifying the ESP32 file I also made the PWM_CONTROLLER implementation more flexible, moving away from FastPWM which seems to be abandoned over to the more standard LEDC.

Would appreciate some help as in order to use the required LEDC channel initiation function, esp32:esp32 core >3.0.0 must be used. I have made an attempt to switch over to the development library sources, but I'm just getting a bunch of undefined reference errors when compiling.

Changes made:

OliverB21 commented 8 months ago

Any update on inclusion of this feature?

rhysperry111 commented 8 months ago

AFAIK it should be all good to be merged, however I have been unable to test everything as a whole because of compile errors when using the dev ESP32 core. I have no idea how to fix the issues with compilation in the current state, however you can track the progress of the 3.0.0 core release using the milestone on espressif/arduino-esp32.

You shouldn't need the specific hardware to reproduce the compilation error, so help from anyone with Arduino compiler knowledge would be appreciated.

rhysperry111 commented 7 months ago

@me-no-dev helped fix the compilation over in an issue in the espressif repo. Will add a commit to change some flags and then after some testing this should be ready for merge.

rhysperry111 commented 7 months ago

Seems to be working - although I've had to do a hacky workaround because subprocess.Popen really doesn't seem to like the idea of having an argument to a command with a space in it. Will see what I can do tomorrow

note for tomorrow me: some basic "$@" arg debugging seems to suggest getting rid of quotes might help

rhysperry111 commented 7 months ago

Looks all good from my side although I haven't been able to test anything on an OS other than Linux. I ended up removing the quotes from the compile args as they seemed to be messing with things.

thiagoralves commented 7 months ago

@rhysperry111 Thank you so much for all your changes. I took a look at them and they look good, however I have a main (huge) concern. If I understood it correctly, you're replacing the ESP32 board file source with another source from Espressif that is basically a dev branch. Is that right? I'm worried about making that change and in one way or another break all the other ESP32 boards that are currently working fine on OpenPLC. Could we make that change exclusive for the ESP32S3?

rhysperry111 commented 7 months ago

If I understood it correctly, you're replacing the ESP32 board file source with another source from Espressif that is basically a dev branch. Is that right?

Yeah that's correct. It's because the new esp32.cpp file uses functions which are only available in the 3.0.0 release which is currently in rc2. If it eases your conscience the dev sources aren't nightly but just the release candidates.

I'm worried about making that change and in one way or another break all the other ESP32 boards that are currently working fine on OpenPLC. Could we make that change exclusive for the ESP32S3?

Not really an easy way to do this without splitting off esp32.cpp as well so I'd rather not. I don't see any reason why other boards would break (although there is a breaking change in that PWM_CONTROLLER now maps channels to pins differently).

I'm happy to wait until 3.0.0 is in a stable release if it's a huge problem.

thiagoralves commented 7 months ago

Yeah, I believe waiting for a final release would be better. I'm just thinking about a million of other things that can go wrong, especially some crucial things like Serial that directly affects live monitoring.

rhysperry111 commented 6 months ago

3.0.0 has just been released upstream - not sure if there's any testing of other areas of code (serial, WiFi) that you want to do. I'll send in a commit switching back to stable sources soon.

thiagoralves commented 6 months ago

Awesome! If you can, just make sure that a simple program compiles for the ESP32 and that Modbus serial and TCP work

thiagoralves commented 6 months ago

It seems that ESP32 core 3.0.0 is broken. I updated my core and it won’t compile the blink example. See https://openplc.discussion.community/post/esp32-compilation-failed-13378321?pid=1339594513

me-no-dev commented 6 months ago

3.0.0 is not broken. There is something wrong on your end @thiagoralves

rhysperry111 commented 6 months ago

Probably due to the overriding of compile flags. One of the commits in this PR fixes that

thiagoralves commented 6 months ago

@rhysperry111 is this ready to merge or do you still need to change the board source file back to the original?

rhysperry111 commented 6 months ago

Already changed the board file back with the force push two days ago. I've not done any testing to verify modbus over RTU, however I had modbus over TCP working.

Probably worth merging so at least ESP32 will compile again and then fixing anything that crops up with another PR. More than happy to help with that.

thiagoralves commented 6 months ago

Awesome! Merging now. Thanks again!

rhysperry111 commented 6 months ago

If you plan on doing a release with this in, it's worth mentioning that existing ESP32 projects that use PWM_CONTROLLER blocks will need to be tweaked slightly.

The pin input to the block now directly maps to the output PWM pin rather than being an index to an arbitrary mapping