Simplify Class1SwitchingCmd code and prevent duplication of the command in-session

From Lee:
|  commit e152937c32a44a47e6e07f0ef98a2c6b979321de
|  Author: Lee Howard <faxguy@howardsilvan.com>
|  Date:   Thu Sep 7 18:59:29 2006 +0000
|
|    okay, this should prevent the AT+FRS=7 duplication.  It simplifies all
|    Class1SwitchingCmd processing and it provides for an abort situation, too.

diff --git a/CHANGES b/CHANGES
index c897172366d290c195dcae575975be097547c415..9254007bf5c18284f2cfc299fc4d57291a3112d1 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -2,6 +2,8 @@
 
 Changelog since HylaFAX 4.3.3
 
+* Simplify Class1SwitchingCmd code and prevent duplication of the
+  command in session (12 Mar 2007)
 * Replace Class1TrainingRecovery with Class1SwitchingCmd
 * Fix error of sending a block twice, empty the 2nd time (12 Mar 2007)
 * Don't trigger hasV17Trouble for pages other than the first (12 Mar 2007)

diff --git a/faxd/Class1.c++ b/faxd/Class1.c++
index 7aea350e524c64a2b98a356404e9ef2be6c040ca..0f585f041e986bb368e8798853ddfb29f6acd24f 100644 (file)
--- a/faxd/Class1.c++
+++ b/faxd/Class1.c++
@@ -103,6 +103,7 @@ Class1Modem::Class1Modem(FaxServer& s, const ModemConfig& c)
     fxAssert(ecmStuffedBlock != NULL, "ECM procedure error (stuffed block).");
     gotCTRL = false;
     repeatPhaseB = false;
+    silenceHeard = false;
 }
 
 Class1Modem::~Class1Modem()
@@ -112,6 +113,13 @@ Class1Modem::~Class1Modem()
     free(ecmStuffedBlock);
 }
 
