Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TW#15817] MCPWM hardcoded resolution (IDFGH-2398) #1101

Closed
michprev opened this issue Oct 11, 2017 · 9 comments
Closed

[TW#15817] MCPWM hardcoded resolution (IDFGH-2398) #1101

michprev opened this issue Oct 11, 2017 · 9 comments
Assignees
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally

Comments

@michprev
Copy link

michprev commented Oct 11, 2017

Hello,
according to 'ESP32 Technical Reference Manual' MCPWM uses 16-bit counter with 8-bit clock prescaler. In 'mcpwm_set_duty_in_us' we set compare value to duty time in microseconds so that MCPWM timer tick frequency is always 1 MHz no matter what frequency we set in 'mcpwm_set_frequency'. Am I right?

In this case we do not get 16-bit PWM resolution, e. g. when we set frequency to 2 kHz (period 500 us) we can get only 500 different values.

EDIT: We need to make MCPWM_CLK_PRESCL and TIMER_CLK_PRESCALE configurable

@FayeY FayeY changed the title MCPWM hardcoded resolution [TW#15817] MCPWM hardcoded resolution Oct 13, 2017
@costaud costaud self-assigned this Oct 18, 2017
michprev added a commit to michprev/flyhero-esp32-idf that referenced this issue Nov 20, 2017
…tead of 1 MHz

In our application we want to generate signal behaving like Oneshot125 where 125 us (1000 ticks) is minimum pulse and 250 us (2000 ticks) maximum

Fix requested at espressif#1101
@panfeng-espressif
Copy link
Contributor

Hello michprev,
I am quite sorry for the late response. I wonder whether you have solved this issue?
When we call mcpwm_set_frequency, the frequency we set is the frequency of the PWM pulse, not the tick frequency. So the answer to your first question is YES.
The answer to your second question is also YES. When the period of PWM pulse is 2khz, the output values is at most 500. So the maximum frequency your application could use is 4khz.
I think you want a higher frequency, is that correct?

@michprev
Copy link
Author

Hello @panfeng-espressif,
no problem.

I would like to use up to 32 kHz frequency with 2048 output values (if possible). However, if noone else ever faced this problem I think that there is no need to change MCPWM API - I can make my own MCPWM driver.

I think that it is a good idea to include something like "max_output_values = 1000000 / frequency" in the docs, though.

@panfeng-espressif
Copy link
Contributor

Maybe you could use mcpwm_set_duty instead. In this way, you need to input a percentage to control the duty.

@michprev
Copy link
Author

Yes but this does not change number of the output values. It sets timer compare values to a percentage of the timer period which is defined at

mcpwm_num_of_pulse = MCPWM_CLK / (frequency * (TIMER_CLK_PRESCALE + 1));

With 2 kHz frequency there are still 500 different output values.

@panfeng-espressif
Copy link
Contributor

You can set percentage from 0.1% to 99.9%. So it's up to 1000 different output values.
In this way, it's nothing to do with pulse number.
When you set a high frequency, the difference between 0.1% duty is not very clear. And actually we haven't receive some requirement like 32khz and 2048 different values. I'm quite sorry for that

@michprev
Copy link
Author

I don't think that you are right. Please consider following code:

extern "C" void app_main(void)
{
    ESP_ERROR_CHECK(mcpwm_gpio_init(MCPWM_UNIT_0, MCPWM0A, GPIO_NUM_25));

    mcpwm_config_t pwm_config;
    pwm_config.cmpr_a = 0;
    pwm_config.cmpr_b = 0;
    pwm_config.counter_mode = MCPWM_UP_COUNTER;
    pwm_config.duty_mode = MCPWM_DUTY_MODE_0;
    pwm_config.frequency = 2000;

    ESP_ERROR_CHECK(mcpwm_init(MCPWM_UNIT_0, MCPWM_TIMER_0, &pwm_config));

    ESP_ERROR_CHECK(mcpwm_start(MCPWM_UNIT_0, MCPWM_TIMER_0));

    printf("--------------------------\n");

    ESP_ERROR_CHECK(mcpwm_set_duty(MCPWM_UNIT_0, MCPWM_TIMER_0, MCPWM_OPR_A, 0));
    ESP_ERROR_CHECK(mcpwm_set_duty(MCPWM_UNIT_0, MCPWM_TIMER_0, MCPWM_OPR_A, 0.1));
    ESP_ERROR_CHECK(mcpwm_set_duty(MCPWM_UNIT_0, MCPWM_TIMER_0, MCPWM_OPR_A, 0.2));
    ESP_ERROR_CHECK(mcpwm_set_duty(MCPWM_UNIT_0, MCPWM_TIMER_0, MCPWM_OPR_A, 0.3));
    ESP_ERROR_CHECK(mcpwm_set_duty(MCPWM_UNIT_0, MCPWM_TIMER_0, MCPWM_OPR_A, 0.4));
    ESP_ERROR_CHECK(mcpwm_set_duty(MCPWM_UNIT_0, MCPWM_TIMER_0, MCPWM_OPR_A, 0.5));
}

If you insert printf("%d duty\n", set_duty); right after this line

set_duty = (MCPWM[mcpwm_num]->timer[timer_num].period.period) * (duty) / 100;

the result is:

0 duty
0 duty
--------------------------
0 duty
0 duty
1 duty
1 duty
2 duty
2 duty

As you can see every second value is the same as the previous one so that there are only 500 output values.
If we doubled up frequency we would get even only 250 different output values.

@panfeng-espressif
Copy link
Contributor

Yes, you are right.
Based on different pwm frequency, the number of output values will range.

@drenehtsral
Copy link

I'd like to second this! It's better not to have a driver for the MCPWM peripheral at all than to include one that is unsuitable for most applications the underlying peripheral is actually capable of.

I don't know that there is a simple way to make this sort thing into a tidy user-friendly API and for the most part you wouldn't even want to bother given that in most such applications you will also want to update the underlying peripheral registers from the PWM ISR (e.g. set the duty cycle for the next PWM period by advancing an index in a waveform lookup table) and most of the MCPWM API functions are not ISR safe (they either live in a far flung region that is not feasible to relink into fast RAM or try to take locks).

The driver is useful as example code -- a reference for the minimum set of registers one must touch to kick it off (especially non-obvious ones like clock gates and the like which are not always clearly cross-referenced with the peripheral registers) but god help you if you actually try and use the driver in a real motor/power control application (!).

@github-actions github-actions bot changed the title [TW#15817] MCPWM hardcoded resolution [TW#15817] MCPWM hardcoded resolution (IDFGH-2398) Dec 18, 2019
@costaud
Copy link
Collaborator

costaud commented Dec 19, 2019

@drenehtsral Agree with you that it's really hard to provide one single set of APIs in such flexible and various application scenarios. In most real-time applications, user would be able to apply their own implementation via register access.

@espressif-bot espressif-bot added Resolution: Won't Do This will not be worked on Status: Done Issue is done internally labels May 7, 2021
@espressif-bot espressif-bot added Status: Opened Issue is new Status: In Progress Work is in progress and removed Resolution: Won't Do This will not be worked on Status: Done Issue is done internally Status: Opened Issue is new labels Jul 19, 2021
@espressif-bot espressif-bot added Status: Done Issue is done internally Resolution: Done Issue is done internally and removed Status: In Progress Work is in progress labels Jul 31, 2021
espressif-bot pushed a commit that referenced this issue Aug 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally
Projects
None yet
Development

No branches or pull requests

6 participants