Update version

* Avoid "chunkiness" of UART FIFO availability

The UART FIFO will generate an IRQ to transfer data into the SerialUART
FIFOs either every 4 received bytes, or every 4 idle byte times.  This
causes the ::available count to report "0" until either of those two
cases happen, causing a potentially delay in data becoming available to
the app.

Change the code to pull data from the HW FIFO on a read/available/peek.
Use a non-blocking mutex and IRQ disabling to safely empty the FIFO from
user space.  The mutex added to the IRQ is non-blocking and will be
a single CAS the vast majority of the time, so it should not impact the
Serial performance.

Fixes #464 and others where `setPollingMode()` was needed as a workaround.

Make sure we have all mutexes locked before we disable the port and free
the queue to avoid evil cases.

Only init the mutexes once, on object creation.

In polled mode, don't bother acquiring/releasing the fifo mutex.

When begin() is called on an already running port, call end() to clean
up the old data/etc. before making a new queue/config.  This avoids a
memory leak and potential write-after-free case.

cores/rp2040/RP2040Version.h

@@ -1,5 +1,5 @@
 #pragma once
 #define ARDUINO_PICO_MAJOR 1
 #define ARDUINO_PICO_MINOR 13
-#define ARDUINO_PICO_REVISION 0
-#define ARDUINO_PICO_VERSION_STR "1.13.0"
+#define ARDUINO_PICO_REVISION 1
+#define ARDUINO_PICO_VERSION_STR "1.13.1"

cores/rp2040/SerialUART.cpp

