Fix/check valid pins

[pljakobs]

not all pins are good to use for pwm.
The first check is, if the given pin is within SOC_GPIO_VALID_OUTPUT_GPIO_MASK but depending on the module, there will be strapping pins and, most problematic probably, the pins that are connecting to the SPI Flash chip.
On the esp32c3, those are primarily SPIHD(GPIO12) and SPIWP(GPIO13)
I'm unsure what a good way forward is - in the huge majority of modules, those pins will be unavailable (see discussion here: https://esp32.com/viewtopic.php?t=38718) but making assumptions is not good either. If somebody uses or designs a board that allows using those pins, they should be able to do so.

@mikee47 what's the general SMING policy about this?
If someone configures a pin that is being used for SPI flash, the SoC won't stop them doing it, but the code will, obviously, stall (as the cpu can't fetch any code anymore) and get a watchdog timeout. That's a bit difficult to troubleshoot (ask me why I know).
This question is beyond just Hardware_PWM and also depending on the module layout, so we may not be able to do much about it. My only thought would be a make config like "ENFORCE_SAFE_GPIO_PINS" which would then disallow the use of pins that are known to be not safe such as SPIHD and SPIWP.
Extra complication: those can be remapped by the bootloader if I'm not mistaken.

[what-the-diff]

PR Summary

• Updates to pwm.h header file
  This change adds an inclusion for <soc/soc_caps.h> to our header file, ensuring that the necessary resources are properly linked. It also removes a conditional check for SOC_LEDC_CHANNEL_NUM, simplifying the file contents.

• Enhancements to HardwarePWM.cpp
  In our main code file, we incorporated some assertions to double-check that the no_of_pins variable falls within the acceptable range. This helps prevent errors that might occur if invalid values were used. We've also implemented constraints on the types of pins that can be used depending upon the specific system-on-a-chip (SoC) model, either esp32 or esp32c3. The code that would stop execution if no_of_pins was not valid, has been removed since the newly added assertions take care of this.

[mikee47]

what's the general SMING policy about this?

It's up to the application developer to be careful when selecting and mapping pins. Here's some thoughts on the matter.

Put all your static pin mappings in a single header file, appropriately organised, and for only one hardware revision: no #ifdefs all over the place. Use a separate config.h for that, which #includes the appropriate versioned config, e.g #include "config-esp32c3-v1.h. This represents the physical hardware so cannot change. This simplifies manual sanity checking. IMHO, many runtime checks are redundant since validation at this level should be done before the code is even compiled.

For projects with runtime-configurable pins you need to include appropriate sanity checks in the application itself. Simple enough to do using std::bitset, verifying which pins are valid for your specific hardware setup. The hardware configuration would identify groups of such pins as 'reserved for PWM' or similar.

I had an idea ages ago that Sming could offer a way to do all this systematically. We'd have a nice editor which knows about the SOC being targetted, and allows the developer to configure the mapping for each pin. PR #2530 is as far as I've got really. Sming currently provides a JSON configuration for each SOC identifying all the pins and peripheral capabilities (e.g. Sming/Arch/Esp32/esp32c3-pindefs.txt and esp32c3-soc.json). There is a basic Kconfig-based tool, not documented yet but it's pretty basic:

• Run make pinmenu
• Select Peripherals and select just those peripherals you're interested in (e.g. LEDC)
• Go back to top menu and select Pin selections. For each pin you can select the required function.

If you save the configuration, this generates a file out/Esp32/esp32c3/pin.cfg containing the pin mappings.
This could be parsed to produce a .h configuration file, but only got as far as make list-default-pins.
This outputs appropriate definitions such as SMG_PINDEF_UART0_TXD for whichever SOC is being targetted.

Looking at Basic_HwPWM, for example, this approach could replace those conditionally-compiled pin arrays.

As you say though, this gets very complicated indeed when you consider all the different board configurations around. Probably why I run out of steam!

[mikee47]

NB. One of my pet hates is code which has vast swathes of #ifdef code everywhere. Makes it impossible to read and is just asking for problems. Instead, separate out all the hardware-dependent code into modules.

[mikee47]

NB. VSCode has a trim trailing whitespace feature which I recommend you use. Ctrl+K Ctrl+X.

[mikee47]

If you look at ledc_channel_config you'll see that this exact check is already provided LEDC_ARG_CHECK(GPIO_IS_VALID_OUTPUT_GPIO(gpio_num), "gpio_num");

[pljakobs]

sigh. I came from observing the bug that I described above (my current setting, as used on the 8266, uses the two flash pins). First, I started with asserts that would have tripped on using those, but then thought that this is the wrong place to do this.
I'm now adding a list of allowable pins per SoC to the configuration (that would be a great use case for defaults for arrays with objects in ConfigDB, btw).
This assert just seemed to be the minimal sensible thing to do. I should have assumed that someone was already sensible enough to add it.
I'll close the pr then.

[mikee47]

Yeah, I've seen designs for the esp8266 which doesn't use flash at all but boots over SDIO so can't even assume they'll never be used!

[pljakobs]

NB. One of my pet hates is code which has vast swathes of #ifdef code everywhere. Makes it impossible to read and is just asking for problems. Instead, separate out all the hardware-dependent code into modules.

I hate to make any pet (or person, for that matter) unhappy.
This code was not originally written with portability in mind (in fact, I believe, when it was first written, Sming only supported the 8266, right? that would have been 2015/2016)

I see your point and agree with it. I have underestimated the complexity of this whole project when I started out.

[mikee47]

Lot's of Arduino libraries are written like this which is why I invariably don't use them and write my own. And also why I don't use Arduino.

[pljakobs]

pinconfig

hmm... this would, in fact, only be necessary once per SoC, so it's probably reasonable to do it to have a baseline.

on the other hand, at least for UI driven changes, I will just limit the list of pins to chose from in the UI based on a per-SoC list. Actually, I want that list be part of the app-config configdb, so the firmware can also use it when the config is updated trough the api but not by the UI.

[pljakobs]

Lot's of Arduino libraries are written like this which is why I invariably don't use them and write my own. And also why I don't use Arduino.

it's what always happens when the #1 reason to do anything is to "just get this one thing working".

[pljakobs]

closing - check introduced is redundant