earlephilhower / arduino-pico

Raspberry Pi Pico Arduino core, for all RP2040 and RP2350 boards
GNU Lesser General Public License v2.1
1.98k stars 412 forks source link

Problem with WiFiNINA library for Arduino Nano Connect (`__has_include` directive) #373

Closed JAndrassy closed 2 years ago

JAndrassy commented 2 years ago

I found a strange problem for Arduino Nano Connect these lines in nina_pins.h completely disable the WiFiNINA library.

#if __has_include("WiFiNINA.h")
#  define NINA_ATTRIBUTE
#else
#  define NINA_ATTRIBUTE __attribute__ ((error("Please include WiFiNINA.h to use this pin")))
#endif

The compile messages of examples compilation are as if the library were not included. if I comment out all these lines except of # define NINA_ATTRIBUTE, the examples compile.

I know that this file is copied from Arduino mbed core for the Nano Connect. is __has_include enabled in Pico core? it is non standard

EDIT: the problem is only in Arduino IDE. Eclipse Sloeber doesn't have the problem.

earlephilhower commented 2 years ago

__has_include is a GCC thing and it is supported w/the 10.x version of GCC we're using. The SdFat library uses it to exclude its File implementation which clashes w/our own, for example.

For the #define issue, what are you building? Do you have a suggestion to fix? I don't personally have the Arduino Nano Connect, so I'm kind of working by proxy to support it.

Thx!

JAndrassy commented 2 years ago

I have a first user for the ArduinoOTA on RP2040. I don't have the Nano Connect I only try to build. The Arduino Nano Connect has ublox Nina module (esp32) with Arduino nina-fw as WiFi adapter and it is used with the Arduino WiFiNINA library. The error occurs even with the basic WiFiWebClient example of the WiFiNINA library.

The nina-fw allows control of io pins of the WiFi module. For some reasons the functions are declared at the end of nina_pins.h and the purpose of the define is to make them invalid if WiFiNINA is not in sketch. I guess the implementation of those functions is in WiFiNINA library.

khoih-prog commented 2 years ago

You can use this new WiFiNINA_Generic releases v1.8.14-2, without having to modify anything.

Have a look at WiFiWebClientRepeating on ARDUINO_NANO_RP2040_CONNECT


Releases v1.8.14-2

  1. Add support to Nano_RP2040_Connect using arduino-pico core
  2. Update Packages' Patches
earlephilhower commented 2 years ago

Looks like a dup of #376, which @khoih-prog has fixed with a new release of his library, so closing this one. If there is a real core issue, please let us know...

JAndrassy commented 2 years ago

@earlephilhower this is a different issue. it is not resolved with arduino_pins.h fix

khoih-prog commented 2 years ago

Sorry, will post a new PR soon to fix this for WiFiNINA.

khoih-prog commented 2 years ago

@jandrassy

Please test the new PR #403

I'm able now to compile and run OK with WiFiNINA for Nano_RP2040_Connect

JAndrassy commented 2 years ago

@khoih-prog, so throwing the foolproof guard away of curse makes it compile, but I see no reason why it shouldn't work. It is some strange compiler or preprocessor problem. That is of course not a problem you should solve.

khoih-prog commented 2 years ago

@earlephilhower

Can you check why there is no (__has_include) defined? Some compiler directive is missing???

That's why we can't use #if __has_include("WiFiNINA.h")

Use this to check (Ref. 4.2.9 __has_include)

#if defined (__has_include)
  #warning __has_include defined
#endif

@jandrassy

If we remove all NINA_ATTRIBUTE-related from your PR #404 as in #403, it's working OK using your sketch, LED are blinking OK. Otherwise, there is compiler error

In file included from /home/kh/.arduino15/packages/rp2040/hardware/rp2040/1.9.12/variants/arduino_nano_connect/pins_arduino.h:118,
                 from /home/kh/.arduino15/packages/rp2040/hardware/rp2040/1.9.12/cores/rp2040/Arduino.h:30,
                 from sketch/Pico_Core_Test.ino.cpp:1:
/home/kh/Arduino/Testing/WiFiNINA/Pico_Core_Test/Pico_Core_Test.ino: In function 'void setup()':
Pico_Core_Test:4:10: error: call to 'pinMode' declared with attribute error: Please include WiFiNINA.h to use this pin
    4 |   pinMode(LEDR, OUTPUT);
      |   ~~~~~~~^~~~~~~~~~~~~~
Pico_Core_Test:5:10: error: call to 'pinMode' declared with attribute error: Please include WiFiNINA.h to use this pin
    5 |   pinMode(LEDG, OUTPUT);
      |   ~~~~~~~^~~~~~~~~~~~~~
Pico_Core_Test:6:10: error: call to 'pinMode' declared with attribute error: Please include WiFiNINA.h to use this pin
    6 |   pinMode(LEDB, OUTPUT);
      |   ~~~~~~~^~~~~~~~~~~~~~
/home/kh/Arduino/Testing/WiFiNINA/Pico_Core_Test/Pico_Core_Test.ino: In function 'void loop()':
Pico_Core_Test:10:16: error: call to 'digitalWrite' declared with attribute error: Please include WiFiNINA.h to use this pin
   10 |    digitalWrite(LEDR,HIGH); // Turn On RED LED
      |    ~~~~~~~~~~~~^~~~~~~~~~~
