Railcom tidy (AVR timer ok)

Author: Asbelos

DCCEXParser.cpp

@@ -1306,24 +1306,19 @@ bool DCCEXParser::parseC(Print *stream, int16_t params, int16_t p[]) {
         {   // <C RAILCOM ON|OFF|DEBUG >
             if (params<2) return false;
             bool on=false;
-            bool debug=false;
             switch (p[1]) {
                 case "ON"_hk:
                 case 1:
                     on=true;
                     break;
-                case "DEBUG"_hk:
-                    on=true;
-                    debug=true;
-                    break;
                 case "OFF"_hk:
                 case 0:
                      break;
                 default:
                  return false;
             }              
         DIAG(F("Railcom %S")
-            ,DCCWaveform::setRailcom(on,debug)?F("ON"):F("OFF"));
+            ,DCCWaveform::setRailcom(on)?F("ON"):F("OFF"));
         return true;     
         }
 #endif

DCCTimerAVR.cpp

@@ -66,10 +66,14 @@ void DCCTimer::startRailcomTimer(byte brakePin) {
   // diagnostic digitalWrite(4,HIGH);   
 
   /* The Railcom timer is started in such a way that it 
-     - First triggers 58+29 uS after the previous TIMER1 tick. 
+     - First triggers a calculated time after the previous TIMER1 tick. 
        This provides an accurate offset (in High Accuracy mode)
-       for the start of the Railcom cutout.
-     - Sets the Railcom pin high at first tick and subsequent ticks 
+       for the start of the Railcom cutout. The delayBeforeCutout value 
+       reflects that this call is made at the start of the end-packet bit
+       thus the delay is nominally 58+58+Tcs BUT 
+       in HA mode, the dcc pin is flipped at the NEXT 
+       interrupt so runs 1 tick behind the code. Thus an extra 58uS is needed. 
+     - Timer2 Sets the Railcom pin high at first tick and subsequent ticks 
        until its reset to setting pin 9 low at next tick.
         
      - Cycles at cutoutDuration so the second tick is the 
@@ -80,29 +84,32 @@ void DCCTimer::startRailcomTimer(byte brakePin) {
        (there will be 7 DCC timer1 ticks in which to do this.)
     
     */
-  const int Tcs=28;    // (26+32)/2 would be half way spec Desired time from idealised setup call (at previous DCC timer interrupt) to the cutout but choose even
-  const int cutoutDuration = 450;    // As chosen by most
-  const byte delayBeforeCutout=58+58+Tcs; // Expected time from idealised setup call (at previous DCC timer interrupt) to the cutout. This is the time we need to wait before we can set pin 9 high. We will then set pin 9 low at the next tick which is cutoutDuration later. This value should be reduced to reflect the Timer1 value measuring the time since the previous hardware interrupt.
+  const int Tcs=28;    // NMRA spec is 26..32
+  const int cutoutDuration_uS = 450;    // As chosen by most
+  const uint16_t delayBeforeCutout_uS=58+58+58+Tcs;
+  const uint16_t timer1_ticks_per_uS = 8;
+  const uint16_t timer2_uS_per_tick = 2;
 
   // Set Timer2 to CTC mode with set on compare match
   TCCR2A = (1 << WGM21) | (1 << COM2B0) | (1 << COM2B1);
   // Prescaler of 32
   TCCR2B =  (1 << CS21) | (1 << CS20); 
-  OCR2A = cutoutDuration/2; // Compare match value for cutout duration in timer2 ticks (2uSec)
+  OCR2A = cutoutDuration_uS/timer2_uS_per_tick; // Compare match value for cutout duration in timer2 ticks (2uSec)
   // Enable Timer2 output on pin 9 (OC2B)
   DDRB |= (1 << DDB1);
 
   // timeSlip is the expired time since the DCC timer interrupt that triggered this call.
   // This allows us to cope with any delays between the interrupt and this code executing.  
 
-  // tcnt1 = prescaler 64,  tcnt2 prescaler=32
+  
   noInterrupts();
-  uint16_t timeSlip=(TCNT1/64)*32;
-  // Adjust the Timer2 counter so it first triggers at the right place in the waveform.
-  // TCNT1 moves 8 ticks per uSec
-  // TCNT2 uses 2 uSec for each tick
-  // TCNT1 is 8*2 = 16 times faster than TCNT2
-  TCNT2=(cutoutDuration+TCNT1/8-delayBeforeCutout)/2;
+  // Time already used since DCC interrupt 
+  uint16_t timeSlip_uS=TCNT1/timer1_ticks_per_uS;
+  
+  // Calculate the time left before the cutout is to be triggered
+  uint16_t timeToCutout_uS=cutoutDuration_uS+timeSlip_uS-delayBeforeCutout_uS;
+  // Adjust clock2 to trigger on time
+  TCNT2=timeToCutout_uS/timer2_uS_per_tick;
   interrupts();
 }
 

DCCWaveform.cpp

@@ -29,6 +29,7 @@
 #include "DCCTimer.h"
 #include "DCCACK.h"
 #include "DIAG.h"
