Skip to content

PWM.Configure produces a nil pointer dereference #1038

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

Closed
imle opened this issue Apr 8, 2020 · 10 comments
Closed

PWM.Configure produces a nil pointer dereference #1038

imle opened this issue Apr 8, 2020 · 10 comments
Labels
docs Documentation needed enhancement New feature or request

Comments

@imle
Copy link

imle commented Apr 8, 2020

Device: arduino-nano33

The code in src/examples/pwm/pwm.go produces the following error:

panic: runtime error: nil pointer dereference

it looks like the PWM.getTimer method is returning nil from src/machine/machine_atsamd21.go:1196 which doesn't seem to have a matching set of Pin numbers to the src/machine/board_arduino_nano33.go:13 file.

Are these numbers supposed to match the pins described as having PWM?

@deadprogram
Copy link
Member

Hello @imle

The example should probably say in the comment "Arduino Uno". It does say "Change the values below to use different pins." which is the most important part.

As you surmised you need to change the values to match the pins with PWM on whatever board your are targeting. So for an Arduino Nano33 you would probably want to change the example to something like this:

const (
	redPin   = machine.D2
	greenPin = machine.D3
	bluePin  = machine.D5
)

Hope that helps!

@deadprogram deadprogram added the docs Documentation needed label Apr 12, 2020
@imle
Copy link
Author

imle commented Apr 13, 2020

Sorry if I was not clear. I did change the pins and should have included the exact code I used instead of linking to the example.

The problem is the pins that are labeled as having PWM in the code comments (which are correct according to the pinout of the arduino I am using) do not have a matching set of timers in the files specified above.

@deadprogram
Copy link
Member

Could you please provide the specific code you are trying to compile? Thanks.

@imle
Copy link
Author

imle commented Apr 13, 2020

package main

import (
	"time"

	"machine"
)

// cycleColor is just a placeholder until math/rand or some equivalent is working.
func cycleColor(color uint8) uint8 {
	if color < 10 {
		return color + 1
	} else if color < 200 {
		return color + 10
	} else {
		return 0
	}
}

func main() {
	time.Sleep(3 * time.Second)
	machine.InitPWM()

	red := machine.PWM{Pin: machine.D5}
	red.Configure()

	green := machine.PWM{Pin: machine.D3 }
	green.Configure()

	blue := machine.PWM{Pin: machine.D2}
	blue.Configure()

	var rc uint8
	var gc uint8 = 20
	var bc uint8 = 30

	for {
		rc = cycleColor(rc)
		gc = cycleColor(gc)
		bc = cycleColor(bc)

		red.Set(uint16(rc) << 8)
		green.Set(uint16(gc) << 8)
		blue.Set(uint16(bc) << 8)

		time.Sleep(time.Millisecond * 500)
	}
}

TinyGo Info

> tinygo info -target arduino-nano33
LLVM triple:       armv6m-none-eabi
GOOS:              linux
GOARCH:            arm
build tags:        cortexm baremetal linux arm atsamd21g18 atsamd21 sam sam atsamd21g18a arduino_nano33 tinygo gc.conservative scheduler.tasks
garbage collector: conservative
scheduler:         tasks

@imle
Copy link
Author

imle commented Apr 13, 2020

D5 -> PA05 -> 5
D2 -> PB10 -> 42
D3 -> PB11 -> 43

The relevant code for getTimer():

// getTimer returns the timer to be used for PWM on this pin
func (pwm PWM) getTimer() *sam.TCC_Type {
	switch pwm.Pin {
	case 6:
		return sam.TCC1
	case 7:
		return sam.TCC1
	case 8:
		return sam.TCC1
	case 9:
		return sam.TCC1
	case 14:
		return sam.TCC0
	case 15:
		return sam.TCC0
	case 16:
		return sam.TCC0
	case 17:
		return sam.TCC0
	case 18:
		return sam.TCC0
	case 19:
		return sam.TCC0
	case 20:
		return sam.TCC0
	case 21:
		return sam.TCC0
	default:
		return nil // not supported on this pin
	}
}

@imle
Copy link
Author

imle commented May 10, 2020

Just want to bump this issue and say that I don't think this is a docs issue.

@aykevl
Copy link
Member

aykevl commented May 11, 2020

@imle can you try #1095?
Most likely you are using pins that are not PWM capable, hence the panic. PR #1095 will change this to return an error instead.

@imle
Copy link
Author

imle commented May 11, 2020

@aykevl thanks for the reply. I hadn't noticed that tinygo was updated lately so I tried first with the latest version of tinygo. Oddly enough the panic is gone but the pins still do not work as expected on this hardware.

I have put two bits of code below that I have been using to test this. The first, C++, works as expected. The pins (2,3,5) all change brightness in unison. In the go code below it, they do not function as expected and only pin 3 produces any output.

I have uploaded this video to show my issue: https://youtu.be/hkBsszORMp8
timeline:
0 -> 18: Running C++ code
18 -> 24: Flashing go code
24 -> 44: Running go code

#define PIN_RED 5
#define PIN_GREEN 3
#define PIN_BLUE 2

void setup() {
  pinMode(PIN_RED, OUTPUT);
  pinMode(PIN_BLUE, OUTPUT);
  pinMode(PIN_GREEN, OUTPUT);
}

uint8_t r = 0;
uint8_t g = 0;
uint8_t b = 0;

void loop() {
  analogWrite(PIN_RED, r);
  analogWrite(PIN_GREEN, g);
  analogWrite(PIN_BLUE, b);
  
  r = (r + 8) % 255;
  g = (g + 8) % 255;
  b = (b + 8) % 255;
  
  delay(500);
}
package main

import (
	"time"

	"machine"
)

func main() {
	time.Sleep(3 * time.Second)
	machine.InitPWM()

	red := machine.PWM{Pin: machine.D5}
	red.Configure()

	green := machine.PWM{Pin: machine.D3}
	green.Configure()

	blue := machine.PWM{Pin: machine.D2}
	blue.Configure()

	var rc uint8 = 0
	var gc uint8 = 0
	var bc uint8 = 0

	for {
		red.Set(uint16(rc) << 8)
		green.Set(uint16(gc) << 8)
		blue.Set(uint16(bc) << 8)

		rc += 8
		gc += 8
		bc += 8

		time.Sleep(time.Millisecond * 500)
	}
}

@aykevl
Copy link
Member

aykevl commented May 16, 2020

Ah, I see. PWM is indeed supported on the pin but has not been implemented in TinyGo.

I might want to try making a concrete proposal for #932 that would allow all the PWMs to be exposed on this chip.

@deadprogram deadprogram added the enhancement New feature or request label May 29, 2020
@deadprogram
Copy link
Member

The entire PWM interface is now different than when this issue was entered, so now closing. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation needed enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants