Xmodem-CRC protocol in c# problem

Question

I'm porting a C++ xmodem protocol to C# and I'm having an issue with the CRC check. The application uploads files through a modem using the xmodem 128 byte protocol with CRC. I test it using Hyperterminal. I can upload small files fine, but when I try larger files (50K +) the CRC always breaks when the low and high bytes are 255. Can someone help me with this? Thanks!

Here's the CRC code...

..
    if (check(true, _data, 3, 133))
                                {
                                    _currentState = State.Good;
                                }
                                else
                                {
                                    _currentState = State.Bad;


}
...
private bool check(bool isCRC, byte[] buf, int index, int sz)
        {
            if (isCRC) // use CRC checking
            {
                ushort crc = CRC16.CRC16_ccitt(buf, index, sz);
                ushort tcrc = (ushort)((buf[sz + index] << 8) + buf[sz + index + 1]);
                if (crc == tcrc)
                {

                    return true;
                }
            }
            else
            { // use Checksum checking
                int i;
                byte cks = 0;
                for (i = 0; i < sz; ++i)
                {
                    cks += buf[i + index];
                }
                if (cks == buf[sz + index])
                    return true;
            }
            return false;
        }

internal static class CRC16
{
    #region Members

    //size = 256
    private static ushort[] crc16tab = {
      0x0000,0x1021,0x2042,0x3063,0x4084,0x50a5,0x60c6,0x70e7,
      0x8108,0x9129,0xa14a,0xb16b,0xc18c,0xd1ad,0xe1ce,0xf1ef,
      0x1231,0x0210,0x3273,0x2252,0x52b5,0x4294,0x72f7,0x62d6,
      0x9339,0x8318,0xb37b,0xa35a,0xd3bd,0xc39c,0xf3ff,0xe3de,
      0x2462,0x3443,0x0420,0x1401,0x64e6,0x74c7,0x44a4,0x5485,
      0xa56a,0xb54b,0x8528,0x9509,0xe5ee,0xf5cf,0xc5ac,0xd58d,
      0x3653,0x2672,0x1611,0x0630,0x76d7,0x66f6,0x5695,0x46b4,
      0xb75b,0xa77a,0x9719,0x8738,0xf7df,0xe7fe,0xd79d,0xc7bc,
      0x48c4,0x58e5,0x6886,0x78a7,0x0840,0x1861,0x2802,0x3823,
      0xc9cc,0xd9ed,0xe98e,0xf9af,0x8948,0x9969,0xa90a,0xb92b,
      0x5af5,0x4ad4,0x7ab7,0x6a96,0x1a71,0x0a50,0x3a33,0x2a12,
      0xdbfd,0xcbdc,0xfbbf,0xeb9e,0x9b79,0x8b58,0xbb3b,0xab1a,
      0x6ca6,0x7c87,0x4ce4,0x5cc5,0x2c22,0x3c03,0x0c60,0x1c41,
      0xedae,0xfd8f,0xcdec,0xddcd,0xad2a,0xbd0b,0x8d68,0x9d49,
      0x7e97,0x6eb6,0x5ed5,0x4ef4,0x3e13,0x2e32,0x1e51,0x0e70,
      0xff9f,0xefbe,0xdfdd,0xcffc,0xbf1b,0xaf3a,0x9f59,0x8f78,
      0x9188,0x81a9,0xb1ca,0xa1eb,0xd10c,0xc12d,0xf14e,0xe16f,
      0x1080,0x00a1,0x30c2,0x20e3,0x5004,0x4025,0x7046,0x6067,
      0x83b9,0x9398,0xa3fb,0xb3da,0xc33d,0xd31c,0xe37f,0xf35e,
      0x02b1,0x1290,0x22f3,0x32d2,0x4235,0x5214,0x6277,0x7256,
      0xb5ea,0xa5cb,0x95a8,0x8589,0xf56e,0xe54f,0xd52c,0xc50d,
      0x34e2,0x24c3,0x14a0,0x0481,0x7466,0x6447,0x5424,0x4405,
      0xa7db,0xb7fa,0x8799,0x97b8,0xe75f,0xf77e,0xc71d,0xd73c,
      0x26d3,0x36f2,0x0691,0x16b0,0x6657,0x7676,0x4615,0x5634,
      0xd94c,0xc96d,0xf90e,0xe92f,0x99c8,0x89e9,0xb98a,0xa9ab,
      0x5844,0x4865,0x7806,0x6827,0x18c0,0x08e1,0x3882,0x28a3,
      0xcb7d,0xdb5c,0xeb3f,0xfb1e,0x8bf9,0x9bd8,0xabbb,0xbb9a,
      0x4a75,0x5a54,0x6a37,0x7a16,0x0af1,0x1ad0,0x2ab3,0x3a92,
      0xfd2e,0xed0f,0xdd6c,0xcd4d,0xbdaa,0xad8b,0x9de8,0x8dc9,
      0x7c26,0x6c07,0x5c64,0x4c45,0x3ca2,0x2c83,0x1ce0,0x0cc1,
      0xef1f,0xff3e,0xcf5d,0xdf7c,0xaf9b,0xbfba,0x8fd9,0x9ff8,
      0x6e17,0x7e36,0x4e55,0x5e74,0x2e93,0x3eb2,0x0ed1,0x1ef0
  };