+#include "Railcom.h"
 
 bool DCCWaveform::cutoutNextTime=false;
 DCCWaveform  DCCWaveform::mainTrack(PREAMBLE_BITS_MAIN, true);
@@ -77,8 +78,7 @@ void DCCWaveform::interruptHandler() {
   #if defined(HAS_ENOUGH_MEMORY)
   if (cutoutNextTime) {
     cutoutNextTime=false;
-    railcomSampleWindow=false; // about to cutout, stop reading railcom data.
-    railcomCutoutCounter++;
+    Railcom::incCutout();
     DCCTimer::startRailcomTimer(9);
   }
   #endif
@@ -126,21 +126,13 @@ DCCWaveform::DCCWaveform( byte preambleBits, bool isMain) {
 
 bool DCCWaveform::railcomPossible=false;     // High accuracy only    
 volatile bool DCCWaveform::railcomActive=false;     // switched on by user
-volatile bool DCCWaveform::railcomDebug=false;     // switched on by user
-volatile bool DCCWaveform::railcomSampleWindow=false; // true during packet transmit
-volatile byte DCCWaveform::railcomCutoutCounter=0;    // cyclic cutout
-volatile byte DCCWaveform::railcomLastAddressHigh=0;
-volatile byte DCCWaveform::railcomLastAddressLow=0;
-
-bool DCCWaveform::setRailcom(bool on, bool debug) {
+ 
+bool DCCWaveform::setRailcom(bool on) {
   if (on && railcomPossible) {
     railcomActive=true;
-    railcomDebug=debug;
   }
   else {
     railcomActive=false;
-    railcomDebug=false;
-    railcomSampleWindow=false;
   } 
   return railcomActive;
 }
@@ -174,9 +166,7 @@ void DCCWaveform::interrupt2() {
         // if preamble length is 16 then this evaluates to 5
         // Remember address bytes of last sent packet so that Railcom can
         // work out where the channel2 data came from.
-        railcomLastAddressHigh=transmitPacket[0];
-        railcomLastAddressLow =transmitPacket[1];
-        railcomSampleWindow=true;
+        Railcom::setLoco(transmitPacket[0],transmitPacket[1]);
       } else if (remainingPreambles==(requiredPreambles-3)) {
         // cutout can be ended when read
         // see above for requiredPreambles
@@ -258,11 +248,7 @@ void DCCWaveform::promotePendingPacket() {
 
       // nothing to do, just send idles or resets
       // Fortunately reset and idle packets are the same length
-      // Note: If railcomDebug is on, then we send resets to the main
-      //       track instead of idles. This means that all data will be zeros
-      //       and only the presets will be ones, making it much
-      //       easier to read on a logic analyser.
-      memcpy( transmitPacket, (isMainTrack && (!railcomDebug)) ? idlePacket : resetPacket, sizeof(idlePacket));
+      memcpy( transmitPacket, isMainTrack ? idlePacket : resetPacket, sizeof(idlePacket));
       transmitLength = sizeof(idlePacket);
       transmitRepeats = 0;
       if (getResets() < 250) sentResetsSincePacket++; // only place to increment (private!)

DCCWaveform.h

@@ -82,30 +82,18 @@ class DCCWaveform {
     void schedulePacket(const byte buffer[], byte byteCount, byte repeats);
     bool isReminderWindowOpen();
     void promotePendingPacket();
-    static bool setRailcom(bool on, bool debug);
+    static bool setRailcom(bool on);
     inline static bool isRailcom() {
       return railcomActive;
     };
-    inline static byte getRailcomCutoutCounter() {
-      return railcomCutoutCounter;
-    };
-    inline static bool isRailcomSampleWindow() {
-      return railcomSampleWindow;
-    };
     inline static bool isRailcomPossible() {
       return railcomPossible;
     };
     inline static void setRailcomPossible(bool yes) {
       railcomPossible=yes;
-      if (!yes) setRailcom(false,false);
+      if (!yes) setRailcom(false);
     };
-    inline static uint16_t getRailcomLastLocoAddress() {
-      // first 2 bits 00=short loco, 11=long loco , 01/10 = accessory
-      byte addressType=railcomLastAddressHigh & 0xC0;
-      if (addressType==0xC0) return ((railcomLastAddressHigh & 0x3f)<<8) | railcomLastAddressLow;
-      if (addressType==0x00) return railcomLastAddressHigh & 0x3F;
-      return 0; 
-    }
+    
 
   private:
 #ifndef ARDUINO_ARCH_ESP32
@@ -133,10 +121,6 @@ class DCCWaveform {
     byte pendingRepeats;
     static bool railcomPossible; // High accuracy mode only
     static volatile bool railcomActive;     // switched on by user
-    static volatile bool railcomDebug;     // switched on by user
-    static volatile bool railcomSampleWindow; // when safe to sample
-    static volatile byte railcomCutoutCounter; // incremented for each cutout
-    static volatile byte railcomLastAddressHigh,railcomLastAddressLow;
     static bool cutoutNextTime;   // railcom
 #ifdef ARDUINO_ARCH_ESP32
   static RMTChannel *rmtMainChannel;

DCCWaveformRMT.cpp

@@ -34,11 +34,6 @@ RMTChannel *DCCWaveform::rmtProgChannel = NULL;
 
 bool DCCWaveform::railcomPossible=false;     // High accuracy only    
 volatile bool DCCWaveform::railcomActive=false;     // switched on by user
-volatile bool DCCWaveform::railcomDebug=false;     // switched on by user
-volatile bool DCCWaveform::railcomSampleWindow=false; // true during packet transmit
-volatile byte DCCWaveform::railcomCutoutCounter=0;    // cyclic cutout
-volatile byte DCCWaveform::railcomLastAddressHigh=0;
-volatile byte DCCWaveform::railcomLastAddressLow=0;
 
 DCCWaveform::DCCWaveform(byte preambleBits, bool isMain) {
   isMainTrack = isMain;
@@ -112,7 +107,7 @@ void IRAM_ATTR DCCWaveform::loop() {
   DCCACK::checkAck(progTrack.getResets());
 }
 
-bool DCCWaveform::setRailcom(bool on, bool debug) {
+bool DCCWaveform::setRailcom(bool on) {
   // TODO... ESP32 railcom waveform
   return false;
 }

IO_I2CRailcom.cpp

@@ -83,14 +83,15 @@ void I2CRailcom::create(VPIN firstVpin, int nPins, I2CAddress i2cAddress) {
 
     // have we read this cutout already?
     // basically we only poll once per packet when railcom cutout is working
-    auto cut=DCCWaveform::getRailcomCutoutCounter();
+    auto cut=Railcom::getCutout();
     if (cutoutCounter==cut) return; 
     cutoutCounter=cut; 
     Railcom::loop(); // in case a csv read has timed out 
 
     // Obtain data length from the collector
     byte inbuf[1];
-    byte queryLength[]={'?'};
+    uint16_t loco=Railcom::getLoco();
+    byte queryLength[]={'?',(byte)(loco >>8) , (byte)(loco & 0xff)};
     auto state=I2CManager.read(_I2CAddress, inbuf, 1,queryLength,sizeof(queryLength)); 
     if (state) {
       DIAG(F("RC ? state=%d"),state);

Railcom.cpp

@@ -31,6 +31,8 @@
 #include "EXRAIL2.h"
 
 uint16_t Railcom::expectLoco=0;
+uint16_t Railcom::nextLoco=0;
+
 uint16_t Railcom::expectCV=0;
 unsigned long Railcom::expectWait=0;
 ACK_CALLBACK Railcom::expectCallback=0;
@@ -42,6 +44,17 @@ enum ResponseType: byte {
         CV_VALUE_LIST=0xC0, // list of cv values read from a POM, cv id and 4 values follow
     };
 
+void Railcom::setLoco(byte packet0, byte packet1) {
+    // first 2 bits 00=short loco, 11=long loco , 01/10 = accessory
+      byte addressType=packet0 & 0xC0;
+      if (addressType==0xC0) nextLoco=((packet0 & 0x3f)<<8) | packet1;
+      else if (addressType==0x00) nextLoco=packet0 & 0x3F;
+      else nextLoco=0; 
+}
+
+uint16_t Railcom::getLoco() {
+    return nextLoco; 
+}
 
 // anticipate is used when waiting for a CV read from a railcom loco
 void Railcom::anticipate(uint16_t loco, uint16_t cv, ACK_CALLBACK callback) { 
@@ -75,10 +88,7 @@ void Railcom::process(int16_t firstVpin,byte * buffer, byte length) {
             break;
        case CV_VALUE: { // csv value from POM read
             byte value=buffer[i+1];
-            if (expectCV && DCCWaveform::getRailcomLastLocoAddress()==expectLoco) {
-                if (expectCallback) expectCallback(value);
-                expectCV=0;
-            }
+            if (expectCallback) expectCallback(value);
             i+=2;
         }
          break;
@@ -97,3 +107,8 @@ void Railcom::loop() {
                 expectCV=0;
     }
 }
+
+byte Railcom::cutoutCounter=0;
+void Railcom::incCutout() {cutoutCounter++;};
+byte Railcom::getCutout() {return cutoutCounter;};
+ 

Railcom.h

@@ -28,12 +28,18 @@ class Railcom {
   public:
     static void anticipate(uint16_t loco, uint16_t cv, ACK_CALLBACK callback);
     static void process(int16_t firstVpin,byte * buffer, byte length );
+    static void setLoco(byte packet0,byte packet1);
+    static uint16_t getLoco();
     static void loop();
+    static void incCutout();
+    static byte getCutout();
   private:
     static const unsigned long POM_READ_TIMEOUT=500; // as per spec
-    static uint16_t expectCV,expectLoco;
+    static uint16_t expectCV,expectLoco, nextLoco;
     static unsigned long expectWait;
     static ACK_CALLBACK expectCallback;
+    static byte cutoutCounter;    // cyclic cutout
+ 
     static const byte MAX_WAIT_FOR_GLITCH=20; // number of dead or empty packets before assuming loco=0 
 };