PowerBroker2 / SerialTransfer

Arduino library to transfer dynamic, packetized data fast and reliably via Serial, I2C, or SPI
MIT License
427 stars 68 forks source link

SerialTransfer.available() always 0, status is CONTINUE #75

Closed cyourch closed 2 years ago

cyourch commented 3 years ago

Describe the bug SerialTransfer.available() is always returning 0 and when I check the status its CONTINUE. I am able to get a packet to be received (via rxObj) by transmitting packets, fast, from the Windows host.

Essentially calling this code 2 times a second: uint16_t sendSize = m_comPortTransfer.txObj(msg, 0, sizeof(msg)); m_comPortTransfer.sendData(sendSize);

BTW, I ported SerialTransfer to Windows and it was quite easy! I created a simple Stream class wrapping a COM port class that works on Windows.

To Reproduce

  1. Connect COM port from Windows to an Arduino UNO.
  2. Baud and port settings are identical
  3. Changing baud doesn't matter.

Expected behavior SerialTransfer.available() should occasionally return non zero and then I can call rxObj to get the bytes.

Screenshots If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

**Arduino Uno -Latest version of the Sketch IDE.

PowerBroker2 commented 3 years ago

Can you post your entire transmit code and arduino code? Thanks!

cyourch commented 3 years ago

Windows:

    configST config;
    m_comPortTransfer.begin(m_ComPortStream, config);

    MSG msg;
    msg.msg_number  = 3;
    msg.dst_address = 10;
    msg.src_address = 250;
    msg.length = 1;
    msg.payload[0] = id_number;    // payload max of 16 bytes

    uint16_t sendSize = m_comPortTransfer.txObj(msg, 0, sizeof(msg));
    m_comPortTransfer.sendData(sendSize);

Arduino:

SerialTransfer & m_comPortTransfer = *(new SerialTransfer());

void setup()
{
    Serial.begin(115200);
    while (Serial == 0)
    {
    }

    m_comPortTransfer.begin(Serial);
}

void loop()
{
    if (m_comPortTransfer.available())
    {
        MSG msg;

        uint16_t recSize = m_comPortTransfer.rxObj(msg, 0, sizeof(msg));
        if (recSize > 0)
        {
            // Handle the message we just received.

        }
    }
    else
    {
        if (m_comPortTransfer.status == CONTINUE)                //= 3;
            sendDebugMessage("CONTINUE"); 
        else if (m_comPortTransfer.status == NEW_DATA)           //= 2;
            sendDebugMessage("NEW_DATA"); 
        else if (m_comPortTransfer.status == NO_DATA)            //= 1;
            sendDebugMessage("NO_DATA"); 
        else if (m_comPortTransfer.status == CRC_ERROR)          //= 0;
            sendDebugMessage("CRC_ERROR"); 
        else if (m_comPortTransfer.status == PAYLOAD_ERROR)      //= -1;
            sendDebugMessage("PAYLOAD_ERROR"); 
        else if (m_comPortTransfer.status == STOP_BYTE_ERROR)    //= -2;
            sendDebugMessage("STOP_BYTE_ERROR"); 
        else if (m_comPortTransfer.status == STALE_PACKET_ERROR) //= -3;
            sendDebugMessage("STALE_PACKET_ERROR"); 
        else
            sendDebugMessage("??????"); 
    }

}

void sendDebugMessage(const char * pszMessage)
{
    RDCMSG msg;

    msg.msg_number = MSG_DL_PRINT;
    msg.dst_address = 250;                   // Send back over the COM port to the Windows Host.
    msg.src_address = 100;                   // Our I2C address.
    msg.payload[0] = ID_LED_CONTROLLER;      // LED controller's symbolic ID

    unsigned msg_len = strlen(pszMessage);
    if (msg_len > PAYLOAD_LENGTH - 1)        // 1 byte less here. Need space for the LED controller's symbolic ID.
        msg_len = PAYLOAD_LENGTH - 1;

    msg.length = 1 + msg_len;
    memcpy((char *)&msg.payload[1], pszMessage, msg_len); 

    uint16_t sendSize = m_comPortTransfer.txObj(msg, 0, sizeof(msg));
    m_comPortTransfer.sendData(sendSize, 0);
}
cyourch commented 3 years ago

Here's the COM port mode I write to read/write to/from a Windows COM port.

ComPortMgr.h contains the definition for the Stream class that I pass to the begin() method.

ComPortMgr.cpp.txt

ComPortMgr.h.txt

PowerBroker2 commented 3 years ago