@@ -121,12 +121,16 @@ SerialUART::SerialUART(uart_inst_t *uart, pin_size_t tx, pin_size_t rx) {
     _rts = UART_PIN_NOT_DEFINED;
     _cts = UART_PIN_NOT_DEFINED;
     mutex_init(&_mutex);
+    mutex_init(&_fifoMutex);
 }
 
 static void _uart0IRQ();
 static void _uart1IRQ();
 
 void SerialUART::begin(unsigned long baud, uint16_t config) {
+    if (_running) {
+        end();
+    }
     _queue = new uint8_t[_fifoSize];
     _baud = baud;
     uart_init(_uart, baud);
@@ -198,6 +202,7 @@ void SerialUART::end() {
     if (!_running) {
         return;
     }
+    _running = false;
     if (!_polling) {
         if (_uart == uart0) {
             irq_set_enabled(UART0_IRQ, false);
@@ -205,9 +210,28 @@ void SerialUART::end() {
             irq_set_enabled(UART1_IRQ, false);
         }
     }
+    // Paranoia - ensure nobody else is using anything here at the same time
+    mutex_enter_blocking(&_mutex);
+    mutex_enter_blocking(&_fifoMutex);
     uart_deinit(_uart);
     delete[] _queue;
-    _running = false;
+    // Reset the mutexes once all is off/cleaned up
+    mutex_exit(&_fifoMutex);
+    mutex_exit(&_mutex);
+}
+
+void SerialUART::_pumpFIFO() {
+    // Use the _fifoMutex to guard against the other core potentially
+    // running the IRQ (since we can't disable their IRQ handler).
+    // We guard against this core by disabling the IRQ handler and
+    // re-enabling if it was previously enabled at the end.
+    auto irqno = (_uart == uart0) ? UART0_IRQ : UART1_IRQ;
+    bool enabled = irq_is_enabled(irqno);
+    irq_set_enabled(irqno, false);
+    mutex_enter_blocking(&_fifoMutex);
+    _handleIRQ(false);
+    mutex_exit(&_fifoMutex);
+    irq_set_enabled(irqno, enabled);
 }
 
 int SerialUART::peek() {
@@ -216,7 +240,9 @@ int SerialUART::peek() {
         return -1;
     }
     if (_polling) {
-        _handleIRQ();
+        _handleIRQ(false);
+    } else {
+        _pumpFIFO();
     }
     if (_writer != _reader) {
         return _queue[_reader];
@@ -230,7 +256,9 @@ int SerialUART::read() {
         return -1;
     }
     if (_polling) {
-        _handleIRQ();
+        _handleIRQ(false);
+    } else {
+        _pumpFIFO();
     }
     if (_writer != _reader) {
         auto ret = _queue[_reader];
@@ -249,7 +277,9 @@ int SerialUART::available() {
         return 0;
     }
     if (_polling) {
-        _handleIRQ();
+        _handleIRQ(false);
+    } else {
+        _pumpFIFO();
     }
     return (_writer - _reader) % _fifoSize;
 }
@@ -260,7 +290,7 @@ int SerialUART::availableForWrite() {
         return 0;
     }
     if (_polling) {
-        _handleIRQ();
+        _handleIRQ(false);
     }
     return (uart_is_writable(_uart)) ? 1 : 0;
 }
@@ -271,7 +301,7 @@ void SerialUART::flush() {
         return;
     }
     if (_polling) {
-        _handleIRQ();
+        _handleIRQ(false);
     }
     uart_tx_wait_blocking(_uart);
 }
@@ -282,7 +312,7 @@ size_t SerialUART::write(uint8_t c) {
         return 0;
     }
     if (_polling) {
-        _handleIRQ();
+        _handleIRQ(false);
     }
     uart_putc_raw(_uart, c);
     return 1;
@@ -294,7 +324,7 @@ size_t SerialUART::write(const uint8_t *p, size_t len) {
         return 0;
     }
     if (_polling) {
-        _handleIRQ();
+        _handleIRQ(false);
     }
     size_t cnt = len;
     while (cnt) {
@@ -325,7 +355,15 @@ void arduino::serialEvent2Run(void) {
 }
 
 // IRQ handler, called when FIFO > 1/8 full or when it had held unread data for >32 bit times
-void __not_in_flash_func(SerialUART::_handleIRQ)() {
+void __not_in_flash_func(SerialUART::_handleIRQ)(bool inIRQ) {
+    if (inIRQ) {
+        uint32_t owner;
+        if (!mutex_try_enter(&_fifoMutex, &owner)) {
+            // Main app on the other core has the mutex so it is
+            // in the process of pulling data out of the HW FIFO
+            return;
+        }
+    }
     // ICR is write-to-clear
     uart_get_hw(_uart)->icr = UART_UARTICR_RTIC_BITS | UART_UARTICR_RXIC_BITS;
     while (uart_is_readable(_uart)) {
@@ -343,6 +381,9 @@ void __not_in_flash_func(SerialUART::_handleIRQ)() {
             // TODO: Overflow
         }
     }
+    if (inIRQ) {
+        mutex_exit(&_fifoMutex);
+    }
 }
 
 static void __not_in_flash_func(_uart0IRQ)() {

cores/rp2040/SerialUART.h

@@ -63,7 +63,7 @@ public:
     operator bool() override;
 
     // Not to be called by users, only from the IRQ handler.  In public so that the C-language IQR callback can access it
-    void _handleIRQ();
+    void _handleIRQ(bool inIRQ = true);
 
 private:
     bool _running = false;
@@ -78,7 +78,9 @@ private:
     uint32_t _writer;
     uint32_t _reader;
     size_t   _fifoSize = 32;
-    uint8_t  *_queue;
+    uint8_t *_queue;
+    mutex_t  _fifoMutex; // Only needed when non-IRQ updates _writer
+    void _pumpFIFO(); // User space FIFO transfer
 };
 
 extern SerialUART Serial1; // HW UART 0

package.json

@@ -1,6 +1,6 @@
 {
   "name": "framework-arduinopico",
-  "version": "1.11300.0",
+  "version": "1.11301.0",
   "description": "Arduino Wiring-based Framework (RPi Pico RP2040)",
   "keywords": [
     "framework",

platform.txt

@@ -20,7 +20,7 @@
 # https://github.com/arduino/Arduino/wiki/Arduino-IDE-1.5---3rd-party-Hardware-specification
 
 name=Raspberry Pi RP2040 Boards
-version=1.13.0
+version=1.13.1
 
 runtime.tools.pqt-gcc.path={runtime.platform.path}/system/arm-none-eabi
 runtime.tools.pqt-python3.path={runtime.platform.path}/system/python3

tools/platformio-build.py

@@ -1,4 +1,5 @@
 # Copyright 2021-present Maximilian Gerhardt <maximilian.gerhardt@rub.de>
+# TinyUSB ignore snippet from https://github.com/episource/
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -168,6 +169,18 @@ def configure_usb_flags(cpp_defines):
 #
 cpp_defines = env.Flatten(env.get("CPPDEFINES", []))
 
+# Ignore TinyUSB automatically if not active without requiring ldf_mode = chain+
+if not "USE_TINYUSB" in cpp_defines:
+    env_section = "env:" + env["PIOENV"]
+    ignored_libs = platform.config.get(
+            env_section, "lib_ignore", []
+        )
+    if not "Adafruit TinyUSB Library" in ignored_libs:
+        ignored_libs.append("Adafruit TinyUSB Library")
+    platform.config.set(
+            env_section, "lib_ignore", ignored_libs
+        )
+# configure USB stuff
 configure_usb_flags(cpp_defines)
 
 # info about the filesystem is already parsed by the platform's main.py 

tools/uf2conv.py

@@ -380,7 +380,7 @@ def main():
                         proc_out = subprocess.run(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
                         if proc_out.returncode == 0:
                             stdoutput = proc_out.stdout.decode("UTF-8")
-                            match = re.search(r'Mounted\s+.*\s+at\s+([^\.]*)', stdoutput)
+                            match = re.search(r'Mounted\s+.*\s+at\s+([^\.\r\n]*)', stdoutput)
                             if match is None:
                                 print("Warn: {} did not print mount point. Attempting to locate mounted drive in file system. StdOut={}".format(cmd[0], stdoutput))
                                 drives = get_drives()