+bool
+Class1Modem::atCmd(const fxStr& cmd, ATResponse r, long ms)
+{
+    silenceHeard = false;
+    return (ClassModem::atCmd(cmd, r, ms));
+}
+
 /*
  * Check if the modem is a Class 1 modem and,
  * if so, configure it for use.
@@ -458,6 +466,35 @@ Class1Modem::decodePWD(fxStr& ascii, const HDLCFrame& binary)
     return decodeTSI(ascii, binary);
 }
 
+bool
+Class1Modem::switchingPause(fxStr& emsg)
+{
+    if (!silenceHeard && !atCmd(conf.class1SwitchingCmd, AT_OK)) {
+       emsg = "Failure to receive silence.";
+       protoTrace(emsg);
+       if (wasTimeout()) abortReceive();
+       return (false);
+    }
+    /*
+     * We necessarily use class1SwitchingCmd rather frequently... 
+     * sometimes this is programmed to be immediately before our signalling,
+     * and sometimes this is programmed to be immediately after the remote
+     * signalling.  That doesn't therefore give us very good control on 
+     * ensuring that class1SwitchingCmd is not issued in duplicate in some
+     * kinds of program flow.
+     *
+     * Ideally we'd be able to standardize on issuing class1SwitchingCmd
+     * only in one case or another (but not both), but that's not particularly
+     * an easy ideal to realize given the number of exceptions to that rule
+     * that would neccesarily be required.
+     *
+     * So for now we just set this flag here, and we unset it in atCmd, and 
+     * if the flag is set when we come back here, then we can avoid duplication.
+     */
+    silenceHeard = true;
+    return (true);
+}
+
 /*
  * Pass data to modem, filtering DLE's and
  * optionally including the end-of-data
@@ -1345,6 +1382,7 @@ Class1Modem::recvFrame(HDLCFrame& frame, u_char dir, long ms, bool readPending,
             abortReceive();
             return (false);
         }
+       fxStr emsg;
        do {
            if (crpcnt) {
                traceFCF(dir == FCF_SNDR ? "SEND send" : "RECV send", FCF_CRP);
@@ -1359,7 +1397,7 @@ Class1Modem::recvFrame(HDLCFrame& frame, u_char dir, long ms, bool readPending,
            frame.reset();
             gotframe = recvRawFrame(frame);
        } while (!gotframe && docrp && crpcnt++ < 3 && !wasTimeout() &&
-               atCmd(conf.class1SwitchingCmd, AT_OK) && transmitFrame(dir|FCF_CRP));
+               switchingPause(emsg) && transmitFrame(dir|FCF_CRP));
        return (gotframe);
     } else if (lastResponse == AT_ERROR) gotEOT = true;                // on hook
     stopTimeout("waiting for v.21 carrier");

diff --git a/faxd/Class1.h b/faxd/Class1.h
index 7169bbae4558a2d392d97a8ee8abb2adfc4cca8c..c1f8dea606865c3d98d34a11ad063a514ac990da 100644 (file)
--- a/faxd/Class1.h
+++ b/faxd/Class1.h
@@ -77,6 +77,7 @@ protected:
     bool       recvdDCN;               // received DCN frame
     bool       messageReceived;        // expect/don't expect message carrier
     bool       repeatPhaseB;           // return to beginning of Phase B before next page
+    bool       silenceHeard;           // indicates whether the last command was +FRS
     u_int      lastPPM;                // last PPM during receive
     bool       sendCFR;                // received TCF was not confirmed
     u_short    ecmBitPos;              // bit position to populate on ecmByte
@@ -155,6 +156,8 @@ protected:
     };
     virtual ATResponse atResponse(char* buf, long ms = 30*1000);
     virtual bool waitFor(ATResponse wanted, long ms = 30*1000);
+    virtual bool atCmd(const fxStr& cmd, ATResponse = AT_OK, long ms = 30*1000);
+    bool       switchingPause(fxStr& emsg);
     void       encodeTSI(fxStr& binary, const fxStr& ascii);
     void       encodeNSF(fxStr& binary, const fxStr& ascii);
     const fxStr& decodeTSI(fxStr& ascii, const HDLCFrame& binary);

diff --git a/faxd/Class1Recv.c++ b/faxd/Class1Recv.c++
index 7f4f64fb937057bc5336b27320d56ad75831304e..6e80efc818940215e42124d908fa042fa5c83996 100644 (file)
--- a/faxd/Class1Recv.c++
+++ b/faxd/Class1Recv.c++
@@ -266,8 +266,7 @@ Class1Modem::recvIdentification(
             * The best way to do that is to make sure that there is
             * silence on the line, and  we do that with Class1SwitchingCmd.
             */
-           if (!atCmd(conf.class1SwitchingCmd, AT_OK)) {
-               emsg = "Failure to receive silence.";
+           if (!switchingPause(emsg)) {
                return (false);
            }
        }
@@ -432,7 +431,8 @@ Class1Modem::recvTraining()
      * Send training response; we follow the spec
      * by delaying 75ms before switching carriers.
      */
