espressif / arduino-esp32

Arduino core for the ESP32
GNU Lesser General Public License v2.1
13.45k stars 7.38k forks source link

Hardware serial sometimes doesn't send every data. #6642

Closed zekageri closed 2 years ago

zekageri commented 2 years ago

Board

esp32-wrover-e

Device Description

Custom board, using ESP32-Wrover-E ( 16mb flash 8mb external ram )

Hardware Configuration

Ethernet -> ETH_LAN_8720 chip

RTC Module -> DS3231 chip ( i2c )

Transistor for restarting the device -> 2n2222 or similar. ( GPIO 12 )

Two hardware serial UART:

Version

latest master (checkout manually)

IDE Name

Platform IO

Operating System

Windows 10

Flash frequency

80Mhz

PSRAM enabled

yes

Upload speed

115200

Description

Serial1 sometimes does not send out all the bytes and i got a frame timeout in my communication because the modules that my esp communicates doesnt understand the broken message. Happens mostly on web page load or file upload via HTTP.

Communication runs on an available core in a separate task with 115200 baud rate, tries to write as soon as possible.

Sketch

#define MBUS_BAUD 115200
#define MBUS_RX 34  //   6.  PIN
#define MBUS_TX 15  //   23. PIN

void mBusSetup(){
    pinSetup();
    Serial1.begin(MBUS_BAUD,SERIAL_8N1,MBUS_RX,MBUS_TX);
}

void mSystem::pinSetup(){
    pinMode(MBUS_TX_EN, OUTPUT);
    digitalWrite(MBUS_TX_EN, LOW);
}

void mSystem::startWrite(){
    mBusLock();
    //Serial1.flush();
    digitalWrite(MBUS_TX_EN, HIGH);
}

void mSystem::endWrite(){
    Serial1.flush();          // does not help
    //delayMicroseconds(200); // Does not help. Dont want magic delays
    digitalWrite(MBUS_TX_EN, LOW); // TX_ENABLE pin pulled low too soon i assume.
    mBusUnLock();
}

void mSystem::rawWrite(uint8_t * data, int length){
    int crc16 = getCRC_16(data, length);
    startWrite();
    Serial1.write( data, length );
    Serial1.write( lowByte(crc16) );
    Serial1.write( highByte(crc16) );
    endWrite();
}

#define MBUS_DISCOVER_LISTEN_TIMEOUT 150 // Modules will respond back in maximum of 15ms.

void writeReadDevice(int index){
    // This function handles which device address to write and assmebles the writable data and calls
    // rawWrite. If rawWrite returns, this function waits for the response data.
    uint8_t exampleData[] = {0xFF, 0x03, 0x00, 0x06, 0x00, 0x03};
    uint8_t responseData[20]; // Maximum response data is 10-13.
    rawWrite(exampleData, sizeof(exampleData));

    // wait for the response data. The modules will respond back if the data is understandable.
    long startMs = millis();
    while( !Serial1.available() ){
        vTaskDelay(1);
        if( millis() - startMs >= MBUS_DISCOVER_LISTEN_TIMEOUT ){
            Serial.println("FRAME TIMEOUT!");
            return;
        }
    }

    int respDataCount = 0;
    while( Serial1.available() > 0 ){
        responseData[respDataCount] = Serial1.read();
        respDataCount++;
    }
}

void commLoop(){
    writeReadDevice(deviceIndex);
    deviceIndex++; // Used for communicating with different modules
    if( deviceIndex > deviceCounter ){deviceIndex = 0;}
}

void mBusLoopTask(void* parameter){
    mBusSetup();
    for(;;){
        commLoop();
        vTaskDelay(3); // The problem occours even faster if this delay is smaller.
    }
}

Debug Message

No debug message just broken data on Serial.

Other Steps to Reproduce

Try to write frequently on serial1.

I have checked existing issues, online documentation and the Troubleshooting Guide

SuGlider commented 2 years ago

