chipKIT32 / chipKIT-core

Downloadable chipKIT core for use with Arduino 1.6 - 1.8+ IDE, PlatformIO, and UECIDE
http://chipkit.net/
Apache License 2.0
59 stars 53 forks source link

UART hardware TX FIFO too small #326

Open VigibotDev opened 7 years ago

VigibotDev commented 7 years ago

I need to TX a large burst of (lidar) data inside a non blocking project.

The Arduino core code contains a nice little round robin data buffer where you can keep throwing data at it and the arduino code will read the data and process it in order.

HardwareSerial.cpp : #define SERIAL_BUFFER_SIZE 256 And it work.

On PIC32 core I discovered this inside HardwareSerial.cpp :

size_t HardwareSerial::write(uint8_t theChar)
{

    while ((uart->uxSta.reg & (1 << _UARTSTA_UTXBF)) != 0)  //check the UTXBF bit
  {
        //* wait for the transmitter buffer to have room
    }

    uart->uxTx.reg = theChar;
    return 1;
}

A bad blocking while loop freezing my code :(

There is a way to do like Arduino on ChipKit core ?

I found an implementation here : http://phoenix-mindgame.blogspot.fr/2014/09/pic32-uart-transmission-using-tx.html

But I need help to put inside the core code.

Inside pUART.c void PutStrUART(char *pcString)

majenkotech commented 7 years ago

Not without a complete re-write of the UART code, no. Unlike the Arduino, the PIC32 has a hardware FIFO for the TX buffer which we use. Some chips have a 4-byte FIFO, some have an 8-byte FIFO. If you need more I suggest you use the TX interrupt to feed more data to the FIFO from your own buffer. On suitable chips you could even configure the DMA to do the transfer for you so it doesn't use any CPU time at all.

VigibotDev commented 7 years ago

I think it's simpler for me to return to Arduino... 8 byte deep FIFO is like nothing and rewrite entire low level code for UART is better inside a MPLAB project

majenkotech commented 7 years ago

It's not that hard to hook into the interrupt. The interrupt isn't actively being used, so you just define yourself an interrupt handler and attach it to the right interrupt and vector. Then you can feed data in to your heart's content in the background.

You can read all about interrupts in chipKIT here: http://chipkit.net/interrupts-made-easy-with-chipkit/

You'd need to check the board definitions for your board to find which physical UART is used for the pins you are connected to (it's in Board_Defs.h for the variant), but it should be very simple to do.

I'll see if I can come up with some example code for you.

majenkotech commented 7 years ago

Hang on, actually both TX and RX use the same vector. That'll get tricky, since the RX ISR is already running on that vector... :(

VigibotDev commented 7 years ago

