moved the implementation of 'getMaxDuty()' into the cpp file for all …

[pljakobs]

…architectures. Before, for esp32c3, the function in header would always return 0.

[what-the-diff]

PR Summary

• Introduction of getMaxDuty() function:
  • This new function has been added in the program that different hardware architectures (Esp32, Esp8266, Host, and Rp2040) utilize. The purpose of this function is to find out the highest duty cycle possible.
• Modification to HardwarePWM.h file:
  • getMaxDuty() has been defined in a different way than before. Previously, it was fully written out in this file, but with this update, only its blueprint (or 'function prototype') is present here, meaning the details of how this function works are written in another file. This approach helps to keep code organized and easier to maintain.

[mikee47]

With #2904 presumably this PR is no longer required?

[pljakobs]

this one is about moving the getMaxDuty() from the header file into the implementation.
I don't quite understand why the simple return maxduty from the header didn't work for ARCH Esp32 but for Esp8266. It works for both when the function is implemented in the .cpp file.
#2904 just adds an assert() for max_channels - has nothing to do with this one. I hope I managed to properly untangle the two changes.

[mikee47]

I don't quite understand why the simple return maxduty from the header

That's my concern. I can see no logical reason why this PR is necessary so something else is clearly going on.

[pljakobs]

I share the concern, but:
I added a HardwarePWM::myMaxDuty() function that just did the same thing, return maxduty, the only difference was that it was implemented in the .cpp file and only the prototype was in the header.
Calling both functions in the test app one after the other returned 0 for the original getMaxDuty() and the correct 1023 from myGetDuty(). I could not see any reason for this, but I would be very happy to find one.

it's correct that, if the HardwarePWM::HardwarePWM constructor is called with more than max_channels pins, the constructor in the old version would silently fail and therefore maxduty would not be set and remain 0 - but in the tests I did, that was not the case.

[mikee47]

I've spun up the Basic_HwPWM sample for the esp32c3 to check this. This is what I found:

1. Remove COMPONENT_SOC definition from component.mk to allow sample to build for other architectures.
2. Add debug statement in init to verify maxDuty.
3. Flash to esp32c3. This produces a boot-loop:

HW PWM maxDuty = 0
E (292) ledc: ledc_set_duty(805): LEDC is not initialized

4. The esp32c3 only has six PWM channels. Change pins to LED_PIN, 4, 5, 0, and application starts OK:

HW PWM maxDuty = 1023
PWM output set on all 8 Pins. Kindly check...
Now LED_PIN will go from 0 to VCC to 0 in cycles.

[pljakobs]

I've spun up the Basic_HwPWM sample for the esp32c3 to check this. This is what I found:

1. Remove `COMPONENT_SOC` definition from `component.mk` to allow sample to build for other architectures.

2. Add debug statement in `init` to verify `maxDuty`.

3. Flash to esp32c3. This produces a boot-loop:

HW PWM maxDuty = 0
E (292) ledc: ledc_set_duty(805): LEDC is not initialized

4. The esp32c3 only has _six_ PWM channels.  Change `pins` to `LED_PIN, 4, 5, 0,` and application starts OK:

HW PWM maxDuty = 1023
PWM output set on all 8 Pins. Kindly check...
Now LED_PIN will go from 0 to VCC to 0 in cycles.

are you saying you're getting this result with the getMaxDuty() implemented in the .h file?
I need to re-test this, but I had both issues independently from each other.

[pljakobs]

okay, I think I can confirm this.
I don't quite understand it, though.

[mikee47]

Bear in mind that the sample statically initialises the HardwarePWM instance, so that happens before the UART has been initialised for debug output.

[mikee47]

It's a pity that HardwarePWM doesn't follow the usual model where initialisation occurs in a begin() method, rather than in the constructor.

[mikee47]

are you saying you're getting this result with the getMaxDuty() implemented in the .h file?

Yes. I saw exactly what I expected to see.