    #endregion Members

    #region Implementation

    static public ushort CRC16_ccitt(byte[] buf, int index, int len)
    {
        int counter;
        ushort crc = 0;
        for (counter = 0; counter < len; counter++)
            crc = (ushort)((crc << 8) ^ crc16tab[((crc >> 8) ^ buf[index + counter]) & 0x00FF]);
        return crc;
    }

    #endregion Implementation
}

Here's the 133 byte array it fails on.

    [0] 1   byte
    [1] 127 byte
    [2] 128 byte
    [3] 84  byte
    [4] 52  byte
    [5] 51  byte
    [6] 49  byte
    [7] 48  byte
    [8] 50  byte
    [9] 51  byte
    [10]    57  byte
    [11]    50  byte
    [12]    48  byte
    [13]    70  byte
    [14]    48  byte
    [15]    48  byte
    [16]    48  byte
    [17]    48  byte
    [18]    48  byte
    [19]    48  byte
    [20]    48  byte
    [21]    48  byte
    [22]    48  byte
    [23]    48  byte
    [24]    84  byte
    [25]    32  byte
    [26]    32  byte
    [27]    32  byte
    [28]    32  byte
    [29]    32  byte
    [30]    32  byte
    [31]    32  byte
    [32]    32  byte
    [33]    32  byte
    [34]    32  byte
    [35]    32  byte
    [36]    32  byte
    [37]    32  byte
    [38]    32  byte
    [39]    32  byte
    [40]    32  byte
    [41]    32  byte
    [42]    32  byte
    [43]    32  byte
    [44]    32  byte
    [45]    32  byte
    [46]    32  byte
    [47]    32  byte
    [48]    32  byte
    [49]    32  byte
    [50]    32  byte
    [51]    32  byte
    [52]    32  byte
    [53]    32  byte
    [54]    32  byte
    [55]    32  byte
    [56]    32  byte
    [57]    32  byte
    [58]    32  byte
    [59]    32  byte
    [60]    32  byte
    [61]    32  byte
    [62]    32  byte
    [63]    32  byte
    [64]    32  byte
    [65]    32  byte
    [66]    32  byte
    [67]    32  byte
    [68]    32  byte
    [69]    48  byte
    [70]    49  byte
    [71]    45  byte
    [72]    32  byte
    [73]    32  byte
    [74]    32  byte
    [75]    32  byte
    [76]    32  byte
    [77]    32  byte
    [78]    32  byte
    [79]    32  byte
    [80]    32  byte
    [81]    32  byte
    [82]    13  byte
    [83]    10  byte
    [84]    80  byte
    [85]    48  byte
    [86]    51  byte
    [87]    66  byte
    [88]    90  byte
    [89]    66  byte
    [90]    32  byte
    [91]    53  byte
    [92]    55  byte
    [93]    54  byte
    [94]    53  byte
    [95]    53  byte
    [96]    48  byte
    [97]    48  byte
    [98]    48  byte
    [99]    48  byte
    [100]   84  byte
    [101]   52  byte
    [102]   51  byte
    [103]   77  byte
    [104]   79  byte
    [105]   51  byte
    [106]   55  byte
    [107]   49  byte
    [108]   56  byte
    [109]   48  byte
    [110]   48  byte
    [111]   48  byte
    [112]   48  byte
    [113]   48  byte
    [114]   48  byte
    [115]   48  byte
    [116]   48  byte
    [117]   48  byte
    [118]   48  byte
    [119]   56  byte
    [120]   57  byte
    [121]   52  byte
    [122]   48  byte
    [123]   52  byte
    [124]   57  byte
    [125]   32  byte
    [126]   32  byte
    [127]   32  byte
    [128]   32  byte
    [129]   32  byte
    [130]   32  byte
    [131]   255 byte
    [132]   255 byte