What's the entire windows code?

PowerBroker2 commented 3 years ago

Oh wait a second, don't print if the status is CONTINUE, that is one state the parser will be in a lot of the time, especially if you're sending a lot data constantly (i.e. streaming data). In the latest version of the library, you can get around these manual prints by setting the debug flag to true in the begin() method. See here:

https://github.com/PowerBroker2/SerialTransfer/blob/932c426dc134cd6cdf428108b2a2d4a819acae65/src/SerialTransfer.h#L15

cyourch commented 3 years ago

I switched from m_comPortTransfer.begin(Serial);

To: m_comPortTransfer.begin(Serial, false);

And I still get the same behavior.

cyourch commented 3 years ago

Here's the modules I ported to Windows:

PacketCRC.h.txt SerialTransfer.cpp.txt SerialTransfer.h.txt Packet.cpp.txt Packet.h.txt .

PowerBroker2 commented 3 years ago

The behavior should definitely change since CONTINUE should not be printed to the serial monitor. I'm having a really hard time wrapping my head around your setup. What exactly are you trying to accomplish between the two incomplete programs posted in this issue? What are you expecting, what have you debugged, what exactly is going wrong?

Also, why not use Python instead of a windows C++ program? pySerialTransfer is pip-installable and super easy to use.

cyourch commented 3 years ago

I have 2 apps... a Windows Hosts app and the Arduino app. I am printing "CONTINUE" on the Windows Host, not on the Arduino. I am able to transmit from the Arduino to Windows but not the other way... which is the issue I'm having.

I am expecting that I can read packets on the Arduino that are sent from Windows.

BTW, I can read and write if I use the serial port directly so it seems that something in the SerialTransfer library is the issue. and Again, I keep getting the CONTINUE state from the Packet parser.

cyourch commented 3 years ago
    //
    // Check for messages from the Windows host app over the COM port.
    //
    if (m_comPortTransfer.available())
    {
        RDCMSG msg;

        uint16_t recSize = m_comPortTransfer.rxObj(msg, 0, sizeof(msg));
        if (recSize > 0)
        {
            comPortMessageHandler(msg);

            char sz[17];
            sprintf(sz, "Msg %d %db", msg.msg_number, recSize);
            sendDebugMessage(sz); 
        }
        else
        {
            sendDebugMessage("No COM Msg!"); 
        }
    }
    else
    {
        if (m_comPortTransfer.status == CONTINUE)                //= 3;
        {
            sendDebugMessage("CONTINUE"); 
        }
        else if (m_comPortTransfer.status == NEW_DATA)           //= 2;
        {
            sendDebugMessage("NEW_DATA"); 
        }
        else if (m_comPortTransfer.status == NO_DATA)            //= 1;
        {
            //sendDebugMessage("NO_DATA"); 
        }
        else if (m_comPortTransfer.status == CRC_ERROR)          //= 0;
        {
            sendDebugMessage("CRC_ERROR"); 
        }
        else if (m_comPortTransfer.status == PAYLOAD_ERROR)      //= -1;
        {
            sendDebugMessage("PAYLOAD_ERROR"); 
        }
        else if (m_comPortTransfer.status == STOP_BYTE_ERROR)    //= -2;
        {
            sendDebugMessage("STOP_BYTE_ERROR"); 
        }
        else if (m_comPortTransfer.status == STALE_PACKET_ERROR) //= -3;
        {
            sendDebugMessage("STALE_PACKET_ERROR"); 
        }
        else
        {
            sendDebugMessage("??????"); 
        }
    }
cyourch commented 3 years ago

This sends the CONTINUE string to my Windows app.

void sendDebugMessage(const char * pszMessage)
{
    RDCMSG msg;

    msg.msg_number = MSG_DL_PRINT;
    msg.dst_address = I2C_WINDOWS_HOST;      // Send back over the COM port to the Windows Host.
    msg.src_address = I2C_LED_CONTROLLER;    // Our I2C address.
    msg.payload[0] = ID_LED_CONTROLLER;      // LED controller's symbolic ID

    unsigned msg_len = strlen(pszMessage);
    if (msg_len > PAYLOAD_LENGTH - 1)        // 1 byte less here. Need space for the LED controller's symbolic ID.
        msg_len = PAYLOAD_LENGTH - 1;

    msg.length = 1 + msg_len;
    memcpy((char *)&msg.payload[1], pszMessage, msg_len); 

    uint16_t sendSize = m_comPortTransfer.txObj(msg, 0, TRANSMIT_SIZE(msg));
    m_comPortTransfer.sendData(sendSize, 0);
}
cyourch commented 3 years ago

