meshtastic / framework-portduino

An Arduino API that sits on top of Linux and other operating systems. This lets you run Arduino code on Raspberry PI, desktops, etc... All as a standard user-space application.
GNU Lesser General Public License v2.1
22 stars 11 forks source link

Add error handling for LinuxSerial::available ioctl call #13

Closed titan098 closed 5 months ago

titan098 commented 5 months ago

Problem

If LinuxSerial::available() is called when the port has not been opened the ioctl will currently silently fail (from a EBADF) and return an undefined value for bytes.

Reproduction

On a test environment consisting of a:

  1. RaspberryPi 3B+
  2. A UBlox NEO-6M module attached to /dev/ttyAMA0
  3. Native build of the meshtastic firmware for aarch64
  4. The following meshtastic config.yaml set:
    ...
    GPS:
      SerialPath: /dev/ttyAMA0
    ...

The following function will fail to to the value for x being a potentially undefined value returned by available:

https://github.com/meshtastic/firmware/blob/d604a76c7325e54122306308b5c22298ae101726/src/gps/GPS.cpp#L750

// clear the GPS rx buffer as quickly as possible
void GPS::clearBuffer()
{
   int x = _serial_gps->available();
   while (x--)
       _serial_gps->read();
}

This is called very early during startup before the begin is called on:

https://github.com/meshtastic/firmware/blob/d604a76c7325e54122306308b5c22298ae101726/src/gps/GPS.cpp#L476

...
void GPS::setGPSPower(bool on, bool standbyOnly, uint32_t sleepTime)
{
   LOG_INFO("Setting GPS power=%d\n", on);
   if (on) {
       clearBuffer(); // drop any old data waiting in the buffer before re-enabling
       if (en_gpio)
           digitalWrite(en_gpio, on ? GPS_EN_ACTIVE : !GPS_EN_ACTIVE); // turn this on if defined, every time
...

Calls to LinuxSerial::available() work as expected once LinuxSerial::begin(...) has been called.

Solution

Add a check for the return code of the ioctl used in LinuxSerial::available() and return zero on error. Returning zero appears to be consistent with the esp32 arduino core does on error: https://github.com/espressif/arduino-esp32/blob/master/cores/esp32/esp32-hal-uart.c#L624

Testing

Before change:

INFO  | ??:??:?? 0 Meshtastic hwvendor=37, swver=2.2.20.d604a76c
...
DEBUG | ??:??:?? 0 region=3, NODENUM=0xeb13edf6, dbsize=93
DEBUG | ??:??:?? 0 Read RTC time as 1706445313
INFO  | ??:??:?? 0 Setting GPS power=1

<hang>

After change:

INFO  | ??:??:?? 0 Meshtastic hwvendor=37, swver=2.2.20.d604a76c
...
INFO  | ??:??:?? 0 Setting GPS power=1
DEBUG | ??:??:?? 0 WANT GPS=1
INFO  | ??:??:?? 0 Setting GPS power=1
DEBUG | ??:??:?? 0 NeighborInfoModule is disabled
...
DEBUG | ??:??:?? 0 [GPS] Probing for GPS at 9600
INFO  | ??:??:?? 1 [GPS] Found a UBlox Module using baudrate 9600
DEBUG | ??:??:?? 1 [GPS] Module Info :
DEBUG | ??:??:?? 1 [GPS] Soft version: 7.��������������������������������������
DEBUG | ??:??:?? 1 [GPS] Hard version: ����������
DEBUG | ??:??:?? 1 [GPS] Extensions:0
WARN  | ??:??:?? 1 [GPS] Got NAK for class 06 message 3E
...
jp-bennett commented 5 months ago

There's a lot of tightening up the Linux native code could use. This seems like a pretty good start.