• Please review our updated Terms and Rules here

What's wrong with my C# implementation of XMODEM?

nullvalue

Veteran Member
Joined
Oct 8, 2019
Messages
555
Location
Indiana
I'm trying to implement the original XMODEM protocol (128-byte packets with 1-byte checksum) in C#. So far just implementing the send file side of things. I get the 0x15 (NAK) from the client and send my packet, but the client doesn't "see" it, it just sends a NAK again after 3 seconds. Instead of serial communications I am doing this over a socket. (TcpClient GetStream() returns the NetworkStream being used in this class). I have tried a few different telnet terminal clients including Qodem, SyncTERM and Tera Term. Same thing happens in all of them - so what'd I miss? Thanks

C#:
using System;
using System.Collections.Generic;
using System.Linq;
using System.Net.Sockets;
using System.Text;
using System.Threading.Tasks;
using System.Timers;

public class xmodem
{
    private int offset = 0;
    private byte packetNumber = 1;

    public NetworkStream stream;
    public byte[] file;

    public void SendFileToClient()
    {
        offset = 0;
        while (true)
        {
            int b = stream.ReadByte();
            Console.Write(b.ToString("X2") + " ");
            switch(b)
            {
                case 0x01: //SOH
                    break;
                case 0x02: //STX
                    //SendPacket1kCRC();
                    break;
                case 0x04: //EOT
                    break;
                case 0x06: //ACK
                    packetNumber++;
                    offset += 0x80;
                    SendPacket128CS();
                    break;
                case 0x15: //NAK
                    SendPacket128CS();
                    break;
                case 0x17: //ETB
                    break;
                case 0x18: //CAN
                    break;
                case 0x43: //"C"
                    //SendPacket1kCRC();
                    break;
            }
        }
    }
    
    private void SendPacket128CS()
    {
        byte packetInverse = (byte)(0xFF - (int)packetNumber);

        var packetData = file.Skip(offset).Take(offset + 0x80);
        var cs = ComputeAdditionChecksum(packetData.ToArray());

        var packet = new List<byte>();
        packet.Add(0x01); //SOH
        packet.Add(packetNumber);
        packet.Add(packetInverse);
        packet.AddRange(packetData);
        packet.Add(cs);
        
        Console.WriteLine("Sending packet 128CS " + packetNumber.ToString() + " " + packet.ToArray().Length.ToString());
        stream.Write(packet.ToArray(), 0, packet.ToArray().Length);
        stream.Flush();
    }

    public static byte ComputeAdditionChecksum(byte[] data)
    {
        byte check = 0;
        for (int i = 0; i < data.Length; i++)
        {
            check += data[i];
        }
        return check;
    }
}
 

mdh

Experienced Member
Joined
Dec 9, 2021
Messages
134
Location
North East Ohio
Never mind, earliest was checksum, then modem crc and other flavors. I suppose it depends on the receiving end, do they use crc or checksum? Modified from original spec to one of the many other flavors that came along? I’d have to look at the source code… anyone have a z80 version of x modem code?
 

nullvalue

Veteran Member
Joined
Oct 8, 2019
Messages
555
Location
Indiana
Yep original implementation was checksum, which is what I'm going for - for now. Will implement other flavors later. And considering I'm receiving a NAK to start, the client is expecting to use the original protocol. The later improvements used other start byte codes.
 

mbbrutman

Associate Cat Herder
Staff member
Joined
May 3, 2003
Messages
6,276
Yeah, I'm also confused here ...

Just to be clear, you are doing this over a TCP socket but you are also using Telnet clients on the client side? If so you have to escape certain bytes in the data stream because proper Telnet clients using the Telnet protocol will eat those characters and try to use them as in-band signaling for Telnet. If your Telnet clients really are not using the telnet protocol then that will not be a problem for you.

(The mTCP Telnet client is capable of sending and receiving files using Xmodem and Ymodem over a TCP/IP socket with the full Telnet protocol. And the escaping is just irritating, but necessary.)
 

mdh

Experienced Member
Joined
Dec 9, 2021
Messages
134
Location
North East Ohio
Didn’t think of that, I just thought a raw socket. It does depend on if you are using a raw socket or using telnet or some other socket / destination.
 