@zekageri

This will work:

// Necessary include for testing the fix
#include "driver/uart.h"

void setup() {
  // for example, start Serial2 - UART2
  Serial2.begin(115200);

  // right after starting UART2, add this code:
  uart_intr_config_t uart_intr = {
      .intr_enable_mask = (0x1<<0) | (0x8<<0),  // UART_INTR_RXFIFO_FULL | UART_INTR_RXFIFO_TOUT,
      .rx_timeout_thresh = 1,
      .txfifo_empty_intr_thresh = 10,
      .rxfifo_full_thresh = 112,
  };
  uart_intr_config((uart_port_t) 2, &uart_intr);  // Two is the UART number for Arduino Serial
}

Thanks!

SuGlider commented 2 years ago

You code shall be like this:

#include "driver/uart.h"

void mSystem::testSerialSetup(){
    uart_intr_config_t uart_intr = {
        .intr_enable_mask = (0x1<<0) | (0x8<<0),  // UART_INTR_RXFIFO_FULL | UART_INTR_RXFIFO_TOUT,
        .rx_timeout_thresh = 1,
        .txfifo_empty_intr_thresh = 10,
        .rxfifo_full_thresh = 112,
    };
    uart_intr_config((uart_port_t) 1, &uart_intr);
}

void mSystem::setup(){
    Serial1.begin(MBUS_BAUD,SERIAL_8N1,MBUS_RX,MBUS_TX);
    testSerialSetup();
}
zekageri commented 2 years ago

Thank you for the suggestions. I will try that tomorrow.

zekageri commented 2 years ago

As soon as i run this, i get FRAME_TIMEOUT and no communication after that at all. I will debug it now.

zekageri commented 2 years ago

Here is how my code relevant part looks like.

#include "driver/uart.h"

boolean mSystem::newWriteToBuffer(int index, uint16_t registerAddr){
    int byteCount       = modules[index]->writeNumBytes*2;
    int writeBufferSize = byteCount + 7;
    int respDataCount   = 0;
    boolean isSuccess   = false;

    uint8_t writeBuffer[writeBufferSize];
    uint8_t respBuffer[150];
    writeBuffer[0] = uint8_t(modules[index]->address);
    writeBuffer[1] = 0x11;
    writeBuffer[2] = highByte(registerAddr);
    writeBuffer[3] = lowByte(registerAddr);
    writeBuffer[4] = highByte(modules[index]->writeNumBytes);
    writeBuffer[5] = lowByte(modules[index]->writeNumBytes);
    writeBuffer[6] = uint8_t(byteCount);

    // put the buffer data into writeBuffer
    for(int i = 0; i < modules[index]->writeNumBytes; i++){
        writeBuffer[i*2+7] = highByte(modules[index]->writeBuffer[i]);
        writeBuffer[i*2+8] = lowByte(modules[index]->writeBuffer[i]);
    }
    rawWrite(writeBuffer,writeBufferSize);
    long startMS = millis();
    while ( !Serial1.available() ){
        //vTaskDelay(1);
        // READ_WRITE_TIMEOUT == 55ms
        if( !Serial1.available() && ( (millis() - startMS) > READ_WRITE_TIMEOUT) ){
            Serial.printf("FRAME TIMEOUT at millis: %d module id: %d\n",millis(),modules[index]->id);
            return false;
        }
    }
    while ( Serial1.available() ){
        respBuffer[respDataCount] = Serial1.read();
        respDataCount++;
    }
    //Serial1.flush();
    if( respBuffer[0] == uint8_t(modules[index]->address) ){
        uint8_t gotCRC_High = respBuffer[respDataCount-2];
        uint8_t gotCRC_Low  = respBuffer[respDataCount-1];
        uint16_t gotCRC     = (gotCRC_Low << 8) | gotCRC_High;
        uint16_t calcCRC    = getCRC_16(respBuffer,respDataCount-2);
        if( gotCRC == calcCRC ){
            if( respBuffer[1] == 0x90 || respBuffer[1] == 0x83 ){
                Serial.printf("\nNew write failed with code: %02X\n",respBuffer[2]);
                return false;
            }
            for(int i = 0; i < modules[index]->readNumBytes; i++){
                modules[index]->readBuffer[i] = (respBuffer[i*2+3] << 8) | respBuffer[i*2+4];
            }
            return true;
        }else{
            Serial.printf("new write CRC Mismatch. Got: %04X Calculated: %04X\n",gotCRC,calcCRC);
            return false;
        }
    }
    return false;
}

