Duty cycle as integer rather than float

[Laserjones]

As far as I understand, currently the duty cycle needs to be specified as a float value bewteen 0.0 and 100.0 (percent), which is then turned into a counter compare value (integer) by the library. This means that several floating point operations are required each time the duty cycle is changed. This can add up to quite a lot of floating point operations if PWM is used, for example, to output analog waveforms at high frequencies, where the duty cycle can change thousands of times per second. And as far as I understand, these operations consume quite some time, as the RP2040 does not have a floating point unit.

Could it therefore make sense to add an option to provide the duty cycle as an integer directly (i.e. between 0 and the TOP value)? In many cases, e.g. when waveforms are read from a lookup table of samples, the programmer already knows the absolute integer values anyway, and it would make little sense to first convert them into a float percentage value and then immediately reverse that conversion in the library.

An error should then be issued if the duty cycle value exceeds the TOP value.

[khoih-prog]

This can add up to quite a lot of floating point operations if PWM is used, for example, to output analog waveforms at high frequencies, where the duty cycle can change thousands of times per second.

You're correct about this. I didn't think that this demanding use-case would use this library at all, but would write the registers directly using look-up table.

But the idea is interesting and I'll add this function

bool setPWM_manual(uint8_t pin, uint16_t top, uint8_t div, uint16_t level, bool phaseCorrect = false)

to bypass all the calculations / displays.

An error should then be issued if the duty cycle value exceeds the TOP value.

If level/duty_cycle > TOP, it will be trimmed automatically to TOP.

---

You have to prepare a lookup table similar to

typedef struct 
{
  uint16_t top;
  uint8_t div;
  uint16_t level;
} PWD_Data;

PWD_Data PWM_data[NUM_PWM_POINTS] =
{
  { top1, div1, level1 },
  { top2, div2, level2 },
  ......
};

then call the function manually like

for (int index = 0; index < NUM_PWM_POINTS; index++)
{
  setPWM_manual(pin, PWM_data[index].top, PWM_data[index].div, PWM_data[index].level, true);
  // delay something here
  ...
}

Is this OK to you ??

I also need you to test using the current version to see how the float calculation would affect the waveform. Then compare to the new release to be sure if it's much better and worth to use.

I'll publish the new release within some hours.

[khoih-prog]

Hi @Laserjones

The new RP2040_PWM releases v1.2.0 has just been published. Your contribution is noted in Contributions and Thanks

Try and modify the new PWM_Waveform example according to your use-case and report back if there is any bug to fix or enhancement to add

Best Regards,

---

Releases v1.2.0

1. Add efficient setPWM_manual() function to use in wafeform creation using PWM. Check Duty cycle as integer rather than float #6
2. Add example PWM_Waveform to demonstrate how to use new setPWM_manual() function in wafeform creation
3. Optimize library code and examples by using reference-passing instead of value-passing.

[Laserjones]

Wow, the three coffees I bought you really kept you awake! :-D

This will surely help, thanks a lot! I'll check it out in detail later.

I'm just wondering whether it is actually necessary to pass all four parameters each time (which probably means that four registers are written to each time, too?). Maybe I underestimate the scope of possible PWM applications, but all use cases I can imagine would require only the duty cycle to be changed frequently, whereas the TOP and the frequency would be set only once to initialize the PWM output (not sure about the phase correction, as I have no idea what its practical use is). So three out of four registers would unnecessarily be written to in spite of staying unchanged.

So, from a humble Arduino programmer's point of view, who likes everything as simple as possible, I would wish for something like:

PWM_init(uint8_t pin, uint16_t top, float frequency, uint16_t startlevel, bool phaseCorrect) // to be called once

PWM_write_absolute(uint8_t pin, uint16_t level) // level as integer, 0 to counter TOP
PWM_write_percent(uint8_t pin, float level) // level in percent, 0.0 to 100.0

That would be close to the familiar AnalogWrite() function of Arduino, which basically does the same (but in software with only 256 duty cycle values).

[Laserjones]

Even regardless of any performance advantage, I find it definitely useful to have this option of duty cycle as integer, because - as mentioned - in many applications the values used by the programmer will be integers of a known range anyway, so the conversion to float percent and back not only adds unnecessary computing load, but also unneccesary calculations in the program code. For example, audio waves usually are simply an array of 8-bit or 16-bit values, which can directly be handed over to the PWM output if the TOP is configured to be above the maximum wave amplitude. No need for percent values.

[khoih-prog]

This is the reason why I use float instead of uint16_t for duty-cycle

khoih-prog/ESP8266_PWM#1 (comment)

I'm similarly trying to change the duty cycle with a fixed frequency. Since PWM_DutyCycle is a uint32_t , this can only be done in 100 steps. I suggest to have an alternative function where you can specify Period/OnTime to be able to get the full duty cycle resolution. Or change PWM_DutyCycle to a float (I think 0. to 1. would be easier to use than 0-100 if you do this).

