From b131fbe8ae072b34e45dae16930fbfedc5f81d09 Mon Sep 17 00:00:00 2001 From: Tom Nisbet Date: Wed, 14 Oct 2020 11:58:14 -0400 Subject: [PATCH] Fix #19 - XModem changed from 16-bit CRC to 8-bit checksum --- TommyPROM/CmdStatus.h | 3 +- TommyPROM/TommyPROM.ino | 4 +-- TommyPROM/XModem.cpp | 78 ++++++++++++++++++++++++++++++++++++----- TommyPROM/XModem.h | 18 ---------- docs/index.md | 15 ++++---- docs/software.md | 6 ++-- 6 files changed, 83 insertions(+), 41 deletions(-) diff --git a/TommyPROM/CmdStatus.h b/TommyPROM/CmdStatus.h index 567c02e..f731046 100644 --- a/TommyPROM/CmdStatus.h +++ b/TommyPROM/CmdStatus.h @@ -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); }; - diff --git a/TommyPROM/TommyPROM.ino b/TommyPROM/TommyPROM.ino index 233bbf1..bbc4ea3 100644 --- a/TommyPROM/TommyPROM.ino +++ b/TommyPROM/TommyPROM.ino @@ -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) { diff --git a/TommyPROM/XModem.cpp b/TommyPROM/XModem.cpp index effe800..43408c5 100644 --- a/TommyPROM/XModem.cpp +++ b/TommyPROM/XModem.cpp @@ -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); } diff --git a/TommyPROM/XModem.h b/TommyPROM/XModem.h index 6c3f1b9..b4e6824 100644 --- a/TommyPROM/XModem.h +++ b/TommyPROM/XModem.h @@ -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 - diff --git a/docs/index.md b/docs/index.md index 123265e..7db83e8 100755 --- a/docs/index.md +++ b/docs/index.md @@ -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. diff --git a/docs/software.md b/docs/software.md index 08a20eb..387cf2c 100644 --- a/docs/software.md +++ b/docs/software.md @@ -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