tinygo-org / tinygo

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

PWM.Configure produces a nil pointer dereference #1038

Closed imle closed 2 months ago

imle commented 4 years ago

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 commented 4 years ago

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!

imle commented 4 years ago

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 commented 4 years ago

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

imle commented 4 years ago
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 commented 4 years ago

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 commented 4 years ago

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

aykevl commented 4 years ago

@imle can you try https://github.com/tinygo-org/tinygo/pull/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 commented 4 years ago

@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 commented 4 years ago

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 commented 2 months ago

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