Library does not work properly on ESP32 S2 - fix provided.

[tchilton]

Nice code - thanks, but there are several related problems here that have driven me into a deep dive to fix them.

1. The readme is not clear on which versions of the ESP32 are supported - it does not state ESP32 S2 or ESP C3, but there are bugfixes in for C3. Please update the readme to more clearly show which CPU versions are supported.

2. Getting the code to compile - as others have reported, using the stock ESP32 board definitions via Arduino board manager, which is currently listed as in alpha, results in compile errors with `__atomic_exchange_1' type messages.

The solution to this is to update to the latest ESP32/ESP32 S2 code from Expresif's website as detailed in
(https://docs.espressif.com/projects/esp-idf/en/latest/esp32s2/api-guides/tools/idf-tools.html#xtensa-esp32s2-elf) and clarified in the article at (https://www.mischianti.org/2020/12/01/esp32-s2-pinout-specs-and-arduino-ide-configuration-1/). Note the change from the 2020 r3 to 2021 r1 code and the version 3.1 of esptools. Once this is correctly applied, the code will compile properly.

3. The pin restrictions listed in the source code are not accurate for ESP32 S2 and ESP C3 devices and this results in silent configuration failures and what looks like dead code. The root of this is in the begin method. There must be a better way to report config failures - even something as simple as a Serial.println from the class to say that the pin is not valid on a failure from the isValidXxGPIOPin functions. Given that most of the methods have void return types, this appears to be the only sensible route forwards.

The fixes need to make ESP32 S2 and probably ESP C3 (I've not got one to test with) usable on all the mapped out pins are as follows, The definitions are from the Expresif IDF and not standard to Arduino and the links to the vendors module pinouts are listed below.

bool SoftwareSerial::isValidGPIOpin(int8_t pin) {
#if defined(ESP8266)
return (pin >= 0 && pin <= 16) && !isFlashInterfacePin(pin);
#elif defined(ESP32)
#ifdef CONFIG_IDF_TARGET_ESP32
return (pin >= 0 && pin <= 5) || (pin >= 12 && pin <= 19) ||
(pin >= 21 && pin <= 23) || (pin >= 25 && pin <= 27) || (pin >= 32 && pin <= 39);

#elif CONFIG_IDF_TARGET_ESP32S2
// See [https://docs.espressif.com/projects/esp-idf/en/latest/esp32s2/_images/esp32-s2_saola1-pinout.jpg]
return (pin >= 1 && pin <= 21) || (pin >= 33 && pin <= 44);

#elif CONFIG_IDF_TARGET_ESP32C3
// See [https://docs.espressif.com/projects/esp-idf/en/latest/esp32c3/_images/esp32-c3-devkitm-1-v1-pinout.jpg]
return (pin >= 0 && pin <= 1) || (pin >= 3 && pin <= 7) || (pin >= 18 && pin <= 21);
#else
return true;
#endif
#else
return true;
#endif
}

bool SoftwareSerial::isValidTxGPIOpin(int8_t pin) {
return isValidGPIOpin(pin)
#if defined(ESP32)
#ifdef CONFIG_IDF_TARGET_ESP32
&& (pin < 34)

#elif CONFIG_IDF_TARGET_ESP32S2
&& (pin < 45)

#elif CONFIG_IDF_TARGET_ESP32C3
// Nothing to do

#endif
#endif
;
}

Once the above are all applied, the code compiles and works properly and I've had it running successfully at 115200 baud.

Thinking a little further on this, is it really necessary to have the pin restrictions in the code - I know this is done for good intention to stop people tripping over the pins with special functions or limitations, but could this not be better covered in the documentation, rather than a silent error that is even more difficult to diagnose ?

[dok-net]

@tchilton For the sake of discussing your suggested code changes, please provide them as a PR.

[dok-net]

Why not GPIO0 for ESP32S2? Etc.
Please use https://www.espressif.com/sites/default/files/documentation/esp32-s2_datasheet_en.pdf, https://www.espressif.com/sites/default/files/documentation/esp32-c3_datasheet_en.pdf, analogous to https://www.espressif.com/sites/default/files/documentation/esp32_datasheet_en.pdf, for the purpose of reasoning about this. I can't agree with your musings on silent config failures or "dead code", the restrictions are for safety, not to aid in dynamic configuration and probing.

[dok-net]

I don't know that CONFIG_IDF_TARGET_ESP32S2 etc are defined, grepping the generated build sources does not yield a hit. Please use something else to tell the different MCUs apart.

[tchilton]

I don't know that CONFIG_IDF_TARGET_ESP32S2 etc are defined, grepping the generated build sources does not yield a hit. Please use something else to tell the different MCUs apart.

These are the correct definitions from the manufacturer (Espressif). You can find the definitions as part of the broader chip config in
%appdata%\..\Local\Arduino15\packages\esp32\hardware\esp32\2.0.0-alpha1\tools\sdk\esp32s2\include\config\sdkconfig.h
and related other chip families by replacing the esp32s2 part of the part with the other devices.

This is based on a fresh build fully patched Windows 10 machine, fresh install of Arduino framework 1.8.15, then an install of the ESP32 2.0.0-alpha-1 direct from Espressif.

You can also prove they are defined correctly by dropping in this pre-processor chunk into an empty application, select a chip family and compile, you will get the warning for each type you select.

#ifdef CONFIG_IDF_TARGET_ESP32
#warning "ESP32 Vanilla"
#elif CONFIG_IDF_TARGET_ESP32S2
#warning "ESP32 S2"
#elif CONFIG_IDF_TARGET_ESP32C3
#warning "ESP32 C3"
#else
#warning "Something else"
#endif

This is far better than using the Arduino board directives since you would have to constantly keep adding definitions for every new board that gets added to the framework, hence the above is far more robust and easier to maintain, plus it works.

[tchilton]

Why not GPIO0 for ESP32S2? Etc.
Please use https://www.espressif.com/sites/default/files/documentation/esp32-s2_datasheet_en.pdf, https://www.espressif.com/sites/default/files/documentation/esp32-c3_datasheet_en.pdf, analogous to https://www.espressif.com/sites/default/files/documentation/esp32_datasheet_en.pdf, for the purpose of reasoning about this. I can't agree with your musings on silent config failures or "dead code", the restrictions are for safety, not to aid in dynamic configuration and probing.

The sources I used were diagrams direct from Espressif and contain the exact same data as the datasheets, just in an easier and quicker to digest form. see
(https://docs.espressif.com/projects/esp-idf/en/latest/esp32s2/_images/esp32-s2_saola1-pinout.jpg) and (https://docs.espressif.com/projects/esp-idf/en/latest/esp32c3/_images/esp32-c3-devkitm-1-v1-pinout.jpg)

In your initial definitions, you mapped all pins on the ESP32 except for the FLASH SPI bus and the Input only pins. I cloned the same logic. There are three groups to note on the ESP32 S2

• A PSRAM / FLASH memory pin, which can't function as a normal GPIO - GPIO26
• The input only pin (GPIO46), which is mapped for in only, similarly to how you mapped on the ESP32
• The Strapping pins that control the devices boot process, of which there are 3. It makes little sense to use any of them as serial pins and end up with a non-booting board. There are plenty of pins on the ESP32 S2.

The same logic also applies for the ESP32 device. I'd never use BOOT or any of the config pins for IO for the same reason

On the ESP32 C3, there are 4 strapping pins defined.
I can't test this one as I've not got one of those boards.

In regards to the point about silent failures / dead code. I couldn't determine why the library wasn't working as expected when I used it. As I stated, it looked like dead code as nothing worked and there was no error info coming back to me, so I had to spin up an ESP32 DevKit1 board and I tried again with that and got it going, so I then knew that the code could work. That's when I got into the more detailed analysis . Bear in mind that the Espressif code for ESP32 is not yet out of beta, so it could equally have been some framework related issue that I was encountering. Once I started pulling the library around I found the problem. I had used pins 33 and 34 on the ESP32 S2 and the pin checking logic had silently failed them. Once that was understood, it was trivial to fix the code per the submission above.

My suggestion is on improving that experience, either by not having the checks done in code and documenting it in the readme, since we should be able to assume that people read the info on the library and understand the chip well enough for the intended tasks and most other libraries do not do this level of checking.

Alternately, if you want to keep it, then given that the begin function has a void type and hence doesn't return any error value and the isValid* functions are all private, there is no easy way to check the config and if it doesn't work, you have to go deeper than many users would probably want to.

Other than broad alignment to the standard hardware based serial config, I guess there is no reason why the begin function has to return a void, it could return positive values for error allowing for you to do :

...
SoftwareSerial mySerial;

if (! mySerial.begin(blah...)) {
Serial.println("Pin configuration is invalid for this chip, please check documentation");
}

After all, the constructor has different constants and calling arguments already, so this would be a logical extension and results in tidy code.

[dok-net]

@tchilton You forget about operator bool(). Please come forward with a PR, I am buying into your reasoning about the CONFIG_IDF_TARGET_, so please just give us something to quickly review and merge. Your submission will be under the license of the project, obviously.

[tchilton]

@tchilton You forget about operator bool(). Please come forward with a PR, I am buying into your reasoning about the CONFIG_IDF_TARGET_, so please just give us something to quickly review and merge. Your submission will be under the license of the project, obviously.

Thanks. will bool the ::begin.

Working on the pull request next. More than happy on the open source contributions side of things.

[dok-net]

Thanks. will bool the ::begin.

That's not what I meant, tho - there is an operator bool() in the SoftwareSerial class already, please leave the signature of begin() alone.

I forgot about GPIO0, again, it seems to me that the images you refer to are not quite as expressive as the docs I linked, because I cannot see anything that says GPIO0 should not be used on the S2 or C3, whatever. Just stick to the PDFs, please!

[tchilton]

Thanks. will bool the ::begin.

That's not what I meant, tho - there is an operator bool() in the SoftwareSerial class already, please leave the signature of begin() alone.

I forgot about GPIO0, again, it seems to me that the images you refer to are not quite as expressive as the docs I linked, because I cannot see anything that says GPIO0 should not be used on the S2 or C3, whatever. Just stick to the PDFs, please!

OK, so I just learned something - thanks. If only I'd know that had existed that would have saved a lot of head scratching. Definitely one for the documentation somewhere.
For clarity for others reading :

logger.begin(115200, SWSERIAL_8N1, DEBUG_RX, DEBUG_TX, false, 256, 256);
if (!logger) { Serial.println("Bad pin config for logger"); }

So, using the bool operator, you can see if the object is good or not.

GPIO0:
In all the data sheets for each device, check section 2.4 Strapping pins, this defines all the special pins that affect things like bootup and need special care.
On ESP32 S2, GPIO0 is part of the boot logic as are GPIO45 and GPIO46. If you are driving serial comms into these pins and the chip is reset or browns out, you have no idea / control on how the device will boot as it will depend on what was on the pin at the time it was sampled during boot, so it may fail to boot or boot into an indeterminate state. Neither are good for microcontroller based projects, hence why I'd strongly recommend that they are excluded from the config. The same logic applies equally to the C3 and the original ESP32 devices. As you said in your initial response to me "the restrictions are for safety", so excluding them seems to fit that principle very well.

I'll leave the older ESP32 device out of any changes and leave it for you to decide what you want to do on that. GPIO0, GPIO2, GPIO5 and GPIO15 are the strapping pins on that family of devices

For completeness, the Single page pinout for ESP32 Devkit C can be found at
(https://docs.espressif.com/projects/esp-idf/en/latest/esp32/_images/esp32-devkitC-v4-pinout.jpg)

Now, time to get back to that pull request.

[tchilton]

Pull request raised as requested.