PWM seems to be completely broken on nightly builds for both Pico and Pico-W

[gregwa1953]

I had some code working well under the original 1.20 release, but when using any of the fairly recent nightly builds nothing happens.

here is an example. It to a motor driver board.

class motor_driver():
    def __init__(self,M1A,M1B,M2A,M2B):
        
        self.M1A = machine.PWM(machine.Pin(M1A))
        self.M1B = machine.PWM(machine.Pin(M1B))
        self.M2A = machine.PWM(machine.Pin(M2A))
        self.M2B = machine.PWM(machine.Pin(M2B))

The called function (once the init is done) is here...

def speed(self,speedLeft, speedRight):
        speedLeft = self.convert(speedLeft,0,100,0,65534)
        speedRight = self.convert(speedRight,0,100,0,65534)
        if speedLeft > 0:
            self.M1A.duty_u16(speedLeft)
            self.M1B.duty_u16(0)
        else:
            self.M1A.duty_u16(0)
            self.M1B.duty_u16(abs(speedLeft))

        if speedRight > 0:
            self.M2A.duty_u16(speedRight)
            self.M2B.duty_u16(0)
        else:
            self.M2A.duty_u16(0)
            self.M2B.duty_u16(abs(speedRight))
            
    def brake(self):
        self.speed(0,0)

Calling the code from another module is simply like this...

from motor_driver import *         # import motor_driver library

# Drive Motor

# Initialize motor object
motor = motor_driver(4,5,2,3)      # M1A = 4, M1B = 5, M2A = 2, M2B = 3

def move_forward():
    motor.speed(50,50)                 # move forward at speed 50
    utime.sleep(5)                     # sleep 5 second

def turn_left():
    motor.speed(0,50)                  # turn left at speed 50
    utime.sleep(5)

As I said, it worked under the oringinal 1.20 release, but now the nightly builds don't do anything.

Gregwa

[scruss]

Do you ever set the frequency anywhere?

[jimmo]

Yep, exactly this. Thanks @scruss

@gregwa1953 This was a chance since v1.20 to fix an inconsistency between ports. It is now required to set the frequency
84302b2
It will be part of the release notes for v1.21