mbbrutman

Associate Cat Herder
Staff member
Joined
May 3, 2003
Messages
6,276
You might be using a raw socket with your code, but if the other end is interpreting the bytes in a different way then you are going to have a bad time. And a true Telnet client is going to interpret 0xFF as a Telnet command, not a raw 0xFF, so it needs to be escaped if you want to send it.

If you are just sending ASCII data you are fine; Telnet will not touch anything. But binary data requires special handling.
 

nullvalue

Veteran Member
Joined
Oct 8, 2019
Messages
555
Location
Indiana
Ok thanks guys, I wasn't aware Telnet needed binary data to be escaped but that definitely explains the behavior I'm seeing. I always think of Telnet as being a raw socket but it's really not. Yes I'm almost certain the clients I mentioned are expecting me to be compatible with the Telnet protocol because I was seeing some other weird behavior in my tests too. Do you know of any resources that'd dive into what bytes need escaping and how? Otherwise I'll do some digging..
 

nullvalue

Veteran Member
Joined
Oct 8, 2019
Messages
555
Location
Indiana
http://www.faqs.org/rfcs/rfc854.html (especially the section Telnet Command Structure)

Thanks so it's pretty simple. Basically the only byte you have to escape is the 0xFF and you escape it by sending 2 0xFF's.

So that got me past the problem I had. Now I have a new problem - if I send a file with a length exactly divisible by 128, it works fine. According to the spec, if the last packet is less than 128 bytes, you pad it with the <SUB> (0x1A) byte out to 128 bytes. Then send <EOT> (0x04) to indicate transmission is over. This seems to work fine until I evaluate the file received by the client, and the saved file actually includes the <SUB> padding bytes (which I assume are supposed to be removed). Maybe I'm using the wrong padding character? I've found conflicting information online saying it should be 0x1A, 0x1C, and 0x18. I've tried all of these and none seem to work.

EDIT: I checked the mTCP source code and they're using the <SUB> char (dec 26) so pretty sure I'm using the right padding character...

I did find a few other bugs in my code along the way, so here's my current class:

C#:
using System;
using System.Collections.Generic;
using System.Linq;
using System.Net.Sockets;
using System.Text;
using System.Threading.Tasks;
using System.Timers;

public class xmodem
{
    private int offset = 0;
    private byte packetNumber = 1;
    private bool lastPacket = false;

    public NetworkStream stream;
    public byte[] file;

    public void SendFileToClient()
    {
        offset = 0;
        while (true)
        {
            int b = stream.ReadByte();
            Console.Write(b.ToString("X2") + " ");
            switch(b)
            {
                case 0x01: //SOH
                    break;
                case 0x02: //STX
                    //SendPacket1kCRC();
                    break;
                case 0x04: //EOT
                    break;
                case 0x06: //ACK
                    Console.WriteLine("ACK");
                    if (lastPacket)
                        stream.WriteByte(0x04); //EOT
                    else
                    {
                        packetNumber++;
                        offset += 0x80;
                        SendPacket128CS();
                    }
                    break;
                case 0x15: //NAK
                    Console.WriteLine("NAK");
                    if (lastPacket)
                        stream.WriteByte(0x04); //EOT
                    else
                        SendPacket128CS();
                    break;
                case 0x17: //ETB
                    break;
                case 0x18: //CAN
                    break;
                case 0x43: //"C"
                    //SendPacket1kCRC();
                    break;
            }
        }
    }
    private void SendPacket128CS()
    {
        byte packetInverse = (byte)(0xFF - (int)packetNumber);

        var bytes = file.Skip(offset).Take(0x80);
        List<byte> packetData = new List<byte>();
        foreach(byte b in bytes)
            packetData.Add(b);

        if (packetData.Count == 0)
            lastPacket = true;  
        else
        {
            while (packetData.Count < 128)
            {
                packetData.Add(0x1A);
                lastPacket = true;
            }
            var cs = ComputeAdditionChecksum(packetData.ToArray());

            var packet = new List<byte>();
            packet.Add(0x01); //SOH
            packet.Add(packetNumber);
            packet.Add(packetInverse);
            packet.AddRange(packetData);
            packet.Add(cs);

            Console.WriteLine("Sending packet 128CS " + packetNumber.ToString() + " " + packet.ToArray().Length.ToString());
            SendTelnet(packet);
        }
    }