while ((uart->uxSta.reg & (1 << _UARTSTA_UTXBF)) != 0) is an insult to PIC32 power because anywhere I need to write more than 8 byte there is a delay like years inside my code:(

I know the ChipKit interrupt very easy to use. I just need a software trick to never get while() on UART TX anymore !

majenkotech commented 7 years ago

Here is a transmit-only implementation - writing your own ISR clobbers reception, but you can add that in your own ISR if you need it. I am, for simplicity, using my CircularBuffer.h library for this, but that's up to you. https://github.com/MajenkoLibraries/CircularBuffer

#include <CircularBuffer.h>

CircularBuffer<uint8_t> txBuffer(1024);

volatile bool txRunning = false;

void __USER_ISR txIsr() {
    if (getIntFlag(_UART1_TX_IRQ)) {
        if (txBuffer.available()) {
            U1TXREG = txBuffer.read();
            txRunning = true;
        } else {
            txRunning = false;
        }
        clearIntFlag(_UART1_TX_IRQ);
    }
    if (getIntFlag(_UART1_RX_IRQ)) {
        clearIntFlag(_UART1_RX_IRQ);
    }
}

void setup() {
    Serial.begin(115200);
    Serial.println("Haha");
    setIntVector(_UART_1_VECTOR, txIsr);
    setIntPriority(_UART1_TX_IRQ, 4, 0);
    setIntPriority(_UART1_RX_IRQ, 4, 0);
    setIntEnable(_UART1_TX_IRQ);
    setIntEnable(_UART1_RX_IRQ);
}

void loop() {
    delay(1000);
    uint32_t start = micros();
    print("Buffered writing done through your own ISR. There is no reception here though, but you can add that yourself to the ISR.\r\n");
    uint32_t end = micros();
    uint32_t len = end - start;
    char temp[30];
    sprintf(temp, "Time taken: %dus\r\n", len);
    print(temp);
}

void print(const char *txt) {
    for (int i = 0; i < strlen(txt); i++) {
        txBuffer.write(txt[i]);
        // Needed to kickstart the transmission
        if (txRunning == false) {
            U1TXREG = txBuffer.read();
            txRunning = true;
        }
    }
}

This is set up for an Uno32 using the FT232 UART interface (Serial) - change to the right UART registers etc for your chosen interface.

VigibotDev commented 7 years ago

Look promising, Can I use it for all 4 UART same time and what about RX ?

majenkotech commented 7 years ago

Yes, you can use all 4 UARTs at once - you need a separate ISR for each, of course.

When a character is received it will trigger the same ISR, and you can receive the character from the buffer. In the ISR at the moment is:

    if (getIntFlag(_UART1_RX_IRQ)) {
        clearIntFlag(_UART1_RX_IRQ);
    }

Just add your reception code in there to read from U1RXREG (or whatever UART you are using).

I am wondering if maybe I should create my own UART library that is separate from the internal UART system. That way it's easy to add all sorts of funky features to it without disrupting the core...

VigibotDev commented 7 years ago

Thanks a lot now I work to integrate this inside my code...

Why you can't upgrade the core to do buffered TX ? This seems essential because AVR Arduino do it !

VigibotDev commented 7 years ago

I don't need large buffer for RX, can I use this for TX only and use Serial.available/read() from framework ?

majenkotech commented 7 years ago

It could be done, and it may be done. No one else has needed it before now though. There's many other things I'd like to be able to do, though, such as DMA transfers on chips that support it, which I don't think would really sit well in the core (non-standard functionality). Also hardware flow control would be really nice.

majenkotech commented 7 years ago

No, Serial.read() will always return -1 since it isn't getting the data fed to it any more.

majenkotech commented 7 years ago

I have made a new library, "Pipe", which works like CircularBuffer.h but isn't templated. It inherits from Stream, so all the .print() functions etc work on it. It also has blocking writes when the buffer is full if you want them (second parameter true to the constructor):

An updated test sketch to go with it that does reception as well:

#include <Pipe.h>

Pipe txBuffer(1024, true);
Pipe rxBuffer(64);

volatile bool txRunning = false;

void __USER_ISR txIsr() {
    if (getIntFlag(_UART1_TX_IRQ)) {
        if (txBuffer.available()) {
            U1TXREG = txBuffer.read();
            txRunning = true;
        } else {
            txRunning = false;
        }
        clearIntFlag(_UART1_TX_IRQ);
    }
    if (getIntFlag(_UART1_RX_IRQ)) {
        rxBuffer.write(U1RXREG);
        clearIntFlag(_UART1_RX_IRQ);
    }
}

void setup() {
    Serial.begin(115200);
    setIntVector(_UART_1_VECTOR, txIsr);
    setIntPriority(_UART1_TX_IRQ, 4, 0);
    setIntPriority(_UART1_RX_IRQ, 4, 0);
    setIntEnable(_UART1_TX_IRQ);
    setIntEnable(_UART1_RX_IRQ);
}

void loop() {
    delay(1000);
    uint32_t start = micros();
    txBuffer.println("Buffered writing done through your own ISR. There is no reception here though, but you can add that yourself to the ISR.");
    if (txRunning == false) {
        U1TXREG = txBuffer.read();
        txRunning = true;
    }
    uint32_t end = micros();
    uint32_t len = end - start;
    txBuffer.print("Time taken: ");
    txBuffer.print(len);
    txBuffer.println("us");

    while (rxBuffer.available()) {
        txBuffer.write(rxBuffer.read());
    }
}

It's not perfect, since it can lock up with blocking writes if you write too much before kicking the transmission off. I need to think about the best way to hook into the write() function so it can automatically kick the transmission for you...

majenkotech commented 7 years ago

Ok... new update to Pipe that has an onWrite() callback function. It also has an updated version of the above code which uses it as an example sketch.

VigibotDev commented 7 years ago

Very good lib

majenkotech commented 7 years ago

The example with the Pipe library doesn't suffer from that problem. The issue was if you tried writing more data to the Pipe than would fit in it before the Pipe had started to be emptied by the interrupt.

On Sat, 28 Jan 2017, 17:48 Pascal, notifications@github.com wrote:

Sorry I don't understand the "it can lock up with blocking writes if you write too much before kicking the transmission off" (I'm french with bad english) I test it

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/chipKIT32/chipKIT-core/issues/326#issuecomment-275862815, or mute the thread https://github.com/notifications/unsubscribe-auth/ADouHOZdyZF4vkYW0SuyqBhd1Gpwlbdkks5rW39sgaJpZM4LwgiM .

