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

ports/esp32: pwm: prevent duty glitch on init #12650

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

Conversation

karlp
Copy link

@karlp karlp commented Oct 10, 2023

When a new channel was assigned, configure_channel would seet the duty to 50% before resetting to the actual user specified duty. This results in unintentional glitches.

Example code

        p1 = machine.PWM(self.pin1, self.pwm_freq, duty_u16=0)

This would brieflt glitch a pwm motor driver when it's created, instead of leaving it at duty=0 as intended.

Instead, we calculate the duty into native units first, and apply it separately in a second step where needed.

I'm not entirely in love with the split setters, but it made for the least intrusive diffs, to clearly show the problem.

When a new channel was assigned, configure_channel would seet the duty
to 50% before resetting to the actual user specified duty.  This results
in unintentional glitches.

Example code
```
        p1 = machine.PWM(self.pin1, self.pwm_freq, duty_u16=0)
```
This would brieflt glitch a pwm motor driver when it's created, instead of
leaving it at duty=0 as intended.

Instead, we calculate the duty into native units first, and apply it
separately in a second step where needed.

Signed-off-by: Karl Palsson <karlp@tweak.au>
@ricksorensen
Copy link
Contributor

See pull requests:
PR #10854

and discussion:
#12465

This has been sitting around for a while ...

@karlp
Copy link
Author

karlp commented Oct 10, 2023

Yes, I discovered this while testing out #10854. This is much less intrusive, doesn't involve other exernal libs, and given the impact of it, seemed worthy of submitting on it's own, rather than the rest of the "consistency" fixes that old PR attempts to address.

karlp added a commit to karlp/helloween2 that referenced this pull request Oct 11, 2023
See micropython/micropython#12650

It's not _critical_ but it's nice to not glitch the motor when you are
trying to init the pins.
@projectgus
Copy link
Contributor

This is an automated heads-up that we've just merged a Pull Request
that removes the STATIC macro from MicroPython's C API.

See #13763

A search suggests this PR might apply the STATIC macro to some C code. If it
does, then next time you rebase the PR (or merge from master) then you should
please replace all the STATIC keywords with static.

Although this is an automated message, feel free to @-reply to me directly if
you have any questions about this.

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

Successfully merging this pull request may close these issues.

None yet

4 participants