Yeah, I can't imagine any application thinking about PWM in terms of clock ticks. I also see no reason to set frequency and duty separately. I would use an API with two primary functions:
int hal_pwm_enable(int channel, uint32_t hz, int duty); int hal_pwm_disable(int channel); I'd suggest 'duty' is expressed as a [0,65535] value with 32768 as 50%. This gets to within 0.002% accuracy. Note "channel" could also be called "pin", as that's what we're really talking about here. (I dunno what typical newt APIs look like) I would further suggest an error only if the frequency is too fast. Round off freq and duty when needed. The application can use various query functions to determine if the precision is appropriate for its needs, without complicating actual use. I have no feedback, other than Paul's direction looks good. Cheers, -g On Mar 30, 2016 8:13 PM, "will sanfilippo" <[email protected]> wrote: > I am not a huge fan of the API using clock ticks as opposed to time. I > wonder if the API should just be frequency and duty cycle. If the > underlying HW cant support it it can return an error. I have not thought > this through completely so I am sure there is some reason this is not good > :-) > > I guess one possible issue is the issue where the HW cant support the > exact frequency. For example, I ask for 1kHZ but can only get 1.1 or .9. > > Anyway, I dont have an alternate proposal so maybe I shouldnt comment :) > > Will > > > On Mar 30, 2016, at 9:56 AM, [email protected] wrote: > > > > > > > > A quick discussion thread on the PWM API. I currently proposed a PWM > API with five simple methods in my PWM git pull request. > > > > hal_pwm_set_period(uin32_t usec); > > hal_pwm_set_on_duration(uint32_t usec); > > hal_pwm_on(); > > hal_pwm_off(); > > hal_pwm_create(); > > > > The on/off/create APIs are fine, and I don't intend to change them. > > > > But the setting APIs assume a lot about the underlying PWM controller > setup. Mainly that it has lots of resolution (clock rate) and lots of > precision (bits of PWM register). The goal of this simple API was to > abstract the HW enough to make it simple to use, but I think I've got > overboard and made it unusable. So I want to propose a new HAL API like > this. > > > > /* returns the count frequency for the underlying PWM. This is set by > the BSP and HW limitations */ > > uint32_t hal_pwm_get_clock_freq_hz(void) > > > > /* returns the register size of the counter regiser for the underlying > PWM. This is set the by BSP and HW limitations */ > > uint32_t hal_pwm_get_resolution_bits(void); > > > > /* sets the period of the PWM waveform in clock (clock frequency > returned above). Can't exceed the resolution above (2^N) or error */ > > Int hal_pwm_set_period(uin32_t clocks); > > > > /* sets the on duration of the PWM waveform in clocks (clock frequency > returned above). Can't exceed the resolution above or error */ > > Int hal_pwm_set_on_duration(uint32_t clock); > > > > /* sets the duty cycle of the PWM. 0=always low, 255 = always high. > > * Sets the period to the smallest possible to achieve this exact duty > cycle. Overwrites any > > * changes made by hal_pwm_set_period and hal_pwm_set_on_duration. This > is designed to be the simple API that folks > > Would use if they just want a duty cycle for controlling LED brightness > etc */ > > Int hal_pwm_set_duty_cycle(uint8_t frac); > > > > Comments? This API is a bit more complicated but lets the application > know a lot more about the underlying functionality of the PWM provided by > the BSP. > > > > Paul > > > > > >
