adafruit/Adafruit_GPS

Missing 'Serial1' instance when compiling 'GPS_HardwareSerial_EchoTest.ino' for ESP32

mrwgx3 opened this issue · 7 comments

mrwgx3 commented

Basic Infos

Hardware

Adafruit HUZZAH32 - ESP32 Feather Board
https://www.adafruit.com/product/3405

Adalogger FeatherWing - RTC + SD Add-on For All Feather Boards
https://www.adafruit.com/product/2922

Adafruit Ultimate GPS FeatherWing
https://www.adafruit.com/product/3133

FeatherWing Tripler Mini Kit - Prototyping Add-on For Feathers
https://www.adafruit.com/product/3417

OS, IDE, and Commit

OS:       Windows 10 Professional
IDE:      1.8.5
COMMIT?:  via 'git branch -v'
          * master b855eb2 * arduino-esp32 release mgmt. update (#1557)

Settings in IDE

Module:            Adafruit ESP32 Feather
CPU Frequency:     80Mhz  
Core Debug Level:  None

Description

While bringing up the forementioned hardware assembly, my colleague and I found that the basic GPS Rx/Tx test sketch would not compile for the ESP32 using the Arduino IDE.

A search of the Arduino/ESP32 repository with term 'Serial1' found turned up sketch 'WiFiTelnetToSerial.ino', which defines 'Serial1' as an instance of 'HardwareSerial'.

HardwareSerial Serial1(2); // UART1/Serial1 pins 16,17

Adding this instance to the example allowed it to be compiled.

As the Adafruit GPS library includes a sketch 'GPS_HardwareSerial_EchoTest.ino' almost identical to the basic GPS Rx/Tx test, it also needs a 'Serial1' instance to compile correctly.

// Pick one up today at the Adafruit electronics shop
// and help support open source hardware & software! -ada

// ADD THIS LINE HERE FOR ESP32
HardwareSerial Serial1(2); // UART1/Serial1 pins 16,17

// what's the name of the hardware serial port?
#define GPSSerial Serial1

Copy Author: @ladyada

ladyada commented

good idea, can you submit a pull request for that line, commented out by default?

mrwgx3 commented

@ladyada

Please review the comments added to GPS_HardwareSerial_EchoTest.ino and GPS_HardwareSerial_LOCUS_DumpBasic.ino to the 'Adafruit_GPS' library fork in my repository.

Note that comments identical to those placed in 'GPS_HardwareSerial_LOCUS_DumpBasic.ino' have been added to

GPS_HardwareSerial_LOCUS_Start.ino,
GPS_HardwareSerial_LOCUS_Status.ino, and
GPS_HardwareSerial_Parsing.ino

If you're happy with them, I'll generate the requested pull request.

ladyada commented

how about, isntead something like this?

#if defined(ESP32)
HardwareSerial Serial1(2);
#endif

mrwgx3 commented 
how about, isntead something like this?

#if defined(ESP32)
HardwareSerial Serial1(2);
#endif

Up until about (4) days ago, this would have worked. 'Serial1', and 'Serial2' are now globally defined along with 'Serial'; see last comment in closed issue espressif/arduino-esp32#1577. Even if we could still define 'Serial1' without conflict, we could not insure compatibility with future ESP32 variants.

Also since the opening of this issue, it seems I found yet another 'bug' regarding the ESP32 UART ports; see issue espressif/arduino-esp32#1607. After some thought, I do believe that a simple adjustment to HardwareSerial.begin() along with some extra definitions in the 'pins_arudino.h' variant files would lead to a clean solution, one which would require no changes to the sketches here.

Will copy you when I've posted the proposed solution.

ladyada commented

oof ok thanks! either way, #ifdef's are preferred :)

mrwgx3 commented

Proposed solution was posted at espressif/arduino-esp32#1607. It might be simpler to include the following conditional compilation instead:

#if defined ( ESP32 )
  HardwareSerial  GpsSerial( 2 );
#else
   #define  GpsSerial  Serial1;
#endif