The behavior I'm seeing is as if the packet delimiter(s) are missing or not found and the start of a packet, when sent from Windows to the Arduino, is lost.

PowerBroker2 commented 3 years ago

I've been using the latest version of the library for communication in several of my personal projects and haven't had any issues. I suspect the issue is with your ported parsing algorithm, not the original Arduino library. Unless you can prove the original library is at fault, I can't help. However, I am more than happy to bug fix the library if you can prove the bug exists.

cyourch commented 3 years ago

Here's the trace output when I send from Windows to the Arduino direct over the Serial port, no SerialTransfer calls.

Windows SEND:

MSG_DL_STATUS_REQUEST(4): dst=10, src=150, 01 bytes, DataLogger #1 write_bytes(): 4 bytes of data to 'COM6'. write_bytes(): 5 bytes of data to 'COM6'. write_bytes(): 2 bytes of data to 'COM6'.

Arduino RECEIVE on COM6:

1st loop() call to Serial.available() and then Serial.read(): MSG_DL_PRINT(9): dst=150, src=119, 09 bytes, LED Controller #100--> 126 '~' MSG_DL_PRINT(9): dst=150, src=119, 07 bytes, LED Controller #100--> 000 ' MSG_DL_PRINT(9): dst=150, src=119, 09 bytes, LED Controller #100--> 255 'ÿ' MSG_DL_PRINT(9): dst=150, src=119, 09 bytes, LED Controller #100--> 005 '' MSG_DL_PRINT(9): dst=150, src=119, 08 bytes, LED Controller #100--> 4 bytes

2nd loop() call to Serial.available() and then Serial.read(): MSG_DL_PRINT(9): dst=150, src=119, 09 bytes, LED Controller #100--> 004 '' MSG_DL_PRINT(9): dst=150, src=119, 10 bytes, LED Controller #100--> 010 MSG_DL_PRINT(9): dst=150, src=119, 09 bytes, LED Controller #100--> 150 '–' MSG_DL_PRINT(9): dst=150, src=119, 09 bytes, LED Controller #100--> 001 '' MSG_DL_PRINT(9): dst=150, src=119, 09 bytes, LED Controller #100--> 001 '' MSG_DL_PRINT(9): dst=150, src=119, 09 bytes, LED Controller #100--> 124 '|' MSG_DL_PRINT(9): dst=150, src=119, 09 bytes, LED Controller #100--> 129 '' MSG_DL_PRINT(9): dst=150, src=119, 08 bytes, LED Controller #100--> 7 bytes

cyourch commented 3 years ago

You can see that the entire packet is transmitted from Windows and that the Serial object is reading in the entire packet.

cyourch commented 3 years ago

When I replace this: while (Serial.available()) { uint8_t b = Serial.read(); }

With a call to the SerialTranbsfer library: if (m_comPortTransfer.available()) { MSG msg;

    uint16_t recSize = m_comPortTransfer.rxObj(msg, 0, sizeof(msg));
}

It stops working and m_comPortTransfer.status == CONTINUE

cyourch commented 3 years ago

Here's the fix! When bytes are transmitted from Windows they are sent out in 3 separate calls to port->write(). When I grouped them together the transfer worked.

uint8_t SerialTransfer::sendData(const uint16_t& messageLen, const uint8_t packetID) { uint8_t numBytesIncl;

numBytesIncl = packet.constructPacket(messageLen, packetID);

if ORIGINAL_WAY

port->write(packet.preamble, sizeof(packet.preamble));
port->write(packet.txBuff, numBytesIncl);
port->write(packet.postamble, sizeof(packet.postamble));

else

uint8_t big_buffer[PREAMBLE_SIZE + MAX_PACKET_SIZE + POSTAMBLE_SIZE + 1];
unsigned so = sizeof(big_buffer);
unsigned len = 0;

memcpy(&big_buffer[len], packet.preamble, sizeof(packet.preamble));
len += sizeof(packet.preamble);

memcpy(&big_buffer[len], packet.txBuff, numBytesIncl);
len += numBytesIncl;

memcpy(&big_buffer[len], packet.postamble, sizeof(packet.postamble));
len += sizeof(packet.postamble);

port->write(big_buffer, len);

endif

return numBytesIncl;

}

cyourch commented 3 years ago

From the code in available() in SerialTransfer it seems like the packet parsing state is lost between calls. The change I made above causes all the code to be read in one shot and then it works.