-    if (!atCmd(conf.class1SwitchingCmd, AT_OK)) return (false);
+    fxStr emsg;
+    if (!switchingPause(emsg)) return (false);
     if (ok) {
        /*
         * Send CFR later so that we can cancel
@@ -739,9 +739,7 @@ Class1Modem::recvPage(TIFF* tif, u_int& ppm, fxStr& emsg, const fxStr& id)
                     * that we just received, write it to disk.
                     */
                    if (messageReceived) {
-                       if (!useV34 && emsg == "" && !atCmd(conf.class1SwitchingCmd, AT_OK)) {
-                           emsg = "Failure to receive silence.";
-                       }
+                       if (!useV34 && emsg == "") (void) switchingPause(emsg);
                        /*
                         * On servers where disk access may be bottlenecked or stressed,
                         * the TIFFWriteDirectory call can lag.  The strategy, then, is
@@ -794,7 +792,7 @@ Class1Modem::recvPage(TIFF* tif, u_int& ppm, fxStr& emsg, const fxStr& id)
                                                    } else if (params.ec != EC_DISABLE && rrframe.getFCF() != FCF_RR) {
                                                        protoTrace("Ignoring invalid response to RNR.");
                                                    }
-                                                   if (!useV34) atCmd(conf.class1SwitchingCmd, AT_OK);
+                                                   if (!useV34) (void) switchingPause(emsg);
                                                }
                                            } while (!gotEOT && !recvdDCN && !gotresponse && ++rnrcnt < 2 && Sys::now()-rrstart < 60);
                                            if (!gotresponse) emsg = "No response to RNR repeated 3 times.";
@@ -864,8 +862,7 @@ Class1Modem::recvPage(TIFF* tif, u_int& ppm, fxStr& emsg, const fxStr& id)
                     * to implement than using +FRH and more reliable than
                     * using +FTS
                     */
-                   if (!atCmd(conf.class1SwitchingCmd, AT_OK)) {
-                       emsg = "Failure to receive silence.";
+                   if (!switchingPause(emsg)) {
                        return (false);
                    }
                    u_int rtnfcf = conf.badPageHandling == FaxModem::BADPAGE_DCN ? FCF_DCN : FCF_RTN;
@@ -1067,8 +1064,7 @@ Class1Modem::recvPageECMData(TIFF* tif, const Class2Params& params, fxStr& emsg)
                                            case FCF_PRI_EOM:
                                            case FCF_PRI_MPS:
                                            case FCF_PRI_EOP:
-                                               if (!useV34 && !atCmd(conf.class1SwitchingCmd, AT_OK)) {
-                                                   emsg = "Failure to receive silence.";
+                                               if (!useV34 && !switchingPause(emsg)) {
                                                    abortPageECMRecv(tif, params, block, fcount, seq, pagedataseen);
                                                    return (false);
                                                }
@@ -1105,8 +1101,7 @@ Class1Modem::recvPageECMData(TIFF* tif, const Class2Params& params, fxStr& emsg)
                                                    if (signalRcvd) lastblock = true;
                                                    sendERR = true;
                                                } else {
-                                                   if (!useV34 && !atCmd(conf.class1SwitchingCmd, AT_OK)) {
-                                                       emsg = "Failure to receive silence.";
+                                                   if (!useV34 && !switchingPause(emsg)) {
                                                        abortPageECMRecv(tif, params, block, fcount, seq, pagedataseen);
                                                        return (false);
                                                    }
@@ -1134,8 +1129,7 @@ Class1Modem::recvPageECMData(TIFF* tif, const Class2Params& params, fxStr& emsg)
                                        dcs = rtncframe[3] | (rtncframe[4]<<8);
                                        curcap = findSRCapability(dcs&DCS_SIGRATE, recvCaps);
                                        // requisite pause before sending response (CTR)
-                                       if (!atCmd(conf.class1SwitchingCmd, AT_OK)) {
-                                           emsg = "Failure to receive silence.";
+                                       if (!switchingPause(emsg)) {
                                            abortPageECMRecv(tif, params, block, fcount, seq, pagedataseen);
                                            return (false);
                                        }
@@ -1147,8 +1141,7 @@ Class1Modem::recvPageECMData(TIFF* tif, const Class2Params& params, fxStr& emsg)
                                    }
                                case FCF_CRP:
                                    // command repeat... just repeat whatever we last sent
-                                   if (!useV34 && !atCmd(conf.class1SwitchingCmd, AT_OK)) {
-                                       emsg = "Failure to receive silence.";
+                                   if (!useV34 && !switchingPause(emsg)) {
                                        abortPageECMRecv(tif, params, block, fcount, seq, pagedataseen);
                                        return (false);
                                    }
@@ -1384,8 +1377,7 @@ Class1Modem::recvPageECMData(TIFF* tif, const Class2Params& params, fxStr& emsg)
                                }
 
                                // requisite pause before sending response (PPR/MCF)
-                               if (!blockgood && !useV34 && !atCmd(conf.class1SwitchingCmd, AT_OK)) {
-                                   emsg = "Failure to receive silence.";
+                               if (!blockgood && !useV34 && !switchingPause(emsg)) {
                                    abortPageECMRecv(tif, params, block, fcount, seq, pagedataseen);
                                    return (false);
                                }
@@ -1424,8 +1416,7 @@ Class1Modem::recvPageECMData(TIFF* tif, const Class2Params& params, fxStr& emsg)
                                        lastResponse = AT_NOTHING;
                                        while (rtnframe.getFCF() == FCF_PPS && lastResponse != AT_NOCARRIER && recvFrameCount < 5 && gotrtnframe) {
                                            // we sent PPR, but got PPS again...
-                                           if (!useV34 && !atCmd(conf.class1SwitchingCmd, AT_OK)) {
-                                               emsg = "Failure to receive silence.";
+                                           if (!useV34 && !switchingPause(emsg)) {
                                                abortPageECMRecv(tif, params, block, fcount, seq, pagedataseen);
                                                return (false);
                                            }
@@ -1447,8 +1438,7 @@ Class1Modem::recvPageECMData(TIFF* tif, const Class2Params& params, fxStr& emsg)
                                                dcs = rtnframe[3] | (rtnframe[4]<<8);
                                                curcap = findSRCapability(dcs&DCS_SIGRATE, recvCaps);
                                                // requisite pause before sending response (CTR)
-                                               if (!atCmd(conf.class1SwitchingCmd, AT_OK)) {
-                                                   emsg = "Failure to receive silence.";
+                                               if (!switchingPause(emsg)) {
                                                    abortPageECMRecv(tif, params, block, fcount, seq, pagedataseen);
                                                    return (false);
                                                }
@@ -1604,7 +1594,7 @@ Class1Modem::recvPageECMData(TIFF* tif, const Class2Params& params, fxStr& emsg)
                            bool gotresponse = true;
                            u_short rnrcnt = 0;
                            do {
-                               if (!useV34) atCmd(conf.class1SwitchingCmd, AT_OK);
+                               if (!useV34) (void) switchingPause(emsg);
                                if (emsg != "") break;
                                (void) transmitFrame(FCF_RNR|FCF_RCVR);
                                traceFCF("RECV send", FCF_RNR);
@@ -1642,7 +1632,7 @@ Class1Modem::recvPageECMData(TIFF* tif, const Class2Params& params, fxStr& emsg)
 
        if (!lastblock) {
            // confirm block received as good
-           if (!useV34) atCmd(conf.class1SwitchingCmd, AT_OK);
+           if (!useV34) (void) switchingPause(emsg);
            (void) transmitFrame((sendERR ? FCF_ERR : FCF_MCF)|FCF_RCVR);
            traceFCF("RECV send", sendERR ? FCF_ERR : FCF_MCF);
        }
@@ -1702,7 +1692,7 @@ Class1Modem::recvPageData(TIFF* tif, fxStr& emsg)
  * Complete a receive session.
  */
 bool
-Class1Modem::recvEnd(fxStr&)
+Class1Modem::recvEnd(fxStr& emsg)
 {
     if (!recvdDCN && !gotEOT) {
        u_int t1 = howmany(conf.t1Timer, 1000); // T1 timer in seconds
@@ -1718,7 +1708,7 @@ Class1Modem::recvEnd(fxStr&)
                case FCF_PPS:
                case FCF_EOP:
                case FCF_CRP:
-                   if (!useV34) atCmd(conf.class1SwitchingCmd, AT_OK);
+                   if (!useV34) (void) switchingPause(emsg);
                    (void) transmitFrame(FCF_MCF|FCF_RCVR);
                    traceFCF("RECV send", FCF_MCF);
                    break;
@@ -1726,7 +1716,7 @@ Class1Modem::recvEnd(fxStr&)
                    recvdDCN = true;
                    break;
                default:
-                   if (!useV34) atCmd(conf.class1SwitchingCmd, AT_OK);
+                   if (!useV34) (void) switchingPause(emsg);
                    transmitFrame(FCF_DCN|FCF_RCVR);
                    recvdDCN = true;
                    break;

diff --git a/faxd/Class1Send.c++ b/faxd/Class1Send.c++
index b937e6222ca112ae73c35536e28e3ca39535cf69..49dd220942618ffac7b756470b3d8148d5a431ce 100644 (file)
--- a/faxd/Class1Send.c++
+++ b/faxd/Class1Send.c++
@@ -323,8 +323,7 @@ Class1Modem::sendPhaseB(TIFF* tif, Class2Params& next, FaxMachineInfo& info,
             * "before sending any signals using V.27 ter/V.29/V.33/V.17 
             * modulation system"
             */
-           if (!atCmd(conf.class1SwitchingCmd, AT_OK)) {
-               protoTrace("Failure to receive silence.");
+           if (!switchingPause(emsg)) {
                return (send_failed);
            }
        }
@@ -421,7 +420,7 @@ Class1Modem::sendPhaseB(TIFF* tif, Class2Params& next, FaxMachineInfo& info,
                    pph.remove(0,2+5+1);// discard page-chop+handling info
                else
                    pph.remove(0,3);    // discard page-handling info
-               if (params.ec == EC_DISABLE) atCmd(conf.class1SwitchingCmd, AT_OK);
+               if (params.ec == EC_DISABLE) (void) switchingPause(emsg);
                ntrys = 0;
                if (morePages) {        // meaning, more pages in this file, but there may be other files
                    if (!TIFFReadDirectory(tif)) {
@@ -515,9 +514,7 @@ Class1Modem::sendPhaseB(TIFF* tif, Class2Params& next, FaxMachineInfo& info,
                protoTrace(emsg);
                return (send_failed);
            case FCF_CRP:
-               if (!useV34 && !atCmd(conf.class1SwitchingCmd, AT_OK)) {
-                   emsg = "Failure to receive silence.";
-                   protoTrace(emsg);
+               if (!useV34 && !switchingPause(emsg)) {
                    return (send_retry);
                }
                break;
@@ -573,8 +570,8 @@ Class1Modem::sendPrologue(FaxParams& dcs_caps, const fxStr& tsi)
     bool frameSent;
     if (useV34) frameSent = true;
     else {
-       if (!atCmd(conf.class1SwitchingCmd, AT_OK)) {
-           protoTrace("Failure to receive silence.");
+       fxStr emsg;
+       if (!switchingPause(emsg)) {
            return (false);
        }
        frameSent = (atCmd(thCmd, AT_NOTHING) && atResponse(rbuf, 7550) == AT_CONNECT);
@@ -863,8 +860,7 @@ Class1Modem::sendTraining(Class2Params& params, int tries, fxStr& emsg)
                 * The best way to do that is to make sure that there is
                 * silence on the line, and  we do that with Class1SwitchingCmd.
                 */
-               if (!atCmd(conf.class1SwitchingCmd, AT_OK)) {
-                   emsg = "Failure to receive silence.";
+               if (!switchingPause(emsg)) {
                    return (false);
                }
            }
@@ -1093,8 +1089,7 @@ Class1Modem::blockFrame(const u_char* bitrev, bool lastframe, u_int ppmcmd, fxSt
                setXONXOFF(FLOW_XONXOFF, FLOW_NONE, ACT_FLUSH);
            if (!useV34) {
                // T.30 5.3.2.4 (03/93) gives this to be a 75ms minimum
-               if (!atCmd(conf.class1SwitchingCmd, AT_OK)) {
-                   protoTrace("Failure to receive silence.");
+               if (!switchingPause(emsg)) {
                    return (false);
                }
                /*
@@ -1226,18 +1221,14 @@ Class1Modem::blockFrame(const u_char* bitrev, bool lastframe, u_int ppmcmd, fxSt
                        gotppr = false;
                        crpcnt++;
                        ppscnt = 0;
-                       if (!useV34 && !atCmd(conf.class1SwitchingCmd, AT_OK)) {
-                           emsg = "Failure to receive silence.";
-                           protoTrace(emsg);
+                       if (!useV34 && !switchingPause(emsg)) {
                            return (false);
                        }
                    }
                }
            } while (!gotppr && (++ppscnt < 3) && (crpcnt < 3) && !(useV34 && gotEOT));
            if (gotppr) {
-               if (!useV34 && !atCmd(conf.class1SwitchingCmd, AT_OK)) {
-                   emsg = "Failure to receive silence.";
-                   protoTrace(emsg);
+               if (!useV34 && !switchingPause(emsg)) {
                    return (false);
                }
                if (pprframe.getFCF() == FCF_RNR) {
@@ -1268,9 +1259,7 @@ Class1Modem::blockFrame(const u_char* bitrev, bool lastframe, u_int ppmcmd, fxSt
                                    gotmsg = false;
                                    crpcnt++;
                                    rrcnt = 0;
-                                   if (!useV34 && !atCmd(conf.class1SwitchingCmd, AT_OK)) {
-                                       emsg = "Failure to receive silence.";
-                                       protoTrace(emsg);
+                                   if (!useV34 && !switchingPause(emsg)) {
                                        return (false);
                                    }
                                }
@@ -1287,9 +1276,7 @@ Class1Modem::blockFrame(const u_char* bitrev, bool lastframe, u_int ppmcmd, fxSt
                                gotppr = true;
                                break;
                            case FCF_RNR:
-                               if (!useV34 && !atCmd(conf.class1SwitchingCmd, AT_OK)) {
-                                   emsg = "Failure to receive silence.";
-                                   protoTrace(emsg);
+                               if (!useV34 && !switchingPause(emsg)) {
                                    return (false);
                                }
                                break;
@@ -1397,9 +1384,7 @@ Class1Modem::blockFrame(const u_char* bitrev, bool lastframe, u_int ppmcmd, fxSt
                                            gotctr = false;
                                            crpcnt++;
                                            ctccnt = 0;
-                                           if (!atCmd(conf.class1SwitchingCmd, AT_OK)) {
-                                               emsg = "Failure to receive silence.";
-                                               protoTrace(emsg);
+                                           if (!switchingPause(emsg)) {
                                                return (false);
                                            }
                                        }
@@ -1450,9 +1435,7 @@ Class1Modem::blockFrame(const u_char* bitrev, bool lastframe, u_int ppmcmd, fxSt
                                            goterr = false;
                                            crpcnt++;
                                            eorcnt = 0;
-                                           if (!useV34 && !atCmd(conf.class1SwitchingCmd, AT_OK)) {
-                                               emsg = "Failure to receive silence.";
-                                               protoTrace(emsg);
+                                           if (!useV34 && !switchingPause(emsg)) {
                                                return (false);
                                            }
                                        }
@@ -1492,9 +1475,7 @@ Class1Modem::blockFrame(const u_char* bitrev, bool lastframe, u_int ppmcmd, fxSt
                                                    gotmsg = false;
                                                    crpcnt++;
                                                    rrcnt = 0;
-                                                   if (!useV34 && !atCmd(conf.class1SwitchingCmd, AT_OK)) {
-                                                       emsg = "Failure to receive silence.";
-                                                       protoTrace(emsg);
+                                                   if (!useV34 && !switchingPause(emsg)) {
                                                        return (false);
                                                    }
                                                }
@@ -1510,9 +1491,7 @@ Class1Modem::blockFrame(const u_char* bitrev, bool lastframe, u_int ppmcmd, fxSt
                                                goterr = true;
                                                break;
                                            case FCF_RNR:
-                                               if (!useV34 && !atCmd(conf.class1SwitchingCmd, AT_OK)) {
-                                                   emsg = "Failure to receive silence.";
-                                                   protoTrace(emsg);
+                                               if (!useV34 && !switchingPause(emsg)) {
                                                    return (false);
                                                }
                                                break;