-- Matt Jenkins Majenko Technologies http://stores.ebay.co.uk/Majenko-Technologies

VigibotDev commented 7 years ago

Yes I edited because I understood when I read the code:) My first implementation of your lib inside my code don't work but I think I made a mistake with UART2 (Serial1) register naming

VigibotDev commented 7 years ago

_UART1 / U1?XREG is for Serial _UART2 / U2?XREG is for Serial1

VigibotDev commented 7 years ago

_UART_1B_VECTOR freeze my system !

majenkotech commented 7 years ago

Which board are you using?

VigibotDev commented 7 years ago

UBW32 with pic32mx795f512l and Max32 UART bootloader

majenkotech commented 7 years ago

Hmm, I don't have one of those to test with...

VigibotDev commented 7 years ago

Like a Max32... 4 UART U1A U1B U3A U3B

majenkotech commented 7 years ago

I see the board definition only has 2 uarts defined - U1 and U2. Unless you are using the max32 definition with it?

VigibotDev commented 7 years ago

I use Max32 definition with it !

VigibotDev commented 7 years ago

This is why I use Max32 for all. This is the only bootloader with UART programming (for wireless programming) and 4 UART defined

majenkotech commented 7 years ago

U1, U4, U2, U5, are serial, serial1, serial2 and serial3.

VigibotDev commented 7 years ago

lol !!! I test. I think you solved the problem

VigibotDev commented 7 years ago

