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

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

Open
soypat opened this issue Jan 25, 2022 · 7 comments

Comments

@soypat
Copy link
Contributor

soypat commented Jan 25, 2022

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
Copy link
Contributor Author

soypat commented Nov 8, 2022

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
Copy link
Member

aykevl commented Nov 17, 2022

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
Copy link
Contributor Author

soypat commented Nov 17, 2022

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
Copy link
Contributor Author

soypat commented Dec 31, 2022

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
Copy link

biohazduck commented Feb 23, 2023

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
Copy link

0pcom commented Mar 21, 2024

@soypat @biohazduck extremely helpful, thanks

@mbe81
Copy link

mbe81 commented Nov 2, 2024

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

I think this interface should be added to the PWM package itself so it will be easier to initialize PWM in main and pass it to a function where it is controlled.

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

No branches or pull requests

5 participants