search

tinygo-org / tinygo

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

findPinPadMapping panics (index out of range) when noPin is passed #1408

Closed conejoninja closed 3 years ago

conejoninja commented 4 years ago

On the pybadge it's not possible to use the onboard display, the error might be caused by this change : https://github.com/tinygo-org/tinygo/commit/1451eeaf414176f2b96298516f3bf2c3296f8037

SPI1_SDI_PIN is defined as noPin (255) https://github.com/tinygo-org/tinygo/blob/release/src/machine/board_pybadge.go#L103 pinPadMapping is 64 length https://github.com/tinygo-org/tinygo/blob/release/src/machine/machine_atsamd51.go#L267

Code to reproduce the issue

    machine.SPI1.Configure(machine.SPIConfig{
        SCK:       machine.SPI1_SCK_PIN,
        SDO:       machine.SPI1_SDO_PIN,
        SDI:       machine.SPI1_SDI_PIN,
        Frequency: 8000000,
    })

This fix the issue, but not sure about the implications : https://github.com/conejoninja/tinygo/commit/6c39859ec32b0d28ce3aadd0f0966079fc5cc380

sago35 commented 4 years ago

@conejoninja You can maybe set up a PB12 instead of NoPin as with the arduino. You can't read the value because it's not connected, but I think the SPI setup will work!

https://github.com/adafruit/ArduinoCore-samd/blob/master/variants/pybadge_m4/variant.cpp#L104

This fix the issue, but not sure about the implications : conejoninja@6c39859

On a different note, I think it would be good to change it so that it doesn't become panic. So I think this change is necessary as well.

sago35 commented 4 years ago

https://github.com/adafruit/ArduinoCore-samd/blob/master/variants/pygamer_m4/variant.cpp#L100-L101

PB12 is doubly defined here. This seems to be a mistake on the Arduino side.

aykevl commented 4 years ago

@conejoninja your proposed fix looks correct. Can you make a PR with it?

conejoninja commented 4 years ago

@aykevl similar code was already merged here : https://github.com/tinygo-org/tinygo/commit/db27541b1a44a903feeeef91840314a56fcdc725

we still need to consider adding an extra check in findPinPadMapping so pin/2 isn't out of range

aykevl commented 3 years ago

Sorry I didn't see that was for samd51, somehow I thought it was for samd21. Yes, for the samd51 it is indeed already fixed. See #1500.

we still need to consider adding an extra check in findPinPadMapping so pin/2 isn't out of range

Yes, #1491.

aykevl commented 3 years ago

1491 has been merged so this bug is fixed.

deadprogram commented 3 years ago

This has been released with v0.17.0 so now closing. Thanks!