UPDATE I failed to mention that I'm not reading from a serial port. We have a modem bank that receives calls and opens a connection to our service using TCP/IP. So when I read the bytes I'm reading from a TcpClients stream.

I also noticed that when I send a file to my service using Hyperterminal, packet 255 has a packet number of 255 and a compliment of 255. The compliment should be 0, right?

`buf[sz + index]` and `buf[sz + index + 1]`. That's as far as I got. You have a method that accepts an array, an offset, and a length and then reads outside those bounds. I can't say that your problem is there, but I can say that you are standing on thin ice. — Tergiver, Mar 12 '11 at 15:59
Also, if you want others to help, you might consider giving us a complete, compilable sample that demonstrates the behavior so that we don't have to reconstruct it ourselves. In doing so you may even find the problem yourself. — Tergiver, Mar 12 '11 at 16:02
Good point on the buf issue. I'll fix that. I'll try and get a compilable sample...I'm not sure I'm allowed to post our code out like that, maybe I can do a subset that focuses on this only. I'll see what I can do. Thank you for your help. — Dan H, Mar 12 '11 at 16:11
*but when I try larger files (50K +)...* XMODEM is packets of 128 bytes with a single byte packet # that begins with packet #1. That's 127 packets * 128 bytes = 16,256 bytes total. Unless the packet numbers are ignored and only the EOT is considered? — Tergiver, Mar 13 '11 at 15:27

Tergiver · Answer 1 · 2011-03-13T16:51:01.693

Deleted my other "answer" about the CRC because I don't think that is the problem. Dan mentioned in a comment that he got the code from http://trackday.cc/b2evo/blog2.php/2007/08/02/net-xmodem. The code pasted in the original question comes directly from that source.

I downloaded it and started looking it over. It has a lot of flaws(*), mostly non-fatal. However, this part jumped out at me in the XModemReceive method:

#region fill packet with datastream

xbuff[0] = (byte)c;
for (i = 0; i < (bufsz + (useCRC ? 1 : 0) + 3); i++)
{
    xbuff[i + 1] = (byte)this.port.ReadByte();
}

#endregion

If SerialPort.ReadByte encounters End Of Stream it will return -1 (i.e. 255 in byte form). If ReadByte had timed out, a TimeoutException would occur. If EOS is encountered, the correct course of action might be to abort the current packet. I have not spent any time thinking that through.

First I would alter that code just to test if this is in fact the problem you're seeing:

xbuff[0] = (byte)c;
for (i = 0; i < (bufsz + (useCRC ? 1 : 0) + 3); i++)
{
    int byteValue = this.port.ReadByte();
    Debug.Assert(byteValue >= 0, "byteValue >= 0");
    xbuff[i + 1] = (byte)byteValue;
}

That is not a fix, just a test. If it is asserting (in a Debug build), than we can think about fixing it.

(*) I blogged about the first few 'flaws' if you're interested in reading about them.

Thank you for helping me with this. I failed to mention that I am only using the crc check from that code. I'm not using the reading of bytes for xmodem receive, mainly becuase I'm not reading from a serial port. I'm reading from a TcpClient. We have a modem bank that takes calls and then connects the client to the service I'm writing using TCP/IP. I'll edit my question to include that... — Dan H, Mar 13 '11 at 17:10

score 1 · Accepted Answer · answered Mar 16 '11 at 13:35

I figured out the issue I was having. First, my setup. We have a modem bank that takes incoming calls and routes them using telnet TCP/IP to a service running on a windows box. The problem I was having was every time a 255 byte was sent by the client it was preceded by a 255 byte. Since 255 is a valid byte for xmodem packet numbers, packet number compliments and CRC it because an issue when it sent two of them. The reason it was sending two 255 bytes was because Telnet TCP/IP uses byte 255. So, when a 255 byte needs to be sent Telnet will escape it with another 255 byte. This would have never happened if I was working over a serial connection. Since our Modem bank connects to our service using telnet TCP/IP we need to handle the 255 bytes that get escaped. Once I did that everything worked great. I used Hans Passant and Tergiver's advice to fix the errors in my xmodem code, so thank you very much to them for their help.

Xmodem-CRC protocol in c# problem

2 Answers2