tinygo-org / tinygo

Go compiler for small places. Microcontrollers, WebAssembly (WASM/WASI), and command-line tools. Based on LLVM.
https://tinygo.org
Other
15.24k stars 898 forks source link

machine.PWM: Add function to programtically obtain PWM peripheral corresponding to a pin #2583

Open soypat opened 2 years ago

soypat commented 2 years ago

It bothers me that if I want to generate a PWM signal on an arbitrary pin on the Pico, for example, the following is the MWE to do such a thing:

var PWMs = []PWMer{machine.PWM0, machine.PWM1, machine.PWM2, machine.PWM3, machine.PWM4, machine.PWM5, machine.PWM6, machine.PWM7}
peripheral, _ := machine.PWMPeripheral(pin)
pwm := PWMs[peripheral]

which is not obvious at all to users. Maybe PWMPeripheral could instead return the PWM instance? I am available for adding this feature to all the repo.

soypat commented 1 year ago

Another take:

// GetPWM acquires a unconfigured [PWM instance]. The returned PWM
// should be configured before use. e.g:
//
//  pwm, channel, err := GetPWM(machine.GP20)
//  if err != nil {
//      panic(err)
//  }
//  err = pwm.Configure(machine.PWMConfig{Period: 1e9 / 200}) // 200Hz
//  if err != nil {
//      panic(err) // On rp2040 only error is for bad Period.
//  }
//  pwm.Set(channel, pwm.Top()/4) // 25% duty cycle.
//
// [PWM instance]: https://tinygo.org/docs/tutorials/pwm/
func GetPWM(pin machine.Pin) (pwm PWM, channel uint8, err error) {
    slice, err := machine.PWMPeripheral(pin)
    if err != nil {
        return pwm, channel, err
    }
    pwm = pwmFromSlice(slice)
    channel, err = pwm.Channel(pin)
    if err != nil {
        return pwm, channel, err
    }
    return pwm, channel, nil
}

func pwmFromSlice(i uint8) PWM {
    switch i {
    case 0:
        return machine.PWM0
    case 1:
        return machine.PWM1
    case 2:
        return machine.PWM2
    case 3:
        return machine.PWM3
    case 4:
        return machine.PWM4
    case 5:
        return machine.PWM5
    case 6:
        return machine.PWM6
    case 7:
        return machine.PWM7
    }
    panic("bad PWM value")
}
aykevl commented 1 year ago

I don't think returning a machine.PWM instance is the right way forward. It is far too easy to make mistakes that way (multiple PWM instances with different frequencies). Instead, I think it would be possible to add a Pin.SetPWM (or something similar) method that just sets a fixed frequency like in the Arduino IDE, something appropriate for LEDs. It wouldn't be possible to combine that with the more advanced use of PWM (through machine.PWM1 etc) without risking conflicts, but it would be good enough just for dimming LEDs. (Still not a big fan but I can see the appeal for people new to PWMs).

Can you share more of your use case so I can better understand the API requirements?

soypat commented 1 year ago

I think it would be possible to add a Pin.SetPWM

Hrmm, this would not solve my API case, though let me sit on it and see if maybe it ends up sounding like something I'd like to have.

Can you share more of your use case so I can better understand the API requirements?

I've designed a board that makes use of 12 of the pico's PWM. This board has several outputs which are controlled by PWM pins which are protected by an optocoupler. I want to use PWM on these pins knowing the pin numbers. To do this I need to refer to the pico's PWM peripheral documentation or solve it programatically, as I've shown above. I've ended up implementing the PWM interface and a PWMPin type:

type PWM interface {
    Top() uint32
    Set(ch uint8, value uint32)
    Channel(pin machine.Pin) (uint8, error)
    Configure(machine.PWMConfig) error
}

// PWMPin facilitates PWM usage:
//
//  pwms[i].Set(pwms[i].channel, value)
type PWMPin []struct {
    // Embedded type so methods can be used directly.
    PWM
    channel uint8
}

Maybe there is a easier way to do this? I've ran into this issue when working with smaller scope projects too. I find that the pwmGroup type is hard to use directly since it requires a channel and to get the channel one needs to refer to documentation or do what I did above.

Maybe this could be solved by adding complete documentation to a type or function in the machine package. By complete documentation I'm thinking hardware description, pins and their respective PWM slice and channel.

It is far too easy to make mistakes that way (multiple PWM instances with different frequencies).