void mSystem::writeReadDevice(int index){
    if( modules[index] != 0 ){
        if( modules[index]->isActive ){

            if( modules[index]->onError ){
                if( millis() - modules[index]->wentToError >= 60000 ){//modules[index]->onErrorTryCounter >= 1000 ){
                    modules[index]->onError = false;
                }else{
                    return;
                }
            }

            uint16_t regWriteAddr = getRegWriteAddr(modules[index]->id);
            setThermosDefaultDisplay(index,regWriteAddr);
            if( !newWriteToBuffer(index,regWriteAddr) ){
                modules[index]->failCounter++;
                moduleCommError(index);
            }else{
                modules[index]->failCounter = 0;
                modules[index]->onError     = false;
                checkModuleFaultCode(index);
            }
        }
    }
}

void mSystem::testSerialSetup(){
    uart_intr_config_t uart_intr = {
        .intr_enable_mask = (0x1<<0) | (0x8<<0),  // UART_INTR_RXFIFO_FULL | UART_INTR_RXFIFO_TOUT,
        .rx_timeout_thresh = 1,
        .txfifo_empty_intr_thresh = 10,
        .rxfifo_full_thresh = 112,
    };
    uart_intr_config((uart_port_t) 1, &uart_intr);
}

void mSystem::setup(){
    Serial1.begin(MBUS_BAUD,SERIAL_8N1,MBUS_RX,MBUS_TX);
    testSerialSetup();  // WITHOUT THIS SERIAL SETUP, IT IS WORKING.
}

void mSystem::loop(){
  writeReadDevice(deviceIndex);
  deviceIndex++;
  if( deviceIndex > deviceCounter ){ deviceIndex = 0; }
}

void mBusLoopTask(void* parameter){
    for(;;){
        hsh_modbus.loop();
        vTaskDelay(2);
    }
}
SuGlider commented 2 years ago

Please try this change:


void mSystem::testSerialSetup(){
    uart_intr_config_t uart_intr = {
        .intr_enable_mask = (0x1<<0) | (0x8<<0),  // UART_INTR_RXFIFO_FULL | UART_INTR_RXFIFO_TOUT,
        .rx_timeout_thresh = 1,
        .txfifo_empty_intr_thresh = 10,
        .rxfifo_full_thresh = 1,
    };
    uart_intr_config((uart_port_t) 1, &uart_intr);
}

It will force the driver to read each byte, one by one very fast.

zekageri commented 2 years ago

This causes CRC mismatches in every response

new write CRC Mismatch. Got: 1311 Calculated: 807E
new write CRC Mismatch. Got: 110B Calculated: FFFF
new write CRC Mismatch. Got: 943D Calculated: 97BC
new write CRC Mismatch. Got: C03D Calculated: 4C34
new write CRC Mismatch. Got: 94F1 Calculated: 79B2
new write CRC Mismatch. Got: C03D Calculated: 6FDA
new write CRC Mismatch. Got: 943D Calculated: 965C
new write CRC Mismatch. Got: C03D Calculated: E6A2
new write CRC Mismatch. Got: 943D Calculated: C9C7
new write CRC Mismatch. Got: 11C0 Calculated: FFFF
new write CRC Mismatch. Got: 943D Calculated: EECA
new write CRC Mismatch. Got: C03D Calculated: BF9E
Making config from sun times calculated 
new write CRC Mismatch. Got: 943D Calculated: 838A
new write CRC Mismatch. Got: 11C0 Calculated: FFFF
new write CRC Mismatch. Got: 0B3D Calculated: 53E9
new write CRC Mismatch. Got: 943D Calculated: 81E6
new write CRC Mismatch. Got: C03D Calculated: EEE3
new write CRC Mismatch. Got: 943D Calculated: 5DCA
new write CRC Mismatch. Got: 1101 Calculated: FFFF
new write CRC Mismatch. Got: 8CF1 Calculated: 522D
new write CRC Mismatch. Got: C0F1 Calculated: 3C9C
new write CRC Mismatch. Got: 0002 Calculated: 7CA5
new write CRC Mismatch. Got: F110 Calculated: 3021
FS - (copy config) Firmware version: 1.34