Sorry about this! This is one of those "can't make everyone happy" changes unfortunately. The previous behavior was clearly broken (it doesn't make sense to run the PWM with an undefined frequency) and it's important to be consistent across ports but of course changing this means that existing code that essentially did work now breaks.

[scruss]

tbh, PWM code without an explicit frequency should break, because not enough information has been specified.

@gregwa1953 - you seem to declare things as "broken" (like the I2C discussion before) when you've not specified enough parameters. Don't try to get away with writing the shortest line of code that works now: specify everything you need, and future you will thank you for the clarity when you come to reuse/extend/refactor the code. If, however, fully specified parameters fail to work that used to work fine, that might indicate a breaking change, and it could need attention.

[gregwa1953]

First, thank you again @jimmo.
@scruss. FIrst, your first comment was totally unhelpful. In the original 1.20 code if you were to try the code from the documentation at https://docs.micropython.org/en/latest/rp2/quickref.html#pwm-pulse-width-modulation, you would see that it throws an error .

from machine import Pin, PWM

# create PWM object from a pin and set the frequency of slice 0
# and duty cycle for channel A
pwm0 = PWM(Pin(0), freq=2000, duty_u16=32768)

The error it fails with is...

Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
TypeError: function doesn't take keyword arguments

Secondly, the driver for the motor board was written by a vendor for their board. It worked under 1.19. It worked under 1.20.

Don't try to get away with writing the shortest line of code that works now: specify everything you need, ...

That in itself is one of the most rude comments I've seen in a long time, given the fact that the MicroPython documentation FAILS as soon as it is called on the original 1.20 build.

You seem to be happy sitting in your ivory tower and throwing snide comments and unhelpful one sentence "answers" like

Do you ever set the frequency anywhere?

which doesn't help, especially since the code worked on the original 1.20 build.

Did anyone tell the general programming community that this was subject to change soon?
Did anyone ASK the general programming community if this would cause any problems?
Did ANYONE TELL the general programming community that the change was made and it will cause existing projects to quit working?

Not that I'm aware of.

And by the way,

you seem to declare things as "broken" (like the I2C discussion before)

The I2C issue STILL does not work correctly. The default pins have been changed and while I2C can scan and after multiple tries find the fact that, yes, there is a device there, any attempt to read or write to any device on the "new default pins" fail horribly. Every time. Since 1.20 original. In any sane person's mind, that equals BROKEN.

I apologize to the general users here that I had to respond the way I have in this response.

If I have any more questions, I will be sure NOT to use this forum to ask. I will find some other venue to voice my concerns or questions since people like @scruss feel the need to degrade anyone who asks a question and GOD FORBID anyone suggest that something is broken.

Maybe the MicroPython group should look into communication skills.

[jimmo]

FIrst, your first comment was totally unhelpful.

FWIW it was helpful for me. (Because I had the additional context about the recent change about frequency, so I was able to respond much quicker than if I had to read and test your code myself). I spent many many hours per week responding to these posts, most as volunteer time, so any help I can get to get through it faster is very appreciated.

In the original 1.20 code if you were to try the code from the documentation at https://docs.micropython.org/en/latest/rp2/quickref.html#pwm-pulse-width-modulation, you would see that it throws an error .

Yes this is exactly the issue... the documentation was the correct behavior, we fixed the code to match.

I agree, we should have gone back and updated the v1.20 version of the documentation to match. The oversight was that the original implementation for the rp2/pico was incomplete and didn't allow setting these parameters in the constructor. We're a very small team, there's a lot of things to do and we have to focus on the things that have the most impact. Much of this work is done by volunteers, and we're grateful to @robert-hh for fixing this for the rp2/pico!

Did anyone tell the general programming community that this was subject to change soon?

This will be in the release notes for v1.21.0.

Unfortunately we don't have a mechanism for notifying the community of every little change in the nightly builds.

Did anyone ASK the general programming community if this would cause any problems?
Secondly, the driver for the motor board was written by a vendor for their board. It worked under 1.19. It worked under 1.20.

Unfortunately we can only do so much...

FWIW in terms of avoiding this sort of problem in the future -- there are two underlying issues here that we're working on improving:

1. The documentation is problematic. This type of information is spread all over the place (quick refs, API reference, etc), and it's very difficult to understand exactly what each port supports. We are in the process of a big overhaul of the docs (kindly sponsored by Google Season of Docs) with the specific goal of addressing this issue.
2. It's easy to introduce inconsistencies between the ports. The change that led to this behavior changing was part of a refactoring to make the code much more shared between ports and hopefully avoid this in the future.

The I2C issue STILL does not work correctly. The default pins have been changed and while I2C can scan and after multiple tries find the fact that, yes, there is a device there, any attempt to read or write to any device on the "new default pins" fail horribly. Every time. Since 1.20 original. In any sane person's mind, that equals BROKEN.

For others' context, this is referring to https://github.com/orgs/micropython/discussions/11429

I have replied on this issue with my own findings and a possible theory as to what you're seeing. But as far as I can tell, nothing is "failing horribly" at least in my tests, but we will need to fix this dynamic reconfiguration issue.

This was another case of "can't make everyone happy". Unfortunately when we first supported the Pico there was no "official" default pins, so we had to pick some (to match other ports). Then later Raspberry Pi introduced default pins and started publishing documentation/pinouts with their own default pins which were unfortunately different to ours. Unfortunate, but I can't see that we had any other option.

@gregwa1953 Almost everybody replying here is a volunteer (including @scruss). It can take a long time to dig into someone else's code and figure out what the problem is. The initial suggestion to set the frequency was a very good observation.

But I agree, you are right that the initial implementation of rp2/pico PWM was broken, and it's very unfortunate that we had to make changes that cause people's code to stop working, and of course infuriating when the documentation doesn't match reality. We are working to fix all of the above.

[gregwa1953]

@jimmo ,
Thank you for taking the time for the very nice, polite and informative response.

I understand the issues behind it all, I was just hoping for better communications from the MicroPython group. I totally understand that you all are "up to your ears in alligators", so again, I understand.

I have tested the required changes to the PWM definitions and everything is working as it should.
Thank you again.
Gregwa1953

[scruss]

@scruss. FIrst, your first comment was totally unhelpful.

I'm sorry. Sometimes I forget what it's like to be a beginner.

In the original 1.20 code if you were to try the code from the documentation at https://docs.micropython.org/en/latest/rp2/quickref.html#pwm-pulse-width-modulation, you would see that it throws an error

MicroPython documentation has a little quirk: /latest/ contains features that may not be implemented yet. The ‘latest’ pages have a warning box that states “This is the documentation for the latest development branch of MicroPython and may refer to features that are not available in released versions.” The documentation for /v1.20.0/, however, contains code that does work: PWM (pulse width modulation)

from machine import Pin, PWM

pwm0 = PWM(Pin(0))      # create PWM object from a pin
pwm0.freq()             # get current frequency
pwm0.freq(1000)         # set frequency
pwm0.duty_u16()         # get current duty cycle, range 0-65535
pwm0.duty_u16(200)      # set duty cycle, range 0-65535
pwm0.deinit()           # turn off PWM on the pin

Did anyone tell the general programming community that this was subject to change soon? …

That's not really how the MicroPython development team works. It's a very small team. They decide how things should work, mostly; in the same way that CPython is designed by edict from above. All the help comes from volunteers like me.

The I2C issue STILL does not work correctly. The default pins have been changed and while I2C can scan and after multiple tries find the fact that, yes, there is a device there, any attempt to read or write to any device on the "new default pins" fail horribly. Every time. Since 1.20 original. In any sane person's mind, that equals BROKEN.

I'm sorry, but this is wrong. The I²C default pins work for devices that worked previously. Yes, the I2C bus example in the v1.20.0 docs points to the wrong default pins — they should be scl=Pin(5), sda=Pin(4) — but I²C has worked as it always has.

Here is a Raspberry Pi Pico with an I²C OLED and an LED attached:

You can't see the LED doing anything, but it is flickering at 10 Hz. The Raspberry Pi Pico is running stock MicroPython v1.20.0. I hope that the wiring is clear (SDA is blue, SCL is yellow).

Here is the code that is running:

# -*- coding: utf-8 -*-
# 128x32 SSD1306 OLED (I2C addr 0x3C) - SDA:4, SCL: 5
# LED for PWM on Pin 15
# for MicroPython on RP2040 - scruss, 2023-06

from machine import Pin, I2C, PWM
from ssd1306 import SSD1306_I2C
from sys import version

mpy_version = version.split(" ")[2]  # vN.XX.Y string

# I2C OLED init
i2c = I2C(0, scl=Pin(5), sda=Pin(4))  # equivalent to I2C(0)
WIDTH = 128
HEIGHT = 32
oled = SSD1306_I2C(WIDTH, HEIGHT, i2c)

# PWM init
pwm = PWM(Pin(15))

# draw on screen
oled.fill(0)
oled.text("uPython " + mpy_version, 0, 0)
oled.text("I2C addr. 0x%02x" % i2c.scan()[0], 8, 12)
oled.show()

# PWM - set freq first, then duty_u16
#  if you set duty first, PWM may start with unexpected freq
pwm.freq(10)
pwm.duty_u16(32767)
# PWM keeps going after program ends

For me, this code (both the I²C bit and the PWM bit) runs without errors and generates the output pictured. It neither “fail[s] horribly” nor is “BROKEN” in any way.

Also, the change in default channels for the RP2040 came from Raspberry Pi Ltd in March 2021 (back when MicroPython 1.14 was a thing). It took a while for people to notice the subtle change in the data sheet. Code that specifies the pins explicitly continued to work: only if I'd done something like I2C = machine.I2C(0) would I hit problems.

[gregwa1953]

Sorry about that.
Thank you for the answers.
From my limited testing this morning, it looks like adding the frequency breaths life back into the PWM pins.
Yes, it does make sense to have a requirement for frequency.

I'll try to do more testing today.

Gregwa