Registering mcpwm timer event callbacks after enable (IDFGH-13051)

[askuric]

Is your feature request related to a problem?

I was wondering what are the reasons behind the choice that the mcpwm timer event callbacks (i.e. function mcpwm_timer_register_event_callbacks) can be set only before calling mcpwm_timer_enable. I am asking because the earlier version of the mcpwm api (v4.x) did not have this constraint so I guess that the hardware timers are capable of initializing the interrupts even after the enable. Is there a hardware limitation that is addressed by this constraint or was this a convenience choice?

Describe the solution you'd like.

Would you consider making this function callable after the timer enable, the line of code that would have to be removed is this one: https://github.com/espressif/esp-idf/blob/8760e6d2a7e19913bc40675dd71f374bcd51b0ae/components/esp_driver_mcpwm/src/mcpwm_timer.c#L220

Then people could enable the callbacks after the init and call the esp_intr_enable(timer) once that's done.

Describe alternatives you've considered.

An alternative could be to make the mcpwm_timer_t publicly available (not private declared in mcpwm_private.h - here). That way people who really need it could still do a hack like this:

  // set the timer state to init (so that we can call the `mcpwm_timer_register_event_callbacks` )
  // this is a hack, as this function is not supposed to be called when the timer is running
  // the timer does not really go to the init state!
  timer->fsm = MCPWM_TIMER_FSM_INIT;
  // set the callback
  mcpwm_timer_register_event_callbacks(timer, &cbs, user_data);
  // set the timer state to enabled again
  timer->fsm = MCPWM_TIMER_FSM_ENABLE;
  esp_intr_enable(timer->isr);

Additional context.

The issue of adding the timer callbacks after the timer enable is important for us at simplefoc and we'll be happy to contribute if you're in for changing this.

[suda-morris]

Hi @askuric

Only the first call of mcpwm_timer_register_event_callbacks should be before the enable function.

The reason is, in MCPWM driver, the interrupt is lazy installed. If you don't register the event callback, no interrupt service will be installed. The enable function like esp_intr_enable will enable system-like services, e.g. interrupt, power management lock and so on. So if you didn't register any callback function and then call the enable function, the driver would think you don't want any feature offered by the interrupt service.

You can change the callback function to others later BTW. There's no restriction.

May I know why you wish to register the interrupt callback after the enable?

Or can you try register empty callback function first. like

mcpwm_timer_event_callbacks_t cbs = {}; // empty callback
    TEST_ESP_OK(mcpwm_timer_register_event_callbacks(timer, &cbs, NULL));

Then after mcpwm_timer_enable, you can again call this function to regsiter a real callback.

[askuric]

Hey @suda-morris,

So in our use-case with driving BLDC motors we are providing a relatively simple and generic way of implementing different strategies of Field oriented control for steppers and BLDC motors. In our use case, everything is dynamic and only in some cases we do use the timer interrupts, and we decide if we use it or not in runtime, in order to allow the user maximum of flexibility.

So the thing is that we cannot know if the user will need the timer interrupts or not at the moment we are initialising the mcpwm. As you said, we could potentially enable the interrupt on every mcpwm enable (there can be many of them, potentially up to 6 timers) and then change their interrupt callbacks afterwards. But that creates a lot of unnecessary timer calls that might not be used at all. We would prefer to use enable the callbacks only when needed.

Or can you try register empty callback function first. like mcpwm_timer_event_callbacks_t cbs = {}; // empty callback TEST_ESP_OK(mcpwm_timer_register_event_callbacks(timer, &cbs, NULL));

Hmm, this is a good point, I did not think of that! If we enable the interrupts in this way will the function mcpwm_timer_default_isr be called even if we do not change the callbacks afterwards? https://github.com/espressif/esp-idf/blob/8760e6d2a7e19913bc40675dd71f374bcd51b0ae/components/esp_driver_mcpwm/src/mcpwm_timer.c#L361

[suda-morris]

Yes, it will get executed. If there's no user callback function being registered, mcpwm_timer_default_isr should exit quickly.

[askuric]

Ok thank @suda-morris! So in our case this is a partial solution to our problem. We would really like to avoid calling the interrupt callbacks if they are not used. These callbacks can have a significant impact (even empty) when it comes to the real-time motor control.

It would be very beneficial for us to be able to initialize the interrupt handling only if the interrupts are really used, of course in conjunction to being able to do during the time the timer is running.

I'm not sure if this feature would be something you are interested in, to have in your code. If so, we would be happy to contribute. Otherwise, we will try find a way around it.