Using uint16_t only is wasting the resolution of TOP 16-bit register.

I think I'll use either

1. uint16_t with 100 * duty_cycle => 50% == 50 * 100 = 5,000
2. uint32_t with 1000 * duty_cycle => 50% == 50 * 1000 = 50,000

I prefer option 2. to have perfect resolution without adding burden to the MCU

[khoih-prog]

I'm just wondering whether it is actually necessary to pass all four parameters each time (which probably means that four registers are written to each time, too?). Maybe I underestimate the scope of possible PWM applications, but all use cases I can imagine would require only the duty cycle to be changed frequently, whereas the TOP and the frequency would be set only once to initialize the PWM output (not sure about the phase correction, as I have no idea what its practical use is). So three out of four registers would unnecessarily be written to in spite of staying unchanged.

It's possible to optimize like that, but I'm against the idea of modifying the individual register by accessing it directly. It's always better to use the SDK-provided functions to be compatible across platforms and releases.

It's always possible the core's functions has been optimized to modify the registers only if changed.

If the use-case is so demanding to be optimized up to each register access, you have to write the code to access the registers directly, without using the core SDK and the library. Even using assembly code, not C/C++.

So. I won't go to that extreme and make the library so hard to use.

Anyway, don't hesitate to propose the new ideas. Without discussion, we all have the tunnel-vision and don't have a clear picture of what's better.

[Laserjones]

It's possible to optimize like that, but I'm against the idea of modifying the individual register by accessing it directly. It's always better to use the SDK-provided functions to be compatible across platforms and releases.

As far as I can see, there's no need to access registers directly, because the SDK provides functions for each parameter separately (https://raspberrypi.github.io/pico-sdk-doxygen/rp2__common_2hardware__pwm_2include_2hardware_2pwm_8h.html). In fact, looking at your code, it seems like you are doing just that: Your function receives all the parameters at once, but then sets each register individually with SDK calls such as pwm_set_gpio_level(...).

It's always possible the core's functions has been optimized to modify the registers only if changed.

I hope so. But your function still has to handle all the parameters, even if the SDK functions then decide that the registers need not be touched. On the other hand, duplicating all the individual SDK functions in your library would not make a lot of sense. Though at least for the duty cycle, I'd prefer a function that takes care of only this one parameter.

So. I won't go to that extreme and make the library so hard to use.

Actually my thoughts were intended to make it easier to use, not harder. ;-)

But I get your point, and the more I look at your code, the more I think that I might as well call the SDK functions directly, or maybe write my own PWM library, which would be a nice challenge.

By the way, could you help me understand why you are able to call the SDK functions directly in your code without any #include or prefix? As far as I understand the explanation at the bottom of https://github.com/arduino/ArduinoCore-mbed, an mbed:: prefix should be required.

Phew, I never thought I would dive so deeply into this stuff after only some weeks of programming the Pico, with almost no previous C++ or Arduino experience. A steep curve, but definitely fun.

[Laserjones]

By the way, could you help me understand why you are able to call the SDK functions directly in your code without any #include or prefix?

Sorry, I overlooked #include "hardware/pwm.h" in your code. I assume that's the answer?

[khoih-prog]

It's so good that you dive deeply into the SDK, core, library, etc. and had a lot of fun and experience thru it. You also make me reread them as I almost forgot after some time not looking at.

The difference is just these 2 lines to program the config

RP2040_PWM/src/RP2040_PWM.h

Lines 206 to 207 in 51e1826

	pwm_config_set_clkdiv_int(&config, _PWM_config.div);
	pwm_config_set_wrap(&config, _PWM_config.top);

I can create a new function to let you not passing the top and div, just the level as

bool setPWM_manual(const uint8_t& pin, uint16_t& level, bool phaseCorrect = false);

The compiler will auto selects the correct function accordingly.

A new release will be published soon with these features

1. uint32_t with 1000 * duty_cycle => 50% == 50 * 1000 = 50,000
2. this new setPWM_manual function

[khoih-prog]

Hi @Laserjones

The new RP2040_PWM releases v1.3.0 has just been published. Your contribution is noted in Contributions and Thanks

Try and modify the new PWM_Waveform_Fast example according to your use-case and report back if there is any bug to fix or enhancement to add

Best Regards,

---

Releases v1.3.0

1. Add setPWM_manual(pin, level) function for efficiency in wafeform creation using PWM. Check Duty cycle as integer rather than float #6
2. Add example PWM_Waveform_Fast to demonstrate how to use new setPWM_manual(pin, level) function.
3. Add setPWM_Int() function for optional uint32_t dutycycle = real_dutycycle * 1000. Check Duty cycle as integer rather than float #6
4. Add example PWM_DynamicDutyCycle_Int to demonstrate how to use new setPWM_Int() function.
5. Rewrite many functions to take advantage of new features.