adafruit / Adafruit_ILI9341

Library for Adafruit ILI9341 displays
406 stars 281 forks source link

#define for nRF52832 Feather unrecognized in example sketches; differs from BSP #44

Closed oesterle closed 5 years ago

oesterle commented 5 years ago

TL;DR:

Steps to reproduce:

On line 43 of graphicstest_featherwing the sketch uses a different #define identifier than used in the current nRF52832 Feather board support package:

#ifdef ARDUINO_NRF52832_FEATHER /* BSP 0.6.5 and higher! */
   #define TFT_DC   11
   #define TFT_CS   31
   #define STMPE_CS 30
   #define SD_CS    27
#endif

(touchpaint_featherwing example sketch also has this issue beginning line 46.)

In the current BSP, the board uses ARDUINO_NRF52_FEATHER rather than ARDUINO_NRF52832_FEATHER.

While I haven't tested with the Adafruit Feather nRF52840 Express, the pin assignments look correct, and verify/compile is successful with that board selected in the Arduino IDE.

Related discussion on Adafruit Forums: "error: 'TFT_CS' was not declared in this scope" with graphi

mperino commented 5 years ago

I had the same issue. I propose the following:

-#ifdef ARDUINO_NRF52832_FEATHER / BSP 0.6.5 and higher! / +#if defined(ARDUINO_NRF52832_FEATHER) || defined(ARDUINO_NRF52_FEATHER) / BSP 0.6.5 and higher! /

Thank you Oesterle, you were right on.

ladyada commented 5 years ago

well theres NRF52832 and NRF52480 and the pins are different - can you verify that we're only detecing the '832 in this line?

mperino commented 5 years ago

I'm not sure of the broader picture.
I have the nRF52832 with all headers on, plugged into a 2.4" Featherwing (order id 1992783-9644036291) . The code on git produced the errors above, and changing "ARDUINO_NRF52832_FEATHER" to "ARDUINO_NRF52_FEATHER" made it work. I figured that adding the or condition made sense as it would still work on whatever you intended, and add support for the nRF52832 that I just got. I'm more of a Python-esqe person, and new to the feather, so please excuse any misassumptions on my part.

ladyada commented 5 years ago

all good - basically "NRF52_FEATHER" could be two feathers, either the '832 or the '840 - and that pin definition section is only true for the '832. so we should figure out the exact right #define so that we dont accidentally use that pinout for the '840!

ladyada commented 5 years ago

@oesterle wanna take a stab at that?

caternuson commented 5 years ago

Currently: NRF52_FEATHER = nRF52832 NRF52840_FEATHER = nRF52840

Worth changing the nRF52832 board= in the BSP to be more explicit? https://github.com/adafruit/Adafruit_nRF52_Arduino/blob/master/boards.txt#L38 and generally match convention for nRF52840? https://github.com/adafruit/Adafruit_nRF52_Arduino/blob/master/boards.txt#L98

Or would that break a lot of existing #defines for the nRF52832?

ladyada commented 5 years ago

ok how about we add a -D define here https://github.com/adafruit/Adafruit_nRF52_Arduino/blob/master/boards.txt#L41 but keep the NRF52_FEATHER

and in this code, test for NRF52840 feather first so it doesnt get mixedup?

caternuson commented 5 years ago

That'd work too. Could also just leave as is if you're OK with NRF52_FEATHER = nRF52832. Then I think it's just a matter of matching that: https://github.com/adafruit/Adafruit_ILI9341/blob/master/examples/graphicstest_featherwing/graphicstest_featherwing.ino#L43 That #define not matching the BSP seems to be the only issue?

For the -D, are you thinking that would be -D ARDUINO_NRF52832_FEATHER

ladyada commented 5 years ago

i think thats ok for now - for nrf52830 yeah ARDUINO_NRF52832_FEATHER

caternuson commented 5 years ago

Works for me. Guess this has become a BSP repo issue now?

ladyada commented 5 years ago

yep, wanna patch this and open up an issue there ? you can tag @hathach

hathach commented 5 years ago

I updated the BSP as follow https://github.com/adafruit/Adafruit_nRF52_Arduino/commit/76cbab0b010eef5abed098fd26900f2545e151dc

Let's me know if that works

ladyada commented 5 years ago

@oesterle i think this is resolved - let me know if not and we can re-open!