bleeptrack / picoplanet

57 stars 7 forks source link

PlatformIO support / example #5

Open xsrf opened 3 years ago

xsrf commented 3 years ago

Hey, do you have any experience with support for PlatformIO? PlatformIO requires a board definition and won't work out of the box, even if some supported boards already use the SAMD21E18A. I kind of made it work by creating my own board variant based on the Adafruit Trinket M0 but I have no clue if it was done correctly. I may add a PR if I'm confident enough, but I'd be thankful for any additional advice ;)

xsrf commented 3 years ago

Okay, I came up with a working example: https://github.com/xsrf/picoplanet/tree/platformio/example/platformio I have slightly different pin definitions in variant.cpp and variant.h though.

Since I used the Trinket M0 as template, I kept Serial1 using SERCOM0 and used SERCOM2 for I2C. Since SERCOM0 and SERCOM2 share the same pins they can be used interchangeably but of course both cannot be used at the same time. However, this allows for Serial1 with TX/RX on D1/D2.

Also, I think you got the PWM channels wrong for the LEDs. Yours is

  { PORTA,  5, PIO_SERCOM, (PIN_ATTR_DIGITAL|PIN_ATTR_PWM|PIN_ATTR_TIMER), ADC_Channel5, PWM1_CH0, TCC0_CH1, EXTERNAL_INT_5 },
  { PORTA,  6, PIO_SERCOM, (PIN_ATTR_DIGITAL|PIN_ATTR_PWM|PIN_ATTR_TIMER), ADC_Channel6, PWM1_CH1, TCC1_CH0, EXTERNAL_INT_6 },
  { PORTA,  7, PIO_SERCOM, (PIN_ATTR_DIGITAL|PIN_ATTR_PWM|PIN_ATTR_TIMER), ADC_Channel7, PWM1_CH2, TCC1_CH1, EXTERNAL_INT_7 },

But I think it should be

  { PORTA,  5, PIO_DIGITAL, (PIN_ATTR_DIGITAL|PIN_ATTR_ANALOG|PIN_ATTR_PWM|PIN_ATTR_TIMER), ADC_Channel5, PWM0_CH1, TCC0_CH1, EXTERNAL_INT_5 },
  { PORTA,  6, PIO_DIGITAL, (PIN_ATTR_DIGITAL|PIN_ATTR_ANALOG|PIN_ATTR_PWM|PIN_ATTR_TIMER), ADC_Channel6, PWM1_CH0, TCC1_CH0, EXTERNAL_INT_6 },
  { PORTA,  7, PIO_DIGITAL, (PIN_ATTR_DIGITAL|PIN_ATTR_ANALOG|PIN_ATTR_PWM|PIN_ATTR_TIMER), ADC_Channel7, PWM1_CH1, TCC1_CH1, EXTERNAL_INT_7 },

At least analogWrite() works fine with my definition for all three LEDs (see examle code). Does analogWrite() work for you in the ArduinoIDE for all three LEDs?

bleeptrack commented 3 years ago

Hey, thanks so much for your help (and sorry for my late reply)! I did a quick test and you are right: PWM is not working with my definition. Thanks a lot for checking - I'm really new into defining own board definitions for Arduino and it seems I messed up here. I'll test your version tomorrow and change it so it get's updated for the Arduino IDE. Also thanks a lot for making PicoPlanets compatible with platformIO! I never used it myself, so feel free to make a PR :)

xsrf commented 3 years ago

Well, this stuff is really not well documented... PlatformIO and Arduino IDE share kind of the same board definition schemes, but I didn't know that when I started yesterday ;) This is why I first used the Adafruit Trinket M0 as template, because it is one of the Boards already supported by Platform IO that uses the same MCU ( https://platformio.org/boards?filter%5Bmcu%5D=SAMD21E18A ). It was after that, that I learned you had to do kind of the same and only then I had a first look a your arduino board files.

I think that's kind of fortunate at the end, because if I had found your board files first, I'd just copied and used them and maybe wouldn't have found the issues.

I still have to adapt mine a little... e.g. I've chosen the blue LED to be the LED_BUILTIN one, you chose red ;)

Also, I used #define LED_R, you used static const uint8_t LED_R ... Do you know any cases where this makes a difference? I've also seen the Trinket one using some const as well, but it's inconsistent anyways. I guess it matters when you use it by reference 🤔

Regarding PlatformIO support: So far, my example comes together with the boards definition, since your board is not yet officially supported. But this makes the project kinda bulky. I don't know (yet) what the official way is to get a board submitted into the PlatformIO registry, but maybe we could have a look. At least, the example project should contain everything that is needed...

xsrf commented 3 years ago

Oh btw., just in case you'll try it out, the HID example doesn't work for me (in my platformio example). I do get a HID Keyboard in the device manager but the Media-Keys just do nothing... Never used HID so far and haven't looked into that any further for now.

bleeptrack commented 3 years ago

I did a quick check and with your pin definition, PWN works now \o/

I think that's kind of fortunate at the end, because if I had found your board files first, I'd just copied and used them and maybe wouldn't have found the issues.

Tbh I'm very happy that you found these issues! I had quite some trouble getting the definitions to work, so it's great that another person now checks them :) Sorry for your extra work though!

Also, I used #define LED_R, you used static const uint8_t LED_R ... Do you know any cases where this makes a difference? I've also seen the Trinket one using some const as well, but it's inconsistent anyways. I guess it matters when you use it by reference thinking

Not really sure what the difference is. I took most of my definition from another board, but I#m not sure which one it was.

Regarding PlatformIO support: So far, my example comes together with the boards definition, since your board is not yet officially supported. But this makes the project kinda bulky. I don't know (yet) what the official way is to get a board submitted into the PlatformIO registry, but maybe we could have a look. At least, the example project should contain everything that is needed...

From what I found they want a feature request: https://platformio.org/boards which would just be a issue in the github repo. I guess if we would provide all necessary information, the board should be quick to add. From the code base I think that we can not just add it via PR. At least I did not find the spot where the variant files are.

Oh btw., just in case you'll try it out, the HID example doesn't work for me (in my platformio example). I do get a HID Keyboard in the device manager but the Media-Keys just do nothing... Never used HID so far and haven't looked into that any further for now.

Hum, it might be because the HID library I use is not the standard one. I choose the one from NicoHood because it has MediaKeys implemented and the standard one does not :(

I'm a bit occupied in the next few days, but after that I will update the Arduino IDE definitions with your changes :) Again thank you very much for your help!

xsrf commented 3 years ago

Hum, it might be because the HID library I use is not the standard one. I choose the one from NicoHood because it has MediaKeys implemented and the standard one does not :(

I've used the correct library, but the media keys don't work on my windows pc. However, with the picoplanet connected to my android phone, they just work fine. Regular keys also work on windows. Not sure what the deal is, but it proofs that HID in general works fine.

I've now just created a PR ;)