Fix #19 - XModem changed from 16-bit CRC to 8-bit checksum

This commit is contained in:
Tom Nisbet 2020-10-14 11:58:14 -04:00
parent cb71caca4b
commit b131fbe8ae
6 changed files with 83 additions and 41 deletions

View File

@ -23,7 +23,7 @@ class CmdStatus
private:
enum
{
MAX_VALUES = 3
MAX_VALUES = 5
};
enum StatusLevel { SL_NONE, SL_INFO, SL_ERROR };
enum ValueType { VT_NONE, VT_DEC, VT_HEX };
@ -41,4 +41,3 @@ private:
void setLongValue(int index, const char * label, long value, ValueType vt);
};

View File

@ -19,7 +19,7 @@
#include "XModem.h"
static const char * MY_VERSION = "2.4";
static const char * MY_VERSION = "2.5";
// Global status
@ -692,7 +692,6 @@ void loop()
break;
case CMD_READ:
Serial.println(F("Set the terminal to receive XMODEM CRC"));
if (xmodem.SendFile(start, uint32_t(end) - start + 1))
{
cmdStatus.info("Send complete.");
@ -707,7 +706,6 @@ void loop()
case CMD_WRITE:
prom.resetDebugStats();
Serial.println(F("Send the image file using XMODEM CRC"));
numBytes = xmodem.ReceiveFile(start);
if (numBytes)
{

View File

@ -1,6 +1,40 @@
#include "XModem.h"
#include "CmdStatus.h"
// The original TommyPROM code used XModem CRC protocol but some Linux communication
// programs do not seem to support this correctly. The default is now to use basic XModem
// with the 8-bit checksum instead of the 16-bit CRC error checking. This shouldn't
// matter because communication errors aren't likely to happen on a 3 foot USB cable like
// they did over the long distance dail-up lines that XModem was designed for. Uncomment
// the XMODEM_CRC_PROTOCOL line below to restore the original XMODEM CRC support.
//#define XMODEM_CRC_PROTOCOL
enum
{
// XMODEM control characters.
XMDM_SOH = 0x01,
XMDM_EOT = 0x04,
XMDM_ACK = 0x06,
XMDM_NAK = 0x15,
XMDM_CAN = 0x18,
XMDM_ESC = 0x1b,
XMDM_CRC = 'C',
#ifdef XMODEM_CRC_PROTOCOL
XMDM_TRANSFER_START = XMDM_CRC
#else
XMDM_TRANSFER_START = XMDM_NAK
#endif
};
enum
{
// Misc constants for XMODEM.
PKTLEN = 128
};
uint32_t XModem::ReceiveFile(uint32_t address)
{
uint8_t buffer[PKTLEN];
@ -9,6 +43,11 @@ uint32_t XModem::ReceiveFile(uint32_t address)
uint32_t numBytes = 0;
bool complete = false;
#ifdef XMODEM_CRC_PROTOCOL
Serial.println(F("Send the image file using XMODEM CRC"));
#else
Serial.println(F("Send the image file using XMODEM"));
#endif
if (!StartReceive())
{
cmdStatus.error("Timeout waiting for transfer to start.");
@ -78,13 +117,22 @@ bool XModem::SendFile(uint32_t address, uint32_t fileSize)
int rxChar = -1;
uint32_t bytesSent = 0;
#ifdef XMODEM_CRC_PROTOCOL
Serial.println(F("Set the terminal to receive XModem CRC"));
#else
Serial.println(F("Set the terminal to receive XModem"));
#endif
while (rxChar == -1)
{
rxChar = GetChar();
}
if (rxChar != XMDM_CRC)
if (rxChar != XMDM_TRANSFER_START)
{
cmdStatus.error("Expected XModem CRC start char.");
#ifdef XMODEM_CRC_PROTOCOL
cmdStatus.error("Expected XModem CRC start char");
#else
cmdStatus.error("Expected XModem NAK char to start");
#endif
cmdStatus.setValueDec(0, "char", rxChar);
return false;
}
@ -145,6 +193,7 @@ int XModem::GetChar(int msWaitTime)
uint16_t XModem::UpdateCrc(uint16_t crc, uint8_t data)
{
#ifdef XMODEM_CRC_PROTOCOL
crc = crc ^ ((uint16_t)data << 8);
for (int ix = 0; (ix < 8); ix++)
{
@ -157,7 +206,9 @@ uint16_t XModem::UpdateCrc(uint16_t crc, uint8_t data)
crc <<= 1;
}
}
#else
crc = (crc + data) & 0xff;
#endif
return crc;
}
@ -166,11 +217,11 @@ bool XModem::StartReceive()
{
for (int retries = 30; (retries); --retries)
{
// Send the 'C' character, indicating a CRC16 XMODEM transfer, until the sender
// of the file responds with something. The start character will be sent once a
// second for a number of seconds. If nothing is received in that time then
// return false to indicate that the transfer did not start.
Serial.write('C');
// Send the XMDM_TRANSFER_START until the sender of the file responds with
// something. The start character will be sent once a second for a number of
// seconds. If nothing is received in that time then return false to indicate
// that the transfer did not start.
Serial.write(XMDM_TRANSFER_START);
for (int ms = 1000; (ms); --ms)
{
if (Serial.available() > 0)
@ -213,15 +264,22 @@ bool XModem::ReceivePacket(uint8_t buffer[], unsigned bufferSize, uint8_t seq, u
calcCrc = UpdateCrc(calcCrc, buffer[ix]);
}
#ifdef XMODEM_CRC_PROTOCOL
rxCrc = ((uint16_t) GetChar()) << 8;
rxCrc |= GetChar();
#else
rxCrc = GetChar();
#endif
if ((calcCrc != rxCrc) || (rxSeq1 != seq) || ((rxSeq1 ^ rxSeq2) != 0xff))
{
// Fail if the CRC or sequence number is not correct or if the two received
// sequence numbers are not the complement of one another.
cmdStatus.error("Bad CRC or sequence number.");
cmdStatus.setValueDec(0, "seq", seq);
cmdStatus.setValueDec(1, "rxSeq1", rxSeq1);
cmdStatus.setValueDec(2, "rxSeq2", rxSeq2);
cmdStatus.setValueHex(3, "calcCrc", calcCrc);
cmdStatus.setValueHex(4, "rxCrc", rxCrc);
Serial.write(XMDM_CAN);
return false;
}
@ -254,6 +312,8 @@ void XModem::SendPacket(uint32_t address, uint8_t seq)
Serial.write(c);
crc = UpdateCrc(crc, c);
}
#ifdef XMODEM_CRC_PROTOCOL
Serial.write(crc >> 8);
#endif
Serial.write(crc & 0xff);
}

View File

@ -27,23 +27,6 @@ class XModem
void Cancel();
private:
enum
{
// XMODEM control characters.
XMDM_SOH = 0x01,
XMDM_EOT = 0x04,
XMDM_ACK = 0x06,
XMDM_NAK = 0x15,
XMDM_CAN = 0x18,
XMDM_ESC = 0x1b,
XMDM_CRC = 'C'
};
enum
{
// Misc constants for XMODEM.
PKTLEN = 128
};
PromDevice & promDevice;
CmdStatus & cmdStatus;
@ -55,4 +38,3 @@ class XModem
};
#endif // #define INCLUDE_CONFIGURE_H

View File

@ -70,9 +70,7 @@ program, such as TeraTerm (Windows) or Minicom (Linux). The Arduino development
Monitor can also be used as a terminal initially, but it does not support XMODEM
transfers, so the READ and WRITE commands can't be used.
When using minicom, a few parameters need to be set. Disable both hardware and software
flow control. Also add the '-c' command line option to the two xmodem settings to enable
CRC mode for the file transfers.
Disable both hardware and software flow control in the minicom settings for best results.
Set the terminal's serial parameters to 115200 baud, 8 bits, no parity, 1 stop bit to
match the Arduino. Press the Enter key. If the connection is successful, TommyPROM will
@ -93,11 +91,16 @@ command receives a file from the host and writes (burns) it into the device. Th
command needs a start and end address. The W command determines the end address from the
received file size.
The READ and WRITE commands both use XMODEM CRC to complete the file transfers. Many
terminal programs default to XMODEM Checksum format, so be sure to select XMODEM CRC as
the format. Once the READ or WRITE command is issued to the programmer, the transfer must
Once the READ or WRITE command is issued to the programmer, the transfer must
be started on the host program.
Note that previous versions of TommyPROM used the XMODEM-CRC protocol to complete the file
transfers for the READ and WRITE commands. This did not work well with minicom and other
Linux programs that rely on the sz/rz commands. Versions 2.5 and later of TommyPROM now
use basic XModem with the 8-bit checksum. The XModem-CRC support is still available as a
compile-time option. See [issue #19](https://github.com/TomNisbet/TommyPROM/issues/19)
for details.
The files used for READ and WRITE are simple binary images. This can be created directly
by [asm85](http://github.com/TomNisbet/asm85) or can be converted from S-record or Intel
HEX using an external utility.

View File

@ -24,9 +24,9 @@ having to build parameterized error messages with multiple print calls.
## Xmodem class
The Xmodem class implements the communications protocols needed to do XMODEM CRC transmit
and receive. It calls directly into the PROM read and write code, to the complete files
are never stored during the transfer.
The Xmodem class implements the communications protocols needed to do XMODEM or XMODEM-CRC
transmit and receive. It calls directly into the PROM read and write code, so the
complete files are never stored during the transfer.
## CLI code and command implementation