    private void SendTelnet(List<byte> packet)
    {
        for (int i=0; i < packet.Count; i++)
        {
            if (packet[i] == 0xFF)
            {
                packet.Insert(i, 0xFF);
                i++;
            }
        }
        stream.Write(packet.ToArray(), 0, packet.Count);
        stream.Flush();
    }

    public static byte ComputeAdditionChecksum(byte[] data)
    {
        byte check = 0;
        for (int i = 0; i < data.Length; i++)
        {
            check += data[i];
        }
        return check;
    }
}
 
Last edited:

nullvalue

Veteran Member
Joined
Oct 8, 2019
Messages
555
Location
Indiana
Wow really, so the padding characters get saved? That seems... odd. I mean, I guess maybe CP/M wouldn't have care (and maybe even expect it) - but what about MS-DOS or even modern operation systems? Although I was thinking about it and there are other problems if you try to remove the padding characters.. such as what if the last byte of a binary file is legitimately supposed to be a 0x1A? then it would get chopped resulting in a corrupt file....
 

Chuck(G)

25k Member
Joined
Jan 11, 2007
Messages
40,243
Location
Pacific Northwest, USA
Padding bytes will be included, per CP/M convention (filesystem granularity was 128 bytes).
If you want exact length transfers and don't mind passing filenames and higher throughput, consider Ymodem.
 

nullvalue

Veteran Member
Joined
Oct 8, 2019
Messages
555
Location
Indiana
Padding bytes will be included, per CP/M convention (filesystem granularity was 128 bytes).
If you want exact length transfers and don't mind passing filenames and higher throughput, consider Ymodem.

Yep Ymodem was next on my list, wasn't aware of this peculiarity (from a modern perspective) with XMODEM.

There still seems to be something wrong with my implementation over Telnet though. Transfer works with SyncTERM, but gets hung with Tera Term and Qodem. It seems to get hung on packets containing 0xFF whether I escape it or not...
 

Chuck(G)

25k Member
Joined
Jan 11, 2007
Messages
40,243
Location
Pacific Northwest, USA
Dunno what to tell you--I use Ymodem to transfer multi-megabyte files from my MCU setups to my Linux Pee-see. I use miniterm, which shells out to rz/sz as needed.
 

mbbrutman

Associate Cat Herder
Staff member
Joined
May 3, 2003
Messages
6,276
Using Xmodem the padding characters are always going to be saved:
  • The receiving system has no way of knowing if those are padding characters or real data.
  • You didn't send any metadata (filesize in particular) to tell it what to do.
Ymodem fixes this shortcoming by transmitting the file size.

DOS certainly doesn't care about the padding bytes. If you had a text file it was supposed to be terminated with a ^Z. If it was a binary file, all bets are off and it's up to the application to figure out what to do. But generally it's not a problem, and some programs used the "slack space" at the end of a partially filled disk sector to score high score information or other data.
 

Chuck(G)

25k Member
Joined
Jan 11, 2007
Messages
40,243
Location
Pacific Northwest, USA
You can also send the file date (optionally) in the Ymodem zero packet.

Another benefit is that you can send batches of files; that is, Ymodem, after completing a file transfer, asks if there's another. It's also faster than Zmodem on hardwired connections (Zmodem has better line noise tolerance).
 

mbbrutman

Associate Cat Herder
Staff member
Joined
May 3, 2003
Messages
6,276
Ymodem G might be faster than Zmodem, but not regular Ymodem.

Ymodem still has to wait for ACKs on each packet. Zmodem uses a sliding window which enables it to speculatively send more data ahead, thus eliminating the lost time on the ACK packet turnaround.

(Ymodem G just assumes you are on a lossless connection, like TCP/IP that handles retries for you. So it just blasts packets with no regard for ACKs.)
 
Top