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

The PwmOut class does not call the pwmout_free function #3945

Closed
seppestas opened this issue Mar 16, 2017 · 4 comments
Closed

The PwmOut class does not call the pwmout_free function #3945

seppestas opened this issue Mar 16, 2017 · 4 comments

Comments

@seppestas
Copy link
Contributor

seppestas commented Mar 16, 2017

The HAL of the mbed SDK defines a pwmout_free function that needs to be implemented by the targets. However, the PwmOut class that uses the HAL does not use this function anywhere.

The de facto LPC176X target doesn't do anything in it's pwmout_free implementation, but e.g Silicon Lab's EFM32 targets requires the function to be called to enable the improved mbed sleep API. This means on EFM32 MCUs the sleep function won't make the MCU go to a low power mode when a PWMOut object is used, even when it has been freed.

This issue has also been mentioned in a comment to issue #1409.

Expected
The PwmOut class should call pwmout_free in it's descriptor, i.e:

~PwmOut() {
    pwmout_free(&_pwm);
}

or provide some other way to disable the PWM channel and low power sleep.

Actual behavior
The pwmout_free is not used by the mbed SDK used. Why then does this function exist?

@stevew817
Copy link
Contributor

1409 is not the only issue where the topic of routing destructors has come up. There's more mbed (legacy) classes that don't have their destructors implemented. I've been advocating for a while to make the change to actually instrument them, but it appears the mbed team prefers to keep compatibility with code that might have been written assuming the objects don't get destroyed.
(e.g. instantiating a PwmOut inside of a function and expecting the PWM pin to still output after returning from said function).

Anyhow, this feature request gets my vote.

@seppestas
Copy link
Contributor Author

Honestly, I would prefer to have some kind of platfom independant "disable" function to disable the PWM clock and enable the low power sleep modes again. The same goes for other drivers / peripherals. Currently constantly destructing PWM objects before sleeping and reconstructing them on wakeup seems to be "the way to do it". Not the biggest fan of this approach though, and I have yet to figure out if this causes memory fragmentation issues.

When browsing the mbed docs and forum all I found about disabling specific clocks mentioned using platform specific registers for the LPC1768. For me this defeats the purpose of the mbed SDK as a HAL. If I want to use platform specific registers I can just as well use emlib instead of mbed.

@stevew817
Copy link
Contributor

A 'solution' from our side would be to disable the PWM peripheral when all channels have been set to 0, configure those pins as a low output, and release the sleep mode. This way, we're not changing any of the existing mbed-dictated behavior nor the API.
Would that help you?

@0xc0170
Copy link
Contributor

0xc0170 commented Mar 17, 2017

This is planned to implement dtors for drivers (and to all the relevant work around those, it's not just invoke _free() HAL function) . As you might have noticed, _free HAL functions do not have well defined behavior, so its a good start to first specify what init/free should do, write tests for those, then look at drivers how they handle resources and do a proper init/deinit of resources.

Here's the previous issue report: #3106. I'll close this as duplicate (we have this referenced in the 3106 so stays as reference).

@seppestas Thanks for raising this again and describing the issue around pwm. Please add a comment to the referenced issue above, any feedback appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants