Fix auto rejoin after prog cmd (needs version n umber!) (#148)

* ack down flank double check

* ack gap properly reported

* zero gap count; tolerate 2 samples per gap

* Fix auto rejoin after prog cmd

Moved more setup out of the BASELINE loop so its not checked every time while waiting for reset counter.
Added REJOIN diag..

* Stable 100mS and off 30mS

* Init powerOff after flag.

Co-authored-by: Harald Barth <haba@kth.se>

DCC.cpp

@@ -691,9 +691,31 @@ byte   DCC::ackManagerBitNum;
 bool   DCC::ackReceived;
 bool   DCC::ackManagerRejoin;
 
+CALLBACK_STATE DCC::callbackState=READY;
+
 ACK_CALLBACK DCC::ackManagerCallback;
 
 void  DCC::ackManagerSetup(int cv, byte byteValueOrBitnum, ackOp const program[], ACK_CALLBACK callback) {
+  if (!DCCWaveform::progTrack.canMeasureCurrent()) {
+    callback(-2);
+    return;
+  }
+
+  ackManagerRejoin=DCCWaveform::progTrackSyncMain;     
+  if (ackManagerRejoin ) {
+        // Change from JOIN must zero resets packet.
+        setProgTrackSyncMain(false);
+        DCCWaveform::progTrack.sentResetsSincePacket = 0;      
+      }
+      
+   DCCWaveform::progTrack.autoPowerOff=false;           
+   if (DCCWaveform::progTrack.getPowerMode() == POWERMODE::OFF) {
+        DCCWaveform::progTrack.autoPowerOff=true;  // power off afterwards           
+        if (Diag::ACK) DIAG(F("Auto Prog power on"));
+        DCCWaveform::progTrack.setPowerMode(POWERMODE::ON);
+        DCCWaveform::progTrack.sentResetsSincePacket = 0;      
+    }
+
   ackManagerCv = cv;
   ackManagerProg = program;
   ackManagerByte = byteValueOrBitnum;
@@ -703,8 +725,7 @@ void  DCC::ackManagerSetup(int cv, byte byteValueOrBitnum, ackOp const program[]
 
 void  DCC::ackManagerSetup(int wordval, ackOp const program[], ACK_CALLBACK callback) {
   ackManagerWord=wordval;
-  ackManagerProg = program;
+  ackManagerSetup(0, 0, program, callback);
-  ackManagerCallback = callback;
   }
 
 const byte RESET_MIN=8;  // tuning of reset counter before sending message
@@ -723,21 +744,9 @@ void DCC::ackManagerLoop() {
     // (typically waiting for a reset counter or ACK waiting, or when all finished.)
     switch (opcode) {
       case BASELINE:
-      ackManagerRejoin=DCCWaveform::progTrackSyncMain;
+      	  if (checkResets(DCCWaveform::progTrack.autoPowerOff || ackManagerRejoin  ? 20 : 3)) return;
-      if (!DCCWaveform::progTrack.canMeasureCurrent()) {
-        callback(-2);
-        return;
-      }
-      setProgTrackSyncMain(false);
-	  if (DCCWaveform::progTrack.getPowerMode() == POWERMODE::OFF) {
-        if (Diag::ACK) DIAG(F("Auto Prog power on"));
-        DCCWaveform::progTrack.setPowerMode(POWERMODE::ON);
-        DCCWaveform::progTrack.sentResetsSincePacket = 0;
-	      DCCWaveform::progTrack.autoPowerOff=true;
-	      return;
-	  }
-	  if (checkResets(DCCWaveform::progTrack.autoPowerOff ? 20 : 3)) return;
           DCCWaveform::progTrack.setAckBaseline();
+          callbackState=READY;
           break;   
       case W0:    // write 0 bit 
       case W1:    // write 1 bit 
@@ -748,6 +757,7 @@ void DCC::ackManagerLoop() {
               byte message[] = {cv1(BIT_MANIPULATE, ackManagerCv), cv2(ackManagerCv), instruction };
               DCCWaveform::progTrack.schedulePacket(message, sizeof(message), PROG_REPEATS);
               DCCWaveform::progTrack.setAckPending(); 
+             callbackState=AFTER_WRITE;
          }
             break; 
       
@@ -758,6 +768,7 @@ void DCC::ackManagerLoop() {
               byte message[] = {cv1(WRITE_BYTE, ackManagerCv), cv2(ackManagerCv), ackManagerByte};
               DCCWaveform::progTrack.schedulePacket(message, sizeof(message), PROG_REPEATS);
               DCCWaveform::progTrack.setAckPending(); 
+              callbackState=AFTER_WRITE;
             }
             break;
       
@@ -888,21 +899,61 @@ void DCC::ackManagerLoop() {
     ackManagerProg++;
   }
 }
+
 void DCC::callback(int value) {
-    ackManagerProg=NULL;  // no more steps to execute
+    static unsigned long callbackStart;
-    if (DCCWaveform::progTrack.autoPowerOff) {
+    // We are about to leave programming mode
-      if (Diag::ACK) DIAG(F("Auto Prog power off"));
+    // Rule 1: If we have written to a decoder we must maintain power for 100mS
-      DCCWaveform::progTrack.doAutoPowerOff();
+    // Rule 2: If we are re-joining the main track we must power off for 30mS
+
+    switch (callbackState) {    
+       case AFTER_WRITE:  // first attempt to callback after a write operation
+            callbackStart=millis();
+            callbackState=WAITING_100;
+            if (Diag::ACK) DIAG(F("Stable 100mS"));
+            break;
+            
+       case WAITING_100:  // waiting for 100mS
+            if (millis()-callbackStart < 100) break;
+            // stable after power maintained for 100mS
+
+            // If we are going to power off anyway, it doesnt matter
+            // but if we will keep the power on, we must off it for 30mS
+            if (DCCWaveform::progTrack.autoPowerOff) callbackState=READY;
+            else { // Need to cycle power off and on
+                DCCWaveform::progTrack.setPowerMode(POWERMODE::OFF); 
+                callbackStart=millis();
+                callbackState=WAITING_30;
+                if (Diag::ACK) DIAG(F("OFF 30mS"));
+            }
+            break;
+ 
+        case WAITING_30:  // waiting for 30mS with power off
+            if (millis()-callbackStart < 30) break;
+            //power has been off for 30mS
+            DCCWaveform::progTrack.setPowerMode(POWERMODE::ON); 
+            callbackState=READY;
+            break;
+     
+       case READY:  // ready after read, or write after power delay and off period.
+            // power off if we powered it on
+           if (DCCWaveform::progTrack.autoPowerOff) {
+              if (Diag::ACK) DIAG(F("Auto Prog power off"));
+              DCCWaveform::progTrack.doAutoPowerOff();
+           }
+          // Restore <1 JOIN> to state before BASELINE
+          if (ackManagerRejoin) {
+              setProgTrackSyncMain(true);
+              if (Diag::ACK) DIAG(F("Auto JOIN"));
+          }  
+    
+          ackManagerProg=NULL;  // no more steps to execute
+          if (Diag::ACK) DIAG(F("Callback(%d)"),value);
+          (ackManagerCallback)( value);
     }
-
-    // Restore <1 JOIN> to state before BASELINE
-    setProgTrackSyncMain(ackManagerRejoin);
-    
-    if (Diag::ACK) DIAG(F("Callback(%d)"),value);
-    (ackManagerCallback)( value);
 }
 
- void DCC::displayCabList(Print * stream) {
+void DCC::displayCabList(Print * stream) {
 
     int used=0;
     for (int reg = 0; reg < MAX_LOCOS; reg++) {

DCC.h

@@ -54,6 +54,14 @@ enum ackOp : byte
   SKIPTARGET = 0xFF // jump to target
 };
 
+enum   CALLBACK_STATE : byte {
+  AFTER_WRITE,  // Start callback sequence after something was written to the decoder  
+  WAITING_100,        // Waiting for 100mS of stable power 
+  WAITING_30,         // waiting to 30ms of power off gap. 
+  READY,              // Ready to complete callback  
+  }; 
+
+
 // Allocations with memory implications..!
 // Base system takes approx 900 bytes + 8 per loco. Turnouts, Sensors etc are dynamically created
 #ifdef ARDUINO_AVR_UNO
@@ -141,6 +149,7 @@ private:
   static bool ackReceived;
   static bool ackManagerRejoin;
   static ACK_CALLBACK ackManagerCallback;
+  static CALLBACK_STATE callbackState;
   static void ackManagerSetup(int cv, byte bitNumOrbyteValue, ackOp const program[], ACK_CALLBACK callback);
   static void ackManagerSetup(int wordval, ackOp const program[], ACK_CALLBACK callback);
   static void ackManagerLoop();

DCCWaveform.cpp

@@ -31,6 +31,9 @@ DCCWaveform  DCCWaveform::progTrack(PREAMBLE_BITS_PROG, false);
 bool DCCWaveform::progTrackSyncMain=false; 
 bool DCCWaveform::progTrackBoosted=false; 
 int  DCCWaveform::progTripValue=0;
+volatile uint8_t DCCWaveform::numAckGaps=0;
+volatile uint8_t DCCWaveform::numAckSamples=0;
+uint8_t DCCWaveform::trailingEdgeCounter=0;
 
 void DCCWaveform::begin(MotorDriver * mainDriver, MotorDriver * progDriver) {
   mainTrack.motorDriver=mainDriver;
@@ -290,13 +293,15 @@ void DCCWaveform::setAckPending() {
       ackPulseDuration=0;
       ackDetected=false;
       ackCheckStart=millis();
+      numAckSamples=0;
+      numAckGaps=0;
       ackPending=true;  // interrupt routines will now take note
 }
 
 byte DCCWaveform::getAck() {
       if (ackPending) return (2);  // still waiting
-      if (Diag::ACK) DIAG(F("%S after %dmS max=%d/%dmA pulse=%duS"),ackDetected?F("ACK"):F("NO-ACK"), ackCheckDuration, 
+      if (Diag::ACK) DIAG(F("%S after %dmS max=%d/%dmA pulse=%duS samples=%d gaps=%d"),ackDetected?F("ACK"):F("NO-ACK"), ackCheckDuration,
-           ackMaxCurrent,motorDriver->raw2mA(ackMaxCurrent), ackPulseDuration);
+			  ackMaxCurrent,motorDriver->raw2mA(ackMaxCurrent), ackPulseDuration, numAckSamples, numAckGaps);
       if (ackDetected) return (1); // Yes we had an ack
       return(0);  // pending set off but not detected means no ACK.   
 }
@@ -310,10 +315,15 @@ void DCCWaveform::checkAck() {
     }
       
     int current=motorDriver->getCurrentRaw();
+    numAckSamples++;
     if (current > ackMaxCurrent) ackMaxCurrent=current;
     // An ACK is a pulse lasting between minAckPulseDuration and maxAckPulseDuration uSecs (refer @haba)
         
     if (current>ackThreshold) {
+       if (trailingEdgeCounter > 0) {
+	 numAckGaps++;
+	 trailingEdgeCounter = 0;
+       }
        if (ackPulseStart==0) ackPulseStart=micros();    // leading edge of pulse detected
        return;
     }
@@ -321,8 +331,20 @@ void DCCWaveform::checkAck() {
     // not in pulse
     if (ackPulseStart==0) return; // keep waiting for leading edge 
     
+    // if we reach to this point, we have
     // detected trailing edge of pulse
-    ackPulseDuration=micros()-ackPulseStart;
+    if (trailingEdgeCounter == 0) {
+      ackPulseDuration=micros()-ackPulseStart;
+    }
+
+    // but we do not trust it yet and return (which will force another
+    // measurement) and first the third time around with low current
+    // the ack detection will be finalized. 
+    if (trailingEdgeCounter < 2) {
+      trailingEdgeCounter++;
+      return;
+    }
+    trailingEdgeCounter = 0;
 
     if (ackPulseDuration>=minAckPulseDuration && ackPulseDuration<=maxAckPulseDuration) {
         ackCheckDuration=millis()-ackCheckStart;

DCCWaveform.h

@@ -161,8 +161,11 @@ class DCCWaveform {
     unsigned int ackPulseDuration;  // micros
     unsigned long ackPulseStart; // micros
 
-    unsigned int minAckPulseDuration = 2000; // micros
+    unsigned int minAckPulseDuration = 4000; // micros
     unsigned int maxAckPulseDuration = 8500; // micros
 
+    volatile static uint8_t numAckGaps;
+    volatile static uint8_t numAckSamples;
+    static uint8_t trailingEdgeCounter;
 };
 #endif

version.h

@@ -3,10 +3,12 @@
 
 #include "StringFormatter.h"
 
-#define VERSION "3.0.13"
+#define VERSION "3.0.14"
+// 3.0.14 gap in ack tolerant fix,  prog track power management over join fix. 
 // 3.0.13 Functions>127 fix
 // 3.0.12 Fix HOSTNAME function for STA mode for WiFi
 // 3.0.11 ?
+// 3.0.11 28 speedstep support
 // 3.0.10 Teensy Support
 // 3.0.9 rearranges serial newlines for the benefit of JMRI.
 // 3.0.8 Includes <* *> wraps around DIAGs for the benefit of JMRI.