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

Do not re-enable the timer when the user request a PWM frequency of 0. #56

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mkende
Copy link

@mkende mkende commented Apr 10, 2022

This is a tentative fix for for issue #55. I tested that this fix it on a SAMD10 chip (that, setting the PWM freq to 0 will now fully stop the PWM output). I did not test it on SAMD09 or SAMD21 chips (as I don’t have compatible hardware to test).

I’m not entirely convinced by this PR, because it might be more logical to stop the output with a call to setting the PMW value rather than the frequency. However, the current PWMWrite method in the firmware does not disable the PMW output (as opposed to the setFreq method that I’m changing), so this PR is a smaller change.

enableTimer(p.tc);
if (freq > 0){
enableTimer(p.tc);
}
#ifdef USE_TCC_TIMERS
}
else{

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

USE_TCC_TIMERS is the default for samd21. Should the else section be updated here as well? Does currently samd21 emit any PWM when frequency is set to 0?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t know, I don’t have a SAMD21 device to test the behavior, that’s why I didn’t change it.

If you think that it should be changed too I can update the code there, otherwise I would prefer to leave it to someone who can actually test it.

@mkende mkende requested a review from evtimDev May 5, 2024 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants