DCC++ EX Software Thread

David Cutting Apr 8, 2020

  1. Atani

    Atani TrainBoard Member

    1,460
    1,697
    36
    Share away :) I face similar issues on the ESP32 where it is even more complex as there are two cores to contend with and ISRs are bound to specific cores...
     
  2. UkBloke

    UkBloke TrainBoard Member

    42
    5
    3
    What follows is is quite a jump from where we are to where we could be. I have looked at the DCC++ and DCC-EX code and cringe at the big software killer complexity! So, if you are thinking of building the latest and greatest DCC for Arduino then there is a lot that could be done, but not by minor fiddling with the exiting code which would only increase the problems.

    Much credit is due to the original author and others who have maintained and enhanced the DCC++ code base over the years. However, there are still some fundamentals of complexity that mean it's not as easy to test, maintain, extend or use as it might be. Rather than pontificate about coding standards, I would like to point out some areas where I think a restructuring or indeed replacement could be highly beneficial making the code vastly SIMPLER, SHORTER, FASTER and use less memory (or handle more locos simultaneously). And in case you think I'm BSing, I have a prototype working!

    Layer Structure
    I believe the layers as they exist as of today (May 2020) do not sufficiently isolate the purpose they serve. Clearly some work has been done in that the incoming comms interface has been isolated from the command parsing. However from that point down it gets "silly". PacketRegister for example, needs to understand JMRI commands, DCC data stream generation AND transmission queue management (more on that later!)

    IMHO we need to think in terms of the following layers, each of which has very narrow field of responsibility.

    1. The COMMS layer receives commands and sends replies (as done now in the CommInterface* files.) and pass complete strings to the COMMAND layer. This layer is effectively a choice of three processes, each responsible for obtaining a command string from somewhere and passing it on to the COMMAND layer.

    2. The COMMAND layer should completely (and only!) understand the JMRI command syntax, parse each string and make API-like calls to the DCC layer. (e.g. setSpeed(loco,speed,direction), readCV(loco,cvnum) and so on. (This is effectively taking string parsing from PacketRegister into SerialCommand and making the packet register an effective DCC API. The Locoduino guys have done this already but the still suffer from the rest of the packet manager mess.)

    3. The DCC processing (most of the existing PacketRegister) should have no visibility of the JMRI command strings or responses. It should ONLY be responsible for turning commands and parameters into the bit-stream packets to be passed to the TRACK processing layer.

    4. The TRACK processing layer knows nothing about DCC other than the signal pattern of a 0 or 1 bit. What it does know about is timers, interrupts, pins, motor shields, current sensors etc etc. By thinking differently, its obvious that the main and prog tracks can be handled by two instances of a single class, rather than separately coded routines.
    The secret to making this work is to keep the joins between the layers as simple as possible, so that each layer can concentrate on one area of expertise and trust the other layers to do their job. In addition, there is a considerable simplicity improvement to be had (especially to the high speed interrupt routines) by moving the loco keep-alive speed repeats to the loop() function rather then having the TRACK signal generators know about loco register and have to search it within an interrupt.

    And by the way... my working proof-of-concept does not use pointers and has just 2 volatile variables which don't need protecting from interrupts.

    Interested????
     
    Atani likes this.
  3. UkBloke

    UkBloke TrainBoard Member

    42
    5
    3
    Also note.....
    My proof-of-concept only uses 1 interrupt and can (at least on my mega256 drive the motor shiled directly through its normal pins without the need for the jumper from pin 2 (as I recall).
    This interrupt is driven by the TimerOne library which removes all the messing with timer registers etc. The power and direction pins are driven using the DIO2 library which provides massive performance increases when reading/writing.

    I must assume these libs weren't around when the original DCC++ was written.
     
  4. FlightRisk

    FlightRisk TrainBoard Member

    548
    237
    14
    Of course we are interested, but keep in mind JMRI is not the only front end we need to support. So the DCC++ commands can't go away. Not sure if that is what you want the COMMS layer to do. We would have to find a way to integrate things in the simplest way possible. In addition, current sensing as is needed for detecting the ACK for reading and writing CVs has been problematic. So we need a good solution for that. Have you also seen what Haba is doing in his fork? Do you have networking support? If you want to join the team and move this forward, then let's compare notes.
     
  5. Atani

    Atani TrainBoard Member

    1,460
    1,697
    36
    I don't think that he is proposing that but instead moving the processing to a single location and have the DCC data stream do just that and not intermingled with a lot of other unrelated stuff.
     
  6. FlightRisk

    FlightRisk TrainBoard Member

    548
    237
    14
  7. UkBloke

    UkBloke TrainBoard Member

    42
    5
    3
    By separating the JRMI command parsing from the DCC packet generation, it allows for a completely different front end interface to be written and have it call directly into the DCC as an API would. e.g. A rotary encoder and keypad connected direct to Arduino pins.

    In fact, that's the whole reason I started this... I wanted to write a whole-layout automation directly in the Arduino and have it call the DCC layer. (including things like switching the prog track to Main in the code.) but that's another story.
     
  8. David Cutting

    David Cutting TrainBoard Member

    62
    29
    3
    @UkBloke - what you're proposing sounds interesting. I've been working on and off over the last few days trying to turn DCC++ into a library, and your proposals seem right along, and in some cases much better than, the changes I've been making. Can you share your source code so we can review it?
     
  9. FlightRisk

    FlightRisk TrainBoard Member

    548
    237
    14
    If you look at our MotorBoard Manager code, I think that is something like you are talking about? We can register "boards" but they function as tracks since in the default we only have 2 and name them PROG and MAIN. GitHub Code

    @David Cutting , Have you seen the Locduino Library? https://github.com/Locoduino/DCCpp

    @UkBloke So would you like to discuss further in general terms and come up with a diagram or get access to our repo and upload a fork there and we can look at code?
     
  10. UkBloke

    UkBloke TrainBoard Member

    42
    5
    3
    I'll take a look.

    Yes, I went there very quickly after first looking at the classic DCC++ because I was looking for a DCC API that I could call directly from my C++. I did get a working system, but its messy.

    Locoduino have already split the string parsing from the DCC message builder but IMHO have made the mistake of keeping it all in the same file with loads of #ifdef s. Unfortunately, that same file also contains all the awkward stuff to do with the packet registers and the extremely messy volatile interface to the interrupt driven waveform generators.

    I'm working on splitting out the track driver stuff from my other project so I can share it with you. I don't think forking your repo would be a good idea just yet because I would be deleting more code then adding and it could make merges very tricky.... but just to whet your appetite I will post the code of my track driver...standby!
     
  11. UkBloke

    UkBloke TrainBoard Member

    42
    5
    3
    BEWARE... this is prototype proof-of-concept only... and extracted from a larger system
    At one point this was working, but the ACK processing is still being edited so there are some constants missing....

    In general, a DCC packet generating API would create a packet (including checksums etc) and then call
    TPLDCC1::mainTrack.schedulePacket(packet); (or TPLDCC1::progTrack.schedulePacket(packet);
    (Compare that with PacketRegister LOL)

    Also note that TPLDCC1::loop() function must be called by something during the main Arduino loop() to monitor the currents.

    err..... forum wont let me upload cpp or h files,
    ??????????????????? this is the .h file ????????????????????????????
    Code:
    #include <DIO2.h>
    
    // This hardware configuration would normally be setup in a .h file elsewhere. This is for my mega with standard power shield
    // Note pin2 jumper is not needed but will be ignored if present.
    
    const byte MAIN_POWER_PIN=3;
    const byte MAIN_SIGNAL_PIN=12;
    const byte MAIN_SENSE_PIN=A0;
    
    const byte PROG_POWER_PIN=11;
    const byte PROG_SIGNAL_PIN=13;
    const byte PROG_SENSE_PIN=A1;
    
    const int  POWER_SAMPLE_MAX=300;
    const int  POWER_SAMPLE_ON_WAIT=100;
    const int  POWER_SAMPLE_OFF_WAIT=1000;
    const int  POWER_SAMPLE_OVERLOAD_WAIT=4000;
    
    
    struct DCCPacket {
           byte bits;
           byte repeats;
           byte data[10];
    };
    
    enum class POWERMODE { OFF, ON, OVERLOAD };
    
    
    // NOTE: static functions are used for the overall controller, then
    // one instance is created for each track.
    
    class TPLDCC1 {
      public:
       static void begin();
       static void loop();
       static TPLDCC2  mainTrack;
       static TPLDCC2  progTrack;
    
      void beginTrack();
      void setPowerMode(POWERMODE);
      POWERMODE getPowerMode();
      void checkPowerOverload();
      bool interrupt1();
      void interrupt2();
      void schedulePacket(DCCPacket& packet);
      volatile bool packetPending;
      bool  startAckProcess();
      bool  getAck();
    
     
      private:
      static void interruptHandler();
      POWERMODE powerMode;
      DCCPacket transmitPacket;
      DCCPacket pendingPacket;
      byte bits_sent;
      byte state;
      bool currentBit;
      GPIO_pin_t directionPin;
      GPIO_pin_t powerPin;
     
      // current sampling
      bool isMainTrack;
      byte sensePin;
      unsigned long nextSampleDue;
     
    };
    
    ??????????????????? this is the cpp file ???????????????????????????
    Code:
    #include <Arduino.h>
    #include <TimerThree.h>
    #include "TPLDCC1.h"
    
    TPLDCC1  TPLDCC1::mainTrack(MAIN_POWER_PIN,MAIN_SIGNAL_PIN,MAIN_SENSE_PIN,true);
    TPLDCC1  TPLDCC1::progTrack(PROG_POWER_PIN,PROG_SIGNAL_PIN,PROG_SENSE_PIN,false);
    
    
    
    void TPLDCC1::begin() {
       Timer3.initialize(58);
       Timer3.disablePwm(MAIN_SIGNAL_PIN);
       Timer3.disablePwm(PROG_SIGNAL_PIN);
       Timer3.attachInterrupt(interruptHandler);
       mainTrack.begin2();
       progTrack.begin2();
    }
     
     void TPLDCC1::loop() {
       mainTrack.checkPowerOverload();
       progTrack.checkPowerOverload();
     }
     
    
    // static //
    void TPLDCC1::interruptHandler() {
       // call the timer edge sensitive actions for progtrack and maintrack
       bool mainCall2=mainTrack.interrupt1();
       bool progCall2=progTrack.interrupt1();
     
       // call (if necessary) the procs to get the current bits
       // these must complete within 50microsecs of the interrupt
       // but they are only called ONCE PER BIT TRANSMITTED
       // after the rising edge of the signal
       if (mainCall2) mainTrack.interrupt2();
       if (progCall2) progTrack.interrupt2();
    }
    
    
    // An instance of this class handles the DCC transmissions for one track. (main or prog)
    // Interrupts are marshalled via the statics.
    // A track has a current transmit buffer, and a pending buffer.
    // When the current buffer is exhausted, either the pending buffer (if there is one waiting) or an idle buffer.
    const byte bitMask[8]={0x80,0x40,0x20,0x10,0x08,0x04,0x02,0x01};
    
    
    TPLDCC1::TPLDCC1(byte powerPinNo, byte directionPinNo, byte sensePinNo, bool isMain) {
       // establish appropriate pins
       powerPin=Arduino_to_GPIO_pin(powerPinNo);
       directionPin=Arduino_to_GPIO_pin(directionPinNo);
       sensePin=sensePinNo;
       isMainTrack=isMain;
       packetPending=false;
       transmitPacket=TPLDCC::idlePacket;
       state=0;
       bits_sent=0;
       nextSampleDue=0;
    }
     void TPLDCC1::beginTrack() {
       pinMode2f(powerPin,OUTPUT);
       pinMode2f(directionPin,OUTPUT);
       pinMode(sensePin,INPUT);
       setPowerMode(POWERMODE::ON);
       DIAG(F("\nTrack started sensePin=%d\n"),sensePin);
     }
    
     POWERMODE TPLDCC1::getPowerMode() {
      return powerMode;
     }
    
     void TPLDCC1::setPowerMode(POWERMODE mode) {
      powerMode=mode;
      digitalWrite2f(powerPin, mode==POWERMODE::ON ? HIGH:LOW);
     }
     
     
    void TPLDCC1::checkPowerOverload() {
      if (millis()<nextSampleDue) return;
      int current;
     
      switch (powerMode) {
        case POWERMODE::OFF:
          nextSampleDue=millis()+POWER_SAMPLE_OFF_WAIT;
          break;
       case POWERMODE::ON:
          // Check current
          current=analogRead(sensePin);
          if (current < POWER_SAMPLE_MAX)  nextSampleDue=millis()+POWER_SAMPLE_ON_WAIT;
          else {
            setPowerMode(POWERMODE::OVERLOAD);
            DIAG(F("\n*** %s TRACK POWER OVERLOAD pin=%d current=%d max=%d ***\n"),isMainTrack?"MAIN":"PROG",sensePin,current,POWER_SAMPLE_MAX);
            nextSampleDue=millis()+POWER_SAMPLE_OVERLOAD_WAIT;
          }
          break;
       case POWERMODE::OVERLOAD:
          // Try setting it back on after the OVERLOAD_WAIT
          setPowerMode(POWERMODE::ON);
          break;
      }
    }
    
    
    
    
    
    // process time-edge sensitive part of interrupt
    // return true if second level required
    bool TPLDCC1::interrupt1() {
      // NOTE: this must consume transmission buffers even if the power is off
      // otherwise can cause hangs in main loop waiting for the pendingBuffer.
      switch (state) {
        case 0:  // start of bit transmission
          digitalWrite2f(directionPin, HIGH);
          state = 1;
          return true; // must call interrupt2
    
        case 1:  // 58Ms after case 0
          if (currentBit) {
            digitalWrite2f(directionPin, LOW);
            state = 0;
          }
          else state = 2;
          break;
        case 2:  digitalWrite2f(directionPin, LOW);
          state = 3;
          break;
        case 3:  state = 0;
        break;
      }
      return false;
     
    }
    void TPLDCC1::interrupt2() {
      currentBit = transmitPacket.data[bits_sent / 8]  & bitMask[ bits_sent % 8 ];
      bits_sent++;
      if (bits_sent >= transmitPacket.bits) {
          bits_sent = 0;
          // end of transmission buffer... repeat or switch to next message
          if (transmitPacket.repeats > 0) {
            transmitPacket.repeats--;
          }
          else {
            transmitPacket= packetPending ? pendingPacket : TPLDCC::idlePacket;
            packetPending=false;
          }
          }
        }
     
      // Wait until there is no packet pending, then make this pending
    void TPLDCC1::schedulePacket(DCCPacket& newpacket) {
        while(packetPending) delay(1);
        pendingPacket=newpacket;
        packetPending=true;
      }
     
    // ACK stuff is untested!!!!
     bool  TPLDCC1::startAckProcess() {
      if (sensePin==0) return false;
      int baxse=0;
      for (int j = 0; j < ACK_BASE_COUNT; j++)
      {
        base+= (int)analogRead(sensePin);
      }
      ackBaseCurrent=base / ACK_BASE_COUNT;
      return true;
    }
    
    bool TPLDCC1::getAck()
    {
      int threshold=ackBaseCurrent+ACK_SAMPLE_THRESHOLD;
    
      for (int j = 0; j < ACK_SAMPLE_COUNT; j++)
      {
        if (analogRead(sensePin) > threshold) return true;
      }
      return false;
    }
    
     
    Last edited: May 19, 2020
  12. UkBloke

    UkBloke TrainBoard Member

    42
    5
    3
    See my git https://github.com/Asbelos/TPL ... I've been refactoring like crazy and it compiles but is not yet tested. Forget 99% of whats there, it will be interesting later... but take a look at TPLDCC.cpp (the API layer) and TPLDCC2.cpp (the motor sheld driver) ....

    This sketch doesnt have JMRI command parsing but I have a cunning plan to make a parser without all those wasteful sscanf calls!
     
  13. FlightRisk

    FlightRisk TrainBoard Member

    548
    237
    14
    Thanks for sharing that part. It is a good start in understanding your approach. The ACK detect is a challenge. Right now, we are just sending packets and checking after the group of them is sent. It works, but we probably should be checking after every packet to see if we got an ACK and stop. It works, but could be more "elegant" as my college EE professor used to say all the time. Is settle time between readings enough? Are we handling the differences between current sense boards? etc.
     
  14. UkBloke

    UkBloke TrainBoard Member

    42
    5
    3
    Yes, the ack part of my code is not tested at all, just placemarking. But now that we can see the wood for the trees it should be possible to explore the DCC specs more closely and implement something more robust.
     
  15. UkBloke

    UkBloke TrainBoard Member

    42
    5
    3
    While checking out the Dcc wiki, I discovered this.......
    ... [I removed this ill judged post because I missed an important technical point making my assumptions completely bogus. Sorry if you read the original and got confused as I was.]
     
    Last edited: May 20, 2020
  16. Atani

    Atani TrainBoard Member

    1,460
    1,697
    36
    @UkBloke you are correct in that OPS only needs "more than 11" preamble bits and you are also correct in that the DCC++ loadPacket method is rather complex. However, the reason behind the 22 bit preamble is due to the programming track which needs at least 22 preamble bits. The original author of the code opted to send the same preamble bits to both OPS and PROG outputs for code simplicity (though the code is anything but!).

    This is an area that should be revisited to simplify and disconnect the preamble from the payload if possible. That way OPS can have the 11 bit (no RailCom) or 16 bit (with RailCom) preamble bit lengths and PROG can retain it's minimum of 22 bit preamble.
     
  17. UkBloke

    UkBloke TrainBoard Member

    42
    5
    3
    Aha... you read my post before I deleted it....
    Anyway I did look further into this and now understand about the 0-bit before each message byte.
    Thinking about how messy this is when trying to create a bitpattern of the complete package, it occurred to me that this can be simplified in the transmitter code (well in my version of it anyway ;) by:

    a) The transmitter automatically sends "n" preamble 1s before each message. (where n is different between the progtrack and maintrack)
    b) the transmitter scans each byte of the message 9 (yes NINE) times instead of eight with a bitmask where the first byte of the bitmask is 0x00 so that a zero bit is detected.
    c) and while it's doing the byte loop it can calc the checksum on the fly...
    d) the 1 bit at the end is actually unnecessary because it will be provided by the preamble of whatever comes next!


    This means that the DCCAPI layer need only generate the basic message bytes and pass that to the transmitter... leaving out all the loadPackage bit shifting and the kluge of having to provide 1 more byte than necessary to accomodate the checksum. Yay ... even more code eliminated!

    Did you take a look at my repo https://github.com/Asbelos/TPL TPLDCC.cpp and TPLDCC1.cpp? I'll add the changes above later today.
     
  18. Atani

    Atani TrainBoard Member

    1,460
    1,697
    36
    I would leave the checksum to the loadPacket() method as it doesn't need to be calculated on-the-fly by the transmitter code. The transmit code can then handle only prepending the required preamble and triggering the RailCom cut-out immediately after queuing the first bit of the preamble.
     
  19. UkBloke

    UkBloke TrainBoard Member

    42
    5
    3
    Agreed, I actually moved it to the point where the message is loaded into the pendingPacket... just seemed the neatest place.
    I have done a load of tidying up... its still untested but it does compile. I generally find that the more tidying one does, the less there is to test and the easier it is to spot the problems.

    I did wonder about some things though... does the API really need to support __reading__ CVs on MAIN?
     
  20. UkBloke

    UkBloke TrainBoard Member

    42
    5
    3
    Yup, did that.
     

Share This Page