Pico_Core_Test:12:16: error: call to 'digitalWrite' declared with attribute error: Please include WiFiNINA.h to use this pin
   12 |    digitalWrite(LEDR,LOW); // Turn Off RED LED
      |    ~~~~~~~~~~~~^~~~~~~~~~
Pico_Core_Test:14:16: error: call to 'digitalWrite' declared with attribute error: Please include WiFiNINA.h to use this pin
   14 |    digitalWrite(LEDG,HIGH); // Turn On GREEN LED
      |    ~~~~~~~~~~~~^~~~~~~~~~~
Pico_Core_Test:16:16: error: call to 'digitalWrite' declared with attribute error: Please include WiFiNINA.h to use this pin
   16 |    digitalWrite(LEDG,LOW); // Turn Off GREEN LED
      |    ~~~~~~~~~~~~^~~~~~~~~~
Pico_Core_Test:18:16: error: call to 'digitalWrite' declared with attribute error: Please include WiFiNINA.h to use this pin
   18 |    digitalWrite(LEDB,HIGH); // Turn On BLUE LED
      |    ~~~~~~~~~~~~^~~~~~~~~~~
Pico_Core_Test:20:16: error: call to 'digitalWrite' declared with attribute error: Please include WiFiNINA.h to use this pin
   20 |    digitalWrite(LEDB,LOW); // Turn Off BLUE LED
      |    ~~~~~~~~~~~~^~~~~~~~~~
exit status 1
call to 'pinMode' declared with attribute error: Please include WiFiNINA.h to use this pin

The issue now is (__has_include) is not defined and usable.

earlephilhower commented 2 years ago

__has_include is not a #define, it's a preprocessor built-in macro. You can't check for ifdef(__has_include), youe need to check for a specific GCC extension (I think).

JAndrassy commented 2 years ago

in Eclipse Sloeber I don't have the error (it uses a makefile). the error is only in Arduino IDE. -I for WiFiNINA library is missing on compiler command line. so it points to arduino-cli. something confuses it.

earlephilhower commented 2 years ago

Err, so I did read the link to the GCC 10.1 docs and it seems they do fake it even though it's not a real #define.

But, it still works fine following their examples. So, I think you have another problem somewhere...

void setup() {
  Serial.begin();
  Serial1.begin(1200);
  s.begin(1200, SERIAL_8N1);
  delay(5000);
#if defined __has_include
#if __has_include("stdio.h")
  Serial.printf("__has_include(stdio.h) = true\n");
#else
  Serial.printf("__has_include(stdio.h) = false\n");
#endif
#endif
Serial.printf("xxx\n");
}
void loop() { }

gives

12:23:23.176 -> __has_include(stdio.h) = true
12:23:23.176 -> xxx
earlephilhower commented 2 years ago

in Eclipse Sloeber I don't have the error (it uses a makefile). the error is only in Arduino IDE. -I for WiFiNINA library is missing on compiler command line. so it points to arduino-cli. something confuses it.

Library includes are added by the IDE itself, not the core. So, maybe the __has_include is not understood by the arduino-cli and so it does not search for the appropriate header path to include while building?

And, so, you have a circular dependency. nina.h isn't in the include path when arduino-cli scans for needed libs, and so the #include "nina.h" is never seen by the arduino-cli scanner.

So it seems to me that you can't check for libraries using the __has_include(), ever.

khoih-prog commented 2 years ago

I just check and __has_include() is working correctly inside WiFiNINA ibrary and elsewhere, but inside nina_pins.h, My interpretation is that it's not correct to put those __has_include() checks too early inside nina_pins.h.

This seems to be a kludge to me. I haven't found out why __has_include() is working for Arduino-mbed core inside nina_pins.h

If we move


#if __has_include("WiFiNINA.h")
  #warning __has_include("WiFiNINA.h")
  #  define NINA_ATTRIBUTE
#else
  #warning Not __has_include("WiFiNINA.h")
  #  define NINA_ATTRIBUTE __attribute__ ((error("Please include WiFiNINA.h to use this pin")))
#endif

void      NINA_ATTRIBUTE pinMode     (NinaPin pin, PinMode mode);
PinStatus NINA_ATTRIBUTE digitalRead (NinaPin pin);
void      NINA_ATTRIBUTE digitalWrite(NinaPin pin, PinStatus value);
int       NINA_ATTRIBUTE analogRead  (NinaPin pin);
void      NINA_ATTRIBUTE analogWrite (NinaPin pin, int value);

from nina_pins.h to the end of wifi_drv.h, everything will be OK, and we don't even need to check #if __has_include("WiFiNINA.h")

Just a quick thought for a direction to solve the issue.

earlephilhower commented 2 years ago

Maybe there's a saner way of going about this? Even if there is an include WiFiNINA.h there's no guarantee it's actually been included somewhere beforehand, no? Why not check for something actually defined in the header instead of just seeing it could possibly have been included?

khoih-prog commented 2 years ago

Maybe there's a saner way of going about this?

That's what I'm thinking. These pins are good only for WiFiNINA + Nano_RP2040_Connect. Putting then inside WiFiNINA is better because if WiFiNINA.h is not included, you certainly can't use those pins and get compile error.

JAndrassy commented 2 years ago

We can't change what Arduino does and putting __has_include WiFiNINA into WiFiNINA misses the purpose.

the __has_include WiFiNINA is only an extra move by Arduino for foolproof. it is not necessary for the functionality. so we can stay by the current already merged removal of this 'guard'

JAndrassy commented 2 years ago

reopened. I thought the fix was already merged, but it was not.