earlephilhower / arduino-pico

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

Serial1.available() reporting bytes available when none are available #1021

Closed chrisckc closed 1 year ago

chrisckc commented 1 year ago

I created quick a test harness to verify UART communications between 2 Pico's: https://github.com/chrisckc/TestHarness-UART It is based on the SPI master-slave example from the Pico examples repo. The 2 Pico's are wired together on a breadboard with grounds, VBUS and VSYS connected together. Pin 1 (GP0, UART0_TX) is connected to Pin2 (GP1, UART0_RX) on the other pico and vice versa.

The sending pico waits for one second after the receiving pico is initialised, ie. until it is ready by repeatedly checking Serial1.available(). After this one second wait time, the sending Pico sends data to the receiving pico, the first is a single byte indicating the length of the next set of data which is a 255 byte buffer of incrementing test data bytes starting 0x01, then 0x02, up to 0xFF.

The receiving Pico then sends back an inverted version of the buffer back to the sending Pico.

What I am seeing on both ends is that Serial1.available() is reporting that a byte is available before anything has actually arrived, this happens once at the start and results in errors reported because the subsequent calls to Serial1.read() and Serial1.readBytes(buf, length) return zero's.

When the Pico actually receives UART data it works correctly, it reports 1 byte (the first transmission informing the length of the next set of data) and is also able to correctly read the next 255 bytes using Serial1.readBytes(buf, length).

The scope traces below demonstrate the issue: The yellow trace is DEBUG_PIN2 in the test code for the receiving Pico, green is DEBUG_PIN3 on the receiving Pico. Blue is the RX line on the receiving pico, purple is the TX line on the receiving Pico. ScreenImg-50

The yellow low pulse marks Serial1.available() reporting a value > 0, but as can be seen there was data on the RX line. The receiver then sends data back to the sender as seen by the purple trace.

Zooming out further shows from the moment of powering on the 2 Pico's, there are no extraneous pulses on the RX line. ScreenImg-51

In fact you don't even need to connect up the scope to verify this, if I disconnect the TX and RX pins by pulling out the wires, I still see the issue.

Here is the test setup, the same as used for the previous issue I logged for i2c communications here and SPI over on the Pico-SDK repo. The longer white and green wires are connecting together the TX and RX pins of UART0, other wires were used for testing the other stuff. 20221202_133551

chrisckc commented 1 year ago

As a side note, I originally wanted to use Interrupts for this, I assumed that serialEvent() would do that: https://www.arduino.cc/reference/en/language/functions/communication/serial/serialevent/

The Pico SDK has functions for interrupt driven UART handling: https://github.com/raspberrypi/pico-examples/blob/master/uart/uart_advanced/uart_advanced.c however serialEvent() is just a wrapper around Serialx.available() that is called every time after loop() has completed and before it runs again.

earlephilhower commented 1 year ago

Can you repro this with a single board in loopback (tie Serial2.TX to Serial1.RX)? I'm wondering if a rogue START bit is being detected, because I don't remember hearing about anything like that with other UART users. (i.e. #464, etc)

As for SerialEvent, that's just the Arduino API way. We're using the FIFO-partially full IRQ, so the RP2040 isn't getting an IRQ per char anyway.

chrisckc commented 1 year ago

Haven't tried that, but it also happens with nothing connected to the serial1 / uart0 pins.

On Fri, 2 Dec 2022, 16:40 Earle F. Philhower, III, @.***> wrote:

Can you repro this with a single board in loopback (tie Serial2.TX to Serial1.RX)? I'm wondering if a rogue START bit is being detected, because I don't remember hearing about anything like that with other UART users. (i.e. #464 https://github.com/earlephilhower/arduino-pico/issues/464, etc)

As for SerialEvent, that's just the Arduino API way. We're using the FIFO-partially full IRQ, so the RP2040 isn't getting an IRQ per char anyway.

— Reply to this email directly, view it on GitHub https://github.com/earlephilhower/arduino-pico/issues/1021#issuecomment-1335516034, or unsubscribe https://github.com/notifications/unsubscribe-auth/AASVEDOBM57ENB2442IUODLWLIQ6HANCNFSM6AAAAAASR5WNYI . You are receiving this because you authored the thread.Message ID: @.***>

earlephilhower commented 1 year ago

Thanks. I'm doing some surgery on my RAID arrays this week, but will try and look into it and see if there's anything obvious...

chrisckc commented 1 year ago

No problem, thanks! Btw. Back home now, out of curiosity I just took a brand new PicoW, plugged in the USB Cable and uploaded the receiver code and saw the same issue, brand new board floating in thin air.

I started work on a pure SDK version of this test harness last thing Friday, I will let you know if I see the same issue.

Bodmer commented 1 year ago

A period of signal hiatus would be expected after power up and when the processors are programmed sequentially because signals do not get initialised at identical times in the two processors. I suggest using a delay and serial flush after boot before trying to communicate.

Incidentally, it is not advisable to link VBUS between the two processors since a power voltage conflict can occur if both are connected to USB sources at the same time. It is sufficient to link VSYS in this situation as that will be a diode OR of the two VBUS signals and thus the two USB sources cannot get into a voltage conflict or backdrive state.

chrisckc commented 1 year ago

Hi @Bodmer , thanks for your insight on this, as this issue is predicable and repeatable I can always work around it by adding this at the end of setup() while(UART_INSTANCE.available()) UART_INSTANCE.read(); which should do what you have suggested.

I thought it was still worth logging the issue though because there is no mention of the need to do this in any documentation that I have seen, including the Pico-SDK.

earlephilhower commented 1 year ago

@chrisckc I think you're reading noise on the Serial lines and there's not really anything to be done in the core here.

What I thought was happening was that on any Serial1.begin() you either a) got Serial.available() == 1 but there was no available data so Serial.read() failed, or b) Serial1.available() == 1 and Serial.read() returned 1 byte of junk.

