-
Notifications
You must be signed in to change notification settings - Fork 3k
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
PmwOut: Add method to enable/disable PWM #11641
Conversation
drivers/source/PwmOut.cpp
Outdated
@@ -98,6 +98,19 @@ void PwmOut::pulsewidth_us(int us) | |||
core_util_critical_section_exit(); | |||
} | |||
|
|||
void PwmOut::enable_pwm(bool enabled) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we actually need to pwmout_free
to free the HW (and then to restore the state)? I'm not sure what will happen with HW state if the high frequency clock would go down and also HW may limit the low power state based on active HW.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hugueskamba, thank you for your changes. |
014b58e
to
b2f882b
Compare
This force-push splits the previously introduced method ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Not related to this PR but this class forces users to use floats 😞
b2f882b
to
1645cc7
Compare
This force-push explicitly mentions that calls prior to |
Not in this case - there are integer versions of both float calls, and they don't go through the float. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also need to add greentea tests (is there an existing test suite that can be expanded)?
1645cc7
to
fdc7605
Compare
There is no PWM test that I could find test except for https://github.com/ARMmbed/mbed-os/tree/master/TESTS/mbed_hal_fpga_ci_test_shield/pwm |
fdc7605
to
dcdc66b
Compare
It is now possible to temporarily suspend PWM and safely preserve the duty cycle set. This functionality is needed to allow a device to enter deep sleep as a PWM instance prevents deep sleep in order for the timer it relies on to run so its output can be modified. The duty cycle configuration can be restored upon resuming from deep sleep.
dcdc66b
to
a9496ad
Compare
This force-push fixes the Doxygen spelling error. |
Shall we add there new test cases to test resume/suspend? |
That test file is for testing the hal layer. The change is in the driver level. |
There is no test for the driver layer? |
I run CI meanwhile |
1579347
to
f6fb265
Compare
ac77d3a
to
db1c269
Compare
This force-push adds unit test cases for the constructor and the destructor. |
8bcddc4
to
6352a01
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes.
It might have been easier to use the framework to clear the expectations before the test starts but this works as well.
6352a01
to
b8bcc7f
Compare
CI started |
Test run: SUCCESSSummary: 11 of 11 test jobs passed |
Description
It is now possible to temporarily suspend PWM and safely preserve the duty
cycle set. This functionality is needed to allow a device to enter deep
sleep as a PWM instance prevents deep sleep in order for the timer it
relies on to run so its output can be modified. The duty cycle configuration
can be restored upon resuming from deep sleep.
Pull request type
Reviewers
@evedon @bulislaw @kjbracey-arm
Release Notes