It is possible that i have to adjust my code in order for this to work.

zekageri commented 2 years ago

With my workaround to put a bigger delay in the loop it was working. Now i have upgraded to the latest update and these timeouts are back again with my workaround. The ESP misses the packets...

PLATFORM: Espressif 32 (5.0.0) > Espressif ESP-WROVER-KIT
esp-wrover-kit (platform: espressif32; board: esp-wrover-kit; framework: arduino)

PACKAGES:
 - framework-arduinoespressif32 @ 3.20003.220626 (2.0.3)
 - tool-esptoolpy @ 1.30300.0 (3.3.0)
 - toolchain-xtensa-esp32 @ 8.4.0+2021r2-patch3
SuGlider commented 2 years ago

Which version of Arduino Core were you using before updating to 2.0.3? Was that the 2.0.0?

zekageri commented 2 years ago

I have got a platform update to 5.0.0.

I have so much Espressif 32 platform installed i don't know what i was used. Probably 4.1.0.

image image

zekageri commented 2 years ago

In my PIO ini i did not modified the framework or the platform.

[env:esp-wrover-kit]
platform = espressif32

board       = esp-wrover-kit
framework   = arduino
zekageri commented 2 years ago

Can i jump between versions with this line in the PIO ini?

platform_packages = framework-arduinoespressif32 @ https://github.com/espressif/arduino-esp32.git#2.0.3

If i modify the last version number

SuGlider commented 2 years ago

I'm not familiar with PIO... I think that maybe @Jason2866 can help on it.

zekageri commented 2 years ago

I tested with

platform_packages = framework-arduinoespressif32 @ https://github.com/espressif/arduino-esp32.git#2.0.3
platform_packages = framework-arduinoespressif32 @ https://github.com/espressif/arduino-esp32.git#2.0.1
platform_packages = framework-arduinoespressif32 @ https://github.com/espressif/arduino-esp32.git#2.0.0

same in every case

Jason2866 commented 2 years ago

Platformio actual latest release is 5.0 (which is Arduino core 2.0.3) the way you did to tag the version is working and fetches the corresponding commit. To use latest core 2.0.4 you can do platform = https://github.com/platformio/platform-espressif32.git since this branch (develop) already uses core 2.0.4

zekageri commented 2 years ago

Thank you for the suggestion.

What is the difference beetween platform_packages, platform, and framework?

Framework should be the ESP IDF, but my default PIO configuration for the framework is arduino, but i have seen arduino_espressif too since Arduino framework is top on IDF.

Jason2866 commented 2 years ago

platforms includes all needed toolchains, tools and the framework (here Arduino) since defined as wanted in platformio setup. With platform_packages you can define (override) anything what is default defined in platforms IDF and Arduino are two different frameworks. It does not count here that Arduino framwork is builded with IDF.

zekageri commented 2 years ago

I see. Thank you for the explanation. I will investigate it, it might be a problem from my side.

VojtechBartoska commented 2 years ago

@zekageri Hi, can I consider this as solved?

zekageri commented 2 years ago

Sorry. It seems the update solved the problem. Thank you.