In my testing to avoid any signal issues, I just left the RX pin floating and I'm not getting any spurious data available on either Serial1 or Serial2. IOW, I think you are catching a glitch from the other side when the other serial port side is initting it's TX lines.

The UART itself is completely reset in the SDK uart_init so there's no chance of old data still being in the FIFOs when it begins.

Not really sure what, if anything, can be done in the core.

chrisckc commented 1 year ago

No problem, thanks! Btw. Back home now, out of curiosity I just took a brand new PicoW, plugged in the USB Cable and uploaded the receiver code and saw the same issue, brand new board floating in thin air.

I started work on a pure SDK version of this test harness last thing Friday, I will let you know if I see the same issue.

I have already tried leaving the RX pin floating, this was just a new Pico taken out of the drawer, plugged in, flashed and seen the issue.

After Serial1.begin() is called, Serial1.available() returns 1 and then Serial1.read() returns zero, with nothing connected to the RX pin, or any other pin.

This is the startup code I'm using, with UART_INSTANCE defined as Serial1

#define UART_INSTANCE Serial1 // Serial1 is UART 0 on the Pico, Serial2 is UART1
#define UART_FIFO_SIZE (256)
#define UART_BAUDRATE (921600)
...............

    // Configure the UART
    UART_INSTANCE.setRX(UART_RX);
    UART_INSTANCE.setTX(UART_TX);

    UART_INSTANCE.setFIFOSize(UART_FIFO_SIZE);
    UART_INSTANCE.begin(UART_BAUDRATE);
earlephilhower commented 1 year ago

I think I can repro this now. I tied RX (GP2)->GND (avoid any floating input noise) and dumped a continuous stream of Serial.printf("%d", Serial1.available()); and I see that initially it reports 0 available, but after several usec it does bump up to one.

I wonder if the SP hardware is reading the pulled-high input as a 1 start bit immediately after reset. Maybe I need to drive RX to 0 before resetting...

-doh-

Correcting the pin to GP1 and the value to 3.3V (i.e. UART is return-to-1) shows the same behavior.

Bodmer commented 1 year ago

Sounds like an interrupt flag is not cleared on init.

earlephilhower commented 1 year ago

Thanks, @Bodmer, but the IRQ flags are good. The UART read routine always uses uart_is_readable() (checks the RX FIFO is not empty). So even an extra IRQ would just cause a no-op.

What seems to be happening is that as soon as the UART is put out of reset and configures it always reads in a single byte. I've cleared the RXFIFO after startup, and configured the IO pins before pulling the UART out of reset, same behavior. I think it's a HW thing at this point.

To verify I did this simple test:

void setup() {
  Serial.begin();
  delay(5000);
  Serial.printf("go %d\n", micros());
  Serial1.begin(110);
}

void loop() {
  Serial.printf("%d %d\n", Serial1.available(), micros());
}

The available went from 0-> at 90ms (i.e. 1/110 * 10 (8data+1start+1end bit)). As I adjust the baud, the timing adjusts to match 10 bit times between the "go" message and the 1st byte available. So, the HW immediately starts receiving a byte no matter what AFAICT.

The UART config code is trivial and pulled from the examples, basically, so I'm pretty sure I've got that right. Only thing different is the FIFO enable but that doesn't affect the actual receive, it only affects the transfer to the core.

Dumb workaround is to throw away the 1st byte in the IRQ handler. It's simple to do but unsatisfying.

chrisckc commented 1 year ago

UPDATE: I can confirm that this also happens using pure SDK. I have tried both with the FIFO disabled and enabled and still see the first IRQ firing with uart_is_readable(uart0) returning true and then a zero byte coming back from uart_getc(uart0).

I am also seeing other issues with my "pure SDK" UART testing which I will be logging soon over on the Pico-SDK repo.

earlephilhower commented 1 year ago

The SDK doesn't have any checking of the status bits. So the UART is returning {FRAMERR, 0x00} and they're just ignoring the error bits. The core now should be good (checks and discard errored bytes), so please let us know if it's not.

chrisckc commented 1 year ago

Thanks, thats great! I will check it out. I noticed that if I tie the RX line to ground via a 10k resistor the issue still occurs, However if I tie the RX line to 3.3v via the 10k resistor the issue goes away and I don't see the phantom IRQ.

I logged an issue over on the Pico-SDK repo along with some scope traces. https://github.com/raspberrypi/pico-sdk/issues/1144 The issue is 2 parts, this issue and another issue where I was unable to achieve reliable data transfer above 115200 baud without the FIFO enabled.

chrisckc commented 1 year ago

I can confirm that your commit "Throw away UART bytes with errors in reception" Has fixed the issue reported here, Thanks for your effort on this one!