Also, I'm not sure I understand this. I feel mistakes are always easy to make when working with hardware, for example: Maybe I set a pwmGroup to 20kHz and then use machine.Pin.SetPWM, inadvertently setting the PWM to the default 200Hz for LED dimming. I'm all ears for hearing you out though

soypat commented 1 year ago

For posterity, here's the code as would be used:

package main

import (
    "machine"
    "time"
)

// Add your PWM pins here.
var pwmPins = [...]machine.Pin{
    machine.GP0, machine.GP3, machine.GP4,
}

const numPWMPins = len(pwmPins)

// Contains facilitating methods.
var pwmHandles [numPWMPins]pwmHandle

func main() {
    var err error
    for i, pin := range pwmPins {
        pwmHandles[i].PWM, pwmHandles[i].channel, err = GetPWM(pin)
        if err != nil {
            panic(err)
        }
        err = pwmHandles[i].Configure(machine.PWMConfig{
            Period: 1e9 / 200,
        })
        if err != nil {
            panic(err)
        }
    }

    for {
        for percent := uint8(0); percent < 100; percent++ {
            for i := 0; i < numPWMPins; i++ {
                pwmHandles[i].SetPercent(percent)
            }
            time.Sleep(10 * time.Millisecond)
        }
    }
}

// pwmHandle facilitates PWM usage:
//
//  pwms[i].Set(pwms[i].channel, value)
type pwmHandle struct {
    // Embedded type so methods can be used directly.
    PWM
    channel uint8
}

// SetPercent sets the PWM with a percentage of the max duty cycle.
// percent must be in range 0..100.
func (ph *pwmHandle) SetPercent(percent uint8) {
    top := ph.PWM.Top()
    ph.PWM.Set(ph.channel, uint32(percent)*top/100)
}

type PWM interface {
    Top() uint32
    Set(ch uint8, value uint32)
    Channel(pin machine.Pin) (uint8, error)
    Configure(machine.PWMConfig) error
}

// GetPWM acquires a unconfigured [PWM instance]. The returned PWM
// should be configured before use. e.g:
//
//  pwm, channel, err := GetPWM(machine.GP20)
//  if err != nil {
//      panic(err)
//  }
//  err = pwm.Configure(machine.PWMConfig{Period: 1e9 / 200}) // 200Hz
//  if err != nil {
//      panic(err) // On rp2040 only error is for bad Period.
//  }
//  pwm.Set(channel, pwm.Top()/4) // 25% duty cycle.
//
// [PWM instance]: https://tinygo.org/docs/tutorials/pwm/
func GetPWM(pin machine.Pin) (pwm PWM, channel uint8, err error) {
    slice, err := machine.PWMPeripheral(pin)
    if err != nil {
        return pwm, channel, err
    }
    pwm = pwmFromSlice(slice)
    channel, err = pwm.Channel(pin)
    if err != nil {
        return pwm, channel, err
    }
    return pwm, channel, nil
}

func pwmFromSlice(i uint8) PWM {
    if i > 7 {
        panic("PWM out of range")
    }
    pwms := [...]PWM{
        machine.PWM0, machine.PWM1, machine.PWM2,
        machine.PWM3, machine.PWM4, machine.PWM5,
        machine.PWM6, machine.PWM7,
    }
    return pwms[i]
}
biohazduck commented 1 year ago

This is how I wrote a PWM getter today. Downside to this is having many methods in interface which are not needed for PWM operation.

type PWM interface {
    Set(channel uint8, value uint32)
    SetPeriod(period uint64) error
    Enable(bool)
    Top() uint32
    Configure(config machine.PWMConfig) error
    Channel(machine.Pin) (uint8, error)
}

func getPWM(pin machine.Pin) (PWM, uint8, error) {
    var pwms = [...]PWM{machine.PWM0, machine.PWM1, machine.PWM2, machine.PWM3, machine.PWM4, machine.PWM5, machine.PWM6, machine.PWM7}
    slice, err := machine.PWMPeripheral(pin)
    if err != nil {
        return nil, 0, err
    }
    pwm := pwms[slice]
    err = pwm.Configure(machine.PWMConfig{Period: 1e9 / 100}) // 100Hz for starters.
    if err != nil {
        return nil, 0, err
    }
    channel, err := pwm.Channel(pin)
    return pwm, channel, err
}
0pcom commented 6 months ago

@soypat @biohazduck extremely helpful, thanks