UART4/U4 freeze the system also:(

majenkotech commented 7 years ago

You are starting the serial port with Serial1.Begin, yes?

VigibotDev commented 7 years ago

Yes (cut past for my test code)

#include <SimpleTimer.h>
#include <Pipe.h>

#define DEBUG Serial
#define MODEM Serial1
#define SERVOS Serial2
#define LIDAR Serial3

void __USER_ISR txIsr() {
    if (getIntFlag(_UART4_TX_IRQ)) {
        if (txBuffer.available()) {
            U4TXREG = txBuffer.read();
            txRunning = true;
        } else {
            txRunning = false;
        }
        clearIntFlag(_UART4_TX_IRQ);
    }
    if (getIntFlag(_UART4_RX_IRQ)) {
        rxBuffer.write(U4RXREG);
        clearIntFlag(_UART4_RX_IRQ);
    }
}

void kick() {
    if (txRunning == false) {
        U4TXREG = txBuffer.read();
        txRunning = true;
    }    
}

void setup() {
 DEBUG.begin(115200);
 DEBUG.println("setup()");
 MODEM.begin(115200);
 txBuffer.onWrite(kick);
 setIntVector(_UART_4_VECTOR, txIsr);
 setIntPriority(_UART4_TX_IRQ, 4, 0);
 setIntPriority(_UART4_RX_IRQ, 4, 0);
 setIntEnable(_UART4_TX_IRQ);
 setIntEnable(_UART4_RX_IRQ);
 SERVOS.begin(1000000);
 LIDAR.begin(115200);
majenkotech commented 7 years ago

I am not at my desk at the moment so I can't test. When I get back there later on I will dig out my max32 to have a play.

VigibotDev commented 7 years ago

/ Serial port 1 uses UART4 (aka UART1B) /

define _SER1_BASE _UART4_BASE_ADDRESS

define _SER1_IRQ _UART4_ERR_IRQ

define _SER1_VECTOR _UART_4_VECTOR

define _SER1_IPL_ISR IPL2SOFT

define _SER1_IPL 2

define _SER1_SPL 0

I don't understand why this freeze entire system

majenkotech commented 7 years ago

Well, this is odd. UART4's TX interrupt looks like it's permanently set and will never clear...

majenkotech commented 7 years ago

Oh yeah, I know what this is... The interrupt keeps triggering while the TX FIFO is empty - something that is different between the 4-byte and 8-byte deep FIFOs, which is why it worked on the Uno32 but not the Max32.

So you just need to start with the TX IRQ disabled, and turn it on in the txBuffer's "onWrite()" callback - and then turn it off in the ISR if there is nothing left to transmit.

Just working on some sample code...

majenkotech commented 7 years ago
#include <Pipe.h>

Pipe txBuffer(1024, true);
Pipe rxBuffer(64);

volatile bool txRunning = false;

void __USER_ISR txIsr() {
    digitalWrite(13, HIGH);
    if (getIntFlag(_UART4_TX_IRQ)) {
        if (txBuffer.available()) {
            U4TXREG = txBuffer.read();    
            txRunning = true;
        } else {
            clearIntEnable(_UART4_TX_IRQ);
            txRunning = false;
        }
        clearIntFlag(_UART4_TX_IRQ);
    }

    if (getIntFlag(_UART4_RX_IRQ)) {
        rxBuffer.write(U4RXREG);
        clearIntFlag(_UART4_RX_IRQ);
    }

    if (getIntFlag(_UART4_ERR_IRQ)) {
        clearIntFlag(_UART4_ERR_IRQ);
    }

    digitalWrite(13, LOW);
}

void kick() {
    if (!txRunning) {
        txRunning = true;
        U4TXREG = txBuffer.read();    
        setIntEnable(_UART4_TX_IRQ);
    }    
}

void setup() {
    Serial.begin(115200);
    Serial1.begin(115200);
    Serial2.begin(115200);
    Serial3.begin(115200);
    pinMode(13, OUTPUT); digitalWrite(13, LOW);
    txBuffer.onWrite(kick);
    setIntVector(_UART_4_VECTOR, txIsr);
    setIntPriority(_UART_4_VECTOR, 2, 0);
    clearIntFlag(_UART4_TX_IRQ);
    clearIntFlag(_UART4_RX_IRQ);
    clearIntFlag(_UART4_ERR_IRQ);    
    setIntEnable(_UART4_RX_IRQ);
    clearIntEnable(_UART4_TX_IRQ);
}

void loop() {
    static uint32_t ts = millis();
    if (millis() - ts >= 1000) {
        ts = millis();
        Serial.println(ts);
        txBuffer.println(ts);
    }
    while (Serial.available()) {
        txBuffer.write(Serial.read());
    }
    while (rxBuffer.available()) {
        Serial.write(rxBuffer.read());
    }
}
VigibotDev commented 7 years ago

Thanks a lot for your work, I test this now

VigibotDev commented 7 years ago

There is no more freeze but I get a lot of incomprehensible data corruption

majenkotech commented 7 years ago

Do you have blocking writes turned on for the TX pipe?

On Sun, 29 Jan 2017, 09:02 Pascal, notifications@github.com wrote:

There is no more freeze but I get a lot of incomprehensible data corruption

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/chipKIT32/chipKIT-core/issues/326#issuecomment-275902006, or mute the thread https://github.com/notifications/unsubscribe-auth/ADouHGJL7RY7BOmcnZpOeStPlJi93Bccks5rXFWbgaJpZM4LwgiM .

-- Matt Jenkins Majenko Technologies http://stores.ebay.co.uk/Majenko-Technologies

VigibotDev commented 7 years ago

Pipe txBuffer(256, true); Pipe rxBuffer(64);

VigibotDev commented 7 years ago

There is a "protocol analyser" on my project, this look a timing problem. The RX side of my modem need a perfect timing, no "hole" in data flow else it transmit on radio side.

VigibotDev commented 7 years ago

Also I just found the first byte at the end of all radio frames.

VigibotDev commented 7 years ago

I transmit @ 20Hz and each (digital) frame begin with ascii "$R" (sync bytes). I found the "$" at the end of each frame. And a 1 byte left or right rotation for all recovered data.

VigibotDev commented 7 years ago

I receive @ 10Hz in place of 20Hz - because the decoder can sync one frame for two transmited.

VigibotDev commented 7 years ago

The RX side of your sample work perfectly, the TX side have a 1 byte bug

majenkotech commented 7 years ago

How big are the packets you are sending?

On Sun, 29 Jan 2017, 09:41 Pascal, notifications@github.com wrote:

The RX side of your sample work perfectly, the TX side have a 1 byte bug

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/chipKIT32/chipKIT-core/issues/326#issuecomment-275903529, or mute the thread https://github.com/notifications/unsubscribe-auth/ADouHJ2zKZjBAWwdz2aWY03DbxuAUSSqks5rXF7NgaJpZM4LwgiM .

-- Matt Jenkins Majenko Technologies http://stores.ebay.co.uk/Majenko-Technologies

VigibotDev commented 7 years ago

Tested with fixed 53 bytes.

VigibotDev commented 7 years ago

The problem can be mine, I must restart all to test

VigibotDev commented 7 years ago

No, this is not a problem here because it work when I return to the framework Serial1.write.

VigibotDev commented 7 years ago

I get exactly the same problem with a smaller frame size: I get the "$" at end, and 1/2 frame out of sync.

ModemRX | $RÂ..WN..H....àNÝH$ | 24 52 C2 00 00 57 4E 0A 09 48 80 00 80 00 E0 4E DD 48 24 | 036 082 194 000 000 087 078 010 009 072 128 000 128 000 224 078 221 072 036
ModemTX | $S.ÿ.ÿ....ì.. | 24 53 01 FF 01 FF 03 80 80 80 EC 05 0A | 036 083 001 255 001 255 003 128 128 128 236 005 010
ModemTX | $S.ÿ.ÿ....ì.. | 24 53 01 FF 01 FF 03 80 80 80 EC 05 0A | 036 083 001 255 001 255 003 128 128 128 236 005 010
ModemRX | $R...X+..H....àOÞ^$ | 24 52 BF 00 00 58 2B 0A 04 48 80 00 80 00 E0 4F DE 5E 24 | 036 082 191 000 000 088 043 010 004 072 128 000 128 000 224 079 222 094 036
ModemTX | $S.ÿ.ÿ....ì.. | 24 53 01 FF 01 FF 03 80 80 80 EC 05 0A | 036 083 001 255 001 255 003 128 128 128 236 005 010
ModemTX | $S.ÿ.ÿ....ì.. | 24 53 01 FF 01 FF 03 80 80 80 EC 05 0A | 036 083 001 255 001 255 003 128 128 128 236 005 010
ModemRX | $R...Wø..H....àOÞF$ | 24 52 BF 00 00 57 F8 0A 02 48 80 00 80 00 E0 4F DE 46 24 | 036 082 191 000 000 087 248 010 002 072 128 000 128 000 224 079 222 070 036
ModemTX | $S.ÿ.ÿ....ì.. | 24 53 01 FF 01 FF 03 80 80 80 EC 05 0A | 036 083 001 255 001 255 003 128 128 128 236 005 010
ModemTX | $S.ÿ.ÿ....ì.. | 24 53 01 FF 01 FF 03 80 80 80 EC 05 0A | 036 083 001 255 001 255 003 128 128 128 236 005 010
ModemRX | $R...W...H....àLÞZ$ | 24 52 BF 00 00 57 B9 0A 03 48 80 00 80 00 E0 4C DE 5A 24 | 036 082 191 000 000 087 185 010 003 072 128 000 128 000 224 076 222 090 036
ModemTX | $S.ÿ.ÿ....ì.. | 24 53 01 FF 01 FF 03 80 80 80 EC 05 0A | 036 083 001 255 001 255 003 128 128 128 236 005 010
ModemTX | $S.ÿ.ÿ....ì.. | 24 53 01 FF 01 FF 03 80 80 80 EC 05 0A | 036 083 001 255 001 255 003 128 128 128 236 005 010
ModemRX | $RÂ..W...H....àRÝN$ | 24 52 C2 00 00 57 88 0A 05 48 80 00 80 00 E0 52 DD 4E 24 | 036 082 194 000 000 087 136 010 005 072 128 000 128 000 224 082 221 078 036
ModemTX | $S.ÿ.ÿ....ì.. | 24 53 01 FF 01 FF 03 80 80 80 EC 05 0A | 036 083 001 255 001 255 003 128 128 128 236 005 010
ModemTX | $S.ÿ.ÿ....ì.. | 24 53 01 FF 01 FF 03 80 80 80 EC 05 0A | 036 083 001 255 001 255 003 128 128 128 236 005 010
ModemRX | $RÂ..WW..H....àPÝK$ | 24 52 C2 00 00 57 57 0A 07 48 80 00 80 00 E0 50 DD 4B 24 | 036 082 194 000 000 087 087 010 007 072 128 000 128 000 224 080 221 075 036
ModemTX | $S.ÿ.ÿ....ì.. | 24 53 01 FF 01 FF 03 80 80 80 EC 05 0A | 036 083 001 255 001 255 003 128 128 128 236 005 010
ModemTX | $S.ÿ.ÿ....ì.. | 24 53 01 FF 01 FF 03 80 80 80 EC 05 0A | 036 083 001 255 001 255 003 128 128 128 236 005 010
ModemRX | $RÂ..W0..H....àJÞH$ | 24 52 C2 00 00 57 30 0A 09 48 80 00 80 00 E0 4A DE 48 24 | 036 082 194 000 000 087 048 010 009 072 128 000 128 000 224 074 222 072 036
ModemTX | $S.ÿ.ÿ....ì.. | 24 53 01 FF 01 FF 03 80 80 80 EC 05 0A | 036 083 001 255 001 255 003 128 128 128 236 005 010
ModemTX | $S.ÿ.ÿ....ì.. | 24 53 01 FF 01 FF 03 80 80 80 EC 05 0A | 036 083 001 255 001 255 003 128 128 128 236 005 010
ModemRX | $R...Wö..H....àTÞM$ | 24 52 BF 00 00 57 F6 0A 02 48 80 00 80 00 E0 54 DE 4D 24 | 036 082 191 000 000 087 246 010 002 072 128 000 128 000 224 084 222 077 036
ModemTX | $S.ÿ.ÿ....ì.. | 24 53 01 FF 01 FF 03 80 80 80 EC 05 0A | 036 083 001 255 001 255 003 128 128 128 236 005 010
ModemTX | $S.ÿ.ÿ....ì.. | 24 53 01 FF 01 FF 03 80 80 80 EC 05 0A | 036 083 001 255 001 255 003 128 128 128 236 005 010
ModemRX | $R...WÞ..H....àMÞZ$ | 24 52 BF 00 00 57 DE 0A 03 48 80 00 80 00 E0 4D DE 5A 24 | 036 082 191 000 000 087 222 010 003 072 128 000 128 000 224 077 222 090 036