From ecbedd26bc3dc63cc094a4b6f5976685427b635e Mon Sep 17 00:00:00 2001 From: mstevetodd Date: Wed, 12 Aug 2020 11:17:05 -0400 Subject: [PATCH 1/4] withrottle changes disallow acquire if address in use return error on address 0 and mismatched L/S move init string to 'HU' reply some message format fixes comment-out some DIAGs to show only in and out message traffic --- DCC.cpp | 9 +++++ DCC.h | 1 + DCCEXParser.cpp | 9 ++--- WiThrottle.cpp | 90 ++++++++++++++++++++++++++++------------------- WiThrottle.h | 9 ++--- WifiInterface.cpp | 8 ++--- 6 files changed, 78 insertions(+), 48 deletions(-) diff --git a/DCC.cpp b/DCC.cpp index 6d47188..4d5e567 100644 --- a/DCC.cpp +++ b/DCC.cpp @@ -95,6 +95,15 @@ bool DCC::getThrottleDirection(int cab) { return (speedTable[reg].speedCode & 0x80) !=0; } +bool DCC::isThrottleInUse(int locoId) { + // return true if this loco address is already in table, false otherwise + int reg; + for (reg = 0; reg < MAX_LOCOS; reg++) { + if (speedTable[reg].loco == locoId) return true; + } + return false; +} + // Set function to value on or off void DCC::setFn( int cab, byte functionNumber, bool on) { if (cab<=0 || functionNumber>28) return; diff --git a/DCC.h b/DCC.h index d549490..74ac951 100644 --- a/DCC.h +++ b/DCC.h @@ -56,6 +56,7 @@ class DCC { static void setThrottle( uint16_t cab, uint8_t tSpeed, bool tDirection); static uint8_t getThrottleSpeed(int cab); static bool getThrottleDirection(int cab); + static bool isThrottleInUse(int cab); static void writeCVByteMain(int cab, int cv, byte bValue); static void writeCVBitMain(int cab, int cv, byte bNum, bool bValue); static void setFunction( int cab, byte fByte, byte eByte); diff --git a/DCCEXParser.cpp b/DCCEXParser.cpp index 0ff5d0e..583b9c6 100644 --- a/DCCEXParser.cpp +++ b/DCCEXParser.cpp @@ -147,16 +147,17 @@ void DCCEXParser::parse(Print * stream, byte *com, bool blocking) { case '\0': return; // filterCallback asked us to ignore case 't': // THROTTLE { + if (p[1] == 0) break; // ignore requests for throttle address 0 (returns 'X') // Convert JMRI bizarre -1=emergency stop, 0-126 as speeds // to DCC 0=stop, 1= emergency stop, 2-127 speeds int tspeed=p[2]; if (tspeed>126 || tspeed<-1) break; // invalid JMRI speed code if (tspeed<0) tspeed=1; // emergency stop DCC speed else if (tspeed>0) tspeed++; // map 1-126 -> 2-127 - DCC::setThrottle(p[1],tspeed,p[3]); - // report speed 0 after emergency stop - StringFormatter::send(stream,F(""), p[0], p[2]<0?0:p[2],p[3]); - return; + DCC::setThrottle(p[1],tspeed,p[3]); + // report speed 0 after emergency stop + StringFormatter::send(stream,F(""), p[0], p[2]<0?0:p[2],p[3]); + return; } case 'f': // FUNCTION if (parsef(stream,params,p)) return; diff --git a/WiThrottle.cpp b/WiThrottle.cpp index a5c4335..61c5492 100644 --- a/WiThrottle.cpp +++ b/WiThrottle.cpp @@ -66,9 +66,10 @@ WiThrottle::WiThrottle( int wificlientid) { nextThrottle=firstThrottle; firstThrottle= this; clientid=wificlientid; - heartBeatEnable=false; // until client turns it on - callState=0; - for (int loco=0;loconextTurnout){ - StringFormatter::send(stream,F("]\\[%d}|{T%d}|{%d"), tt->data.id, tt->data.id, (bool)(tt->data.tStatus & STATUS_ACTIVE)); - } - StringFormatter::send(stream,F("\n")); - } - break; - default: // no more special headers required - break; +// DIAG(F("\nWiThrottle(%d)<-[%e]\n"),clientid, cmd); + + //send turnoutlist on next response after the init string + if (initSent && !turnoutListSent) { + // Send turnout list if populated + if (Turnout::firstTurnout) { + StringFormatter::send(stream,F("PTL")); + for(Turnout *tt=Turnout::firstTurnout;tt!=NULL;tt=tt->nextTurnout){ + StringFormatter::send(stream,F("]\\[%d}|{T%d}|{%d"), tt->data.id, tt->data.id, (bool)(tt->data.tStatus & STATUS_ACTIVE)); } + StringFormatter::send(stream,F("\n")); + } + turnoutListSent = true; + } while (cmd[0]) { switch (cmd[0]) { @@ -161,10 +154,17 @@ void WiThrottle::parse(Print & stream, byte * cmdx) { case 'M': // multithrottle multithrottle(stream, cmd); break; - case 'H': // hardware introduction.... + case 'H': // send initial connection info after receiving "HU" message + if (cmd[1] == 'U') { + StringFormatter::send(stream,F("VN2.0\nHTDCC++EX\nRL0\nPPA%x\n"),DCCWaveform::mainTrack.getPowerMode()==POWERMODE::ON); + if (annotateLeftRight) StringFormatter::send(stream,F("PTT]\\[Turnouts}|{Turnout]\\[Left}|{2]\\[Right}|{4\n")); + else StringFormatter::send(stream,F("PTT]\\[Turnouts}|{Turnout]\\[Closed}|{2]\\[Thrown}|{4\n")); + StringFormatter::send(stream,F("*%d\n"),HEARTBEAT_TIMEOUT); + initSent = true; + } break; case 'Q': // - DIAG(F("\nWiThrottle Quit")); + DIAG(F("WiThrottle Quit\n")); delete this; break; } @@ -194,19 +194,36 @@ void WiThrottle::multithrottle(Print & stream, byte * cmd){ while(*aval !=';' && *aval !='\0') aval++; if (*aval) aval+=2; // skip ;> - DIAG(F("\nMultithrottle aval=%c cab=%d"), aval[0],locoid); +// DIAG(F("\nMultithrottle aval=%c cab=%d"), aval[0],locoid); switch(cmd[2]) { - case '+': // add loco + case '+': // add loco request + //return error if address zero requested + if (locoid==0) { + StringFormatter::send(stream, F("HMAddress '0' not supported!\n"), cmd[3] ,locoid); + return; + } + //return error if L or S from request doesn't match DCC++ assumptions + if (cmd[3] != LorS(locoid)) { + StringFormatter::send(stream, F("HMLength '%c' not valid for %d!\n"), cmd[3] ,locoid); + return; + } + //return error if address is already in use + if (DCC::isThrottleInUse(locoid)) { + StringFormatter::send(stream, F("HMAddress '%d' in use!\n"), locoid); + return; + } for (int loco=0;loco\n"), throttleChar, cmd[3] ,locoid); // TODO... get known Fn states from DCC (need memoryStream improvements to handle data length) // for(fKey=0; fKey<29; fKey++)StringFormatter::send(stream,F("M%cA%c<;>F0&s\n"),throttleChar,cmd[3],fkey); - StringFormatter::send(stream, F("M%c+%c%d<;>V0\n"), throttleChar, cmd[3], locoid); - StringFormatter::send(stream, F("M%c+%c%d<;>R1\n"), throttleChar, cmd[3], locoid); - StringFormatter::send(stream, F("M%c+%c%d<;>s1\n"), throttleChar, cmd[3], locoid); + StringFormatter::send(stream, F("M%cA%c%d<;>V0\n"), throttleChar, cmd[3], locoid); //default speed 0 + StringFormatter::send(stream, F("M%cA%c%d<;>R1\n"), throttleChar, cmd[3], locoid); //default forward + StringFormatter::send(stream, F("M%cA%c%d<;>s1\n"), throttleChar, cmd[3], locoid); //default speed step 128 break; } } @@ -215,7 +232,8 @@ void WiThrottle::multithrottle(Print & stream, byte * cmd){ LOOPLOCOS(throttleChar, locoid) { myLocos[loco].throttle='\0'; DCC::setThrottle(myLocos[loco].cab,0,0); - StringFormatter::send(stream, F("M%c-<;>\n"), throttleChar); + DCC::forgetLoco(myLocos[loco].cab); //unregister this loco address + StringFormatter::send(stream, F("M%c-%c%d<;>\n"), throttleChar, LorS(myLocos[loco].cab), myLocos[loco].cab); } break; @@ -226,7 +244,7 @@ void WiThrottle::multithrottle(Print & stream, byte * cmd){ void WiThrottle::locoAction(Print & stream, byte* aval, char throttleChar, int cab){ // Note cab=-1 for all cabs in the consist called throttleChar. - DIAG(F("\nLoco Action aval=%c%c throttleChar=%c, cab=%d"), aval[0],aval[1],throttleChar, cab); +// DIAG(F("\nLoco Action aval=%c%c throttleChar=%c, cab=%d"), aval[0],aval[1],throttleChar, cab); switch (aval[0]) { case 'V': // Vspeed { @@ -275,8 +293,8 @@ void WiThrottle::locoAction(Print & stream, byte* aval, char throttleChar, int c DCC::setThrottle(myLocos[loco].cab,1, DCC::getThrottleDirection(myLocos[loco].cab)); } break; - case 'I': // Idle - case 'Q': // Quit + case 'I': // Idle, set speed to 0 + case 'Q': // Quit, set speed to 0 LOOPLOCOS(throttleChar, cab) { DCC::setThrottle(myLocos[loco].cab,0, DCC::getThrottleDirection(myLocos[loco].cab)); } diff --git a/WiThrottle.h b/WiThrottle.h index 1e64a29..5351c14 100644 --- a/WiThrottle.h +++ b/WiThrottle.h @@ -17,7 +17,7 @@ * along with CommandStation. If not, see . */ #ifndef WiThrottle_h -#define WiTHrottle_h +#define WiThrottle_h struct MYLOCO { @@ -35,8 +35,8 @@ class WiThrottle { WiThrottle( int wifiClientId); ~WiThrottle(); - static const int MAX_MY_LOCO=10; - static const int HEARTBEAT_TIMEOUT=10;// Timeout after 10 seconds, heartbeat at 5 + static const int MAX_MY_LOCO=10; //maximum number of locos assigned to a single client + static const int HEARTBEAT_TIMEOUT=4;// heartbeat at 4secs to provide messaging transport static WiThrottle* firstThrottle; static int getInt(byte * cmd); static int getLocoId(byte * cmd); @@ -46,7 +46,8 @@ class WiThrottle { MYLOCO myLocos[MAX_MY_LOCO]; bool heartBeatEnable; - byte callState; // 0=first, 1=second, 3=third, 4=fourth call... >4=rest. + bool initSent=false; + bool turnoutListSent=false; unsigned long heartBeat; void multithrottle(Print & stream, byte * cmd); diff --git a/WifiInterface.cpp b/WifiInterface.cpp index 8a2a965..b34a02c 100644 --- a/WifiInterface.cpp +++ b/WifiInterface.cpp @@ -167,7 +167,7 @@ void WifiInterface::loop() { int ch=wifiStream->read(); // echo the char to the diagnostic stream in escaped format - StringFormatter::printEscape(&DIAGSERIAL,ch); // DIAG in disguise +// StringFormatter::printEscape(&DIAGSERIAL,ch); // DIAG in disguise switch (loopstate) { case 0: // looking for +IPD @@ -208,7 +208,7 @@ void WifiInterface::loop() { break; } if (ch=='>'){ - DIAG(F("\n> [%e]\n"),buffer); +// DIAG(F("\n> [%e]\n"),buffer); wifiStream->print((char *) buffer); loopTimeoutStart=millis(); loopstate=closeAfter?11:0; @@ -232,7 +232,7 @@ void WifiInterface::loop() { // AT this point we have read an incoming message into the buffer streamer.print('\0'); // null the end of the buffer so we can treat it as a string - DIAG(F("\nWifiRead:%d:%e\n"),connectionId,buffer); + DIAG(F("\nWifi(%d)<-[%e]\n"),connectionId,buffer); streamer.setBufferContentPosition(0,0); // reset write position to start of buffer // SIDE EFFECT WARNING::: // We know that parser will read the entire buffer before starting to write to it. @@ -256,7 +256,7 @@ void WifiInterface::loop() { } // prepare to send reply streamer.print('\0'); // null the end of the buffer so we can treat it as a string - DIAG(F("\nWiFiInterface reply c(%d) l(%d) [%e]\n"),connectionId,streamer.available()-1,buffer); + DIAG(F("WiFi(%d)->[%e] l(%d)\n"),connectionId,buffer,streamer.available()-1); StringFormatter::send(wifiStream,F("AT+CIPSEND=%d,%d\r\n"),connectionId,streamer.available()-1); loopTimeoutStart=millis(); loopstate=10; // non-blocking loop waits for > before sending From 52b0502776b0baeb546c653544b23ae2f0c3b288 Mon Sep 17 00:00:00 2001 From: SteveT Date: Wed, 12 Aug 2020 15:10:51 -0400 Subject: [PATCH 2/4] set initial direction forward, forget locos on 2nd missed heartbeat --- WiThrottle.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/WiThrottle.cpp b/WiThrottle.cpp index 61c5492..48a95fa 100644 --- a/WiThrottle.cpp +++ b/WiThrottle.cpp @@ -149,7 +149,7 @@ void WiThrottle::parse(Print & stream, byte * cmdx) { } break; case 'N': // Heartbeat (2) - StringFormatter::send(stream, F("*%d\n"),HEARTBEAT_TIMEOUT); // 10 second timeout + StringFormatter::send(stream, F("*%d\n"),HEARTBEAT_TIMEOUT); // return timeout value break; case 'M': // multithrottle multithrottle(stream, cmd); @@ -217,7 +217,7 @@ void WiThrottle::multithrottle(Print & stream, byte * cmd){ if (myLocos[loco].throttle=='\0') { myLocos[loco].throttle=throttleChar; myLocos[loco].cab=locoid; - DCC::setThrottle(locoid,0,0); //register this loco address + DCC::setThrottle(locoid,0,1); //register this loco address, speed zero, direction forward StringFormatter::send(stream, F("M%c+%c%d<;>\n"), throttleChar, cmd[3] ,locoid); // TODO... get known Fn states from DCC (need memoryStream improvements to handle data length) // for(fKey=0; fKey<29; fKey++)StringFormatter::send(stream,F("M%cA%c<;>F0&s\n"),throttleChar,cmd[3],fkey); @@ -231,7 +231,7 @@ void WiThrottle::multithrottle(Print & stream, byte * cmd){ case '-': // remove loco LOOPLOCOS(throttleChar, locoid) { myLocos[loco].throttle='\0'; - DCC::setThrottle(myLocos[loco].cab,0,0); + DCC::setThrottle(myLocos[loco].cab,0,1); DCC::forgetLoco(myLocos[loco].cab); //unregister this loco address StringFormatter::send(stream, F("M%c-%c%d<;>\n"), throttleChar, LorS(myLocos[loco].cab), myLocos[loco].cab); } @@ -309,17 +309,17 @@ void WiThrottle::loop() { } void WiThrottle::checkHeartbeat() { - if(heartBeatEnable && (millis()-heartBeat > HEARTBEAT_TIMEOUT*1000)) { + // if 2 heartbeats missed... STOP and forget all locos for this client + if(heartBeatEnable && (millis()-heartBeat > HEARTBEAT_TIMEOUT*2000)) { DIAG(F("WiThrottle hearbeat missed client=%d"),clientid); - // Haertbeat missed... STOP all locos for this client for (int loco=0;loco Date: Wed, 12 Aug 2020 21:39:57 -0400 Subject: [PATCH 3/4] return new speed and dir to client, add timestamp to DIAG --- WiThrottle.cpp | 12 +++++++----- WiThrottle.h | 2 +- WifiInterface.cpp | 4 ++-- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/WiThrottle.cpp b/WiThrottle.cpp index 48a95fa..6d26809 100644 --- a/WiThrottle.cpp +++ b/WiThrottle.cpp @@ -144,12 +144,12 @@ void WiThrottle::parse(Print & stream, byte * cmdx) { case 'C': newstate=false; break; case '2': newstate=!Turnout::isActive(id); } - Turnout::activate(id,newstate); + Turnout::activate(id,newstate); StringFormatter::send(stream, F("PTA%c%d\n"),newstate?'4':'2',id ); } break; case 'N': // Heartbeat (2) - StringFormatter::send(stream, F("*%d\n"),HEARTBEAT_TIMEOUT); // return timeout value + StringFormatter::send(stream, F("*%d\n"),HEARTBEAT_TIMEOUT); // return timeout value break; case 'M': // multithrottle multithrottle(stream, cmd); @@ -251,6 +251,7 @@ void WiThrottle::locoAction(Print & stream, byte* aval, char throttleChar, int c byte locospeed=getInt(aval+1); LOOPLOCOS(throttleChar, cab) { DCC::setThrottle(myLocos[loco].cab,locospeed, DCC::getThrottleDirection(myLocos[loco].cab)); + StringFormatter::send(stream,F("M%cA%c%d<;>V%d\n"), throttleChar, LorS(myLocos[loco].cab), myLocos[loco].cab, DCC::getThrottleSpeed(myLocos[loco].cab)); } } break; @@ -282,9 +283,10 @@ void WiThrottle::locoAction(Print & stream, byte* aval, char throttleChar, int c case 'R': { bool forward=aval[1]!='0'; - LOOPLOCOS(throttleChar, cab) { - DCC::setThrottle(myLocos[loco].cab, DCC::getThrottleSpeed(myLocos[loco].cab), forward); - } + LOOPLOCOS(throttleChar, cab) { + DCC::setThrottle(myLocos[loco].cab, DCC::getThrottleSpeed(myLocos[loco].cab), forward); + StringFormatter::send(stream,F("M%cA%c%d<;>R%d\n"), throttleChar, LorS(myLocos[loco].cab), myLocos[loco].cab, DCC::getThrottleDirection(myLocos[loco].cab)); + } } break; case 'X': diff --git a/WiThrottle.h b/WiThrottle.h index 5351c14..612e95a 100644 --- a/WiThrottle.h +++ b/WiThrottle.h @@ -36,7 +36,7 @@ class WiThrottle { ~WiThrottle(); static const int MAX_MY_LOCO=10; //maximum number of locos assigned to a single client - static const int HEARTBEAT_TIMEOUT=4;// heartbeat at 4secs to provide messaging transport + static const int HEARTBEAT_TIMEOUT=3;// heartbeat at 3secs to provide messaging transport static WiThrottle* firstThrottle; static int getInt(byte * cmd); static int getLocoId(byte * cmd); diff --git a/WifiInterface.cpp b/WifiInterface.cpp index b34a02c..da471c7 100644 --- a/WifiInterface.cpp +++ b/WifiInterface.cpp @@ -232,7 +232,7 @@ void WifiInterface::loop() { // AT this point we have read an incoming message into the buffer streamer.print('\0'); // null the end of the buffer so we can treat it as a string - DIAG(F("\nWifi(%d)<-[%e]\n"),connectionId,buffer); + DIAG(F("\n%d Wifi(%d)<-[%e]\n"),millis(),connectionId,buffer); streamer.setBufferContentPosition(0,0); // reset write position to start of buffer // SIDE EFFECT WARNING::: // We know that parser will read the entire buffer before starting to write to it. @@ -256,7 +256,7 @@ void WifiInterface::loop() { } // prepare to send reply streamer.print('\0'); // null the end of the buffer so we can treat it as a string - DIAG(F("WiFi(%d)->[%e] l(%d)\n"),connectionId,buffer,streamer.available()-1); + DIAG(F("%d WiFi(%d)->[%e] l(%d)\n"),millis(),connectionId,buffer,streamer.available()-1); StringFormatter::send(wifiStream,F("AT+CIPSEND=%d,%d\r\n"),connectionId,streamer.available()-1); loopTimeoutStart=millis(); loopstate=10; // non-blocking loop waits for > before sending From 68255dcadd4344d3c5ab5e503b78bdd729ce81ce Mon Sep 17 00:00:00 2001 From: mstevetodd Date: Thu, 13 Aug 2020 16:38:22 -0400 Subject: [PATCH 4/4] drop any acquired locos on Q(uit), reduce heartbeat to 2s also some indentation fixes note that 2s heartbeat works best with next version of ED. --- WiThrottle.cpp | 39 ++++++++++++++++++++++++--------------- WiThrottle.h | 2 +- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/WiThrottle.cpp b/WiThrottle.cpp index 6d26809..644b4a1 100644 --- a/WiThrottle.cpp +++ b/WiThrottle.cpp @@ -48,7 +48,7 @@ #include "DIAG.h" #define LOOPLOCOS(THROTTLECHAR, CAB) for (int loco=0;loco\n"), myLocos[loco].throttle, LorS(myLocos[loco].cab), myLocos[loco].cab); + myLocos[loco].throttle='\0'; + } + } + DIAG(F("WiThrottle(%d) Quit\n"), clientid); delete this; break; } @@ -228,10 +236,10 @@ void WiThrottle::multithrottle(Print & stream, byte * cmd){ } } break; - case '-': // remove loco + case '-': // stop and remove loco(s) LOOPLOCOS(throttleChar, locoid) { myLocos[loco].throttle='\0'; - DCC::setThrottle(myLocos[loco].cab,0,1); + DCC::setThrottle(myLocos[loco].cab,0, DCC::getThrottleDirection(myLocos[loco].cab)); DCC::forgetLoco(myLocos[loco].cab); //unregister this loco address StringFormatter::send(stream, F("M%c-%c%d<;>\n"), throttleChar, LorS(myLocos[loco].cab), myLocos[loco].cab); } @@ -293,14 +301,14 @@ void WiThrottle::locoAction(Print & stream, byte* aval, char throttleChar, int c //Emergency Stop (speed code 1) LOOPLOCOS(throttleChar, cab) { DCC::setThrottle(myLocos[loco].cab,1, DCC::getThrottleDirection(myLocos[loco].cab)); - } - break; + } + break; case 'I': // Idle, set speed to 0 case 'Q': // Quit, set speed to 0 LOOPLOCOS(throttleChar, cab) { DCC::setThrottle(myLocos[loco].cab,0, DCC::getThrottleDirection(myLocos[loco].cab)); - } - break; + } + break; } } @@ -313,12 +321,13 @@ void WiThrottle::loop() { void WiThrottle::checkHeartbeat() { // if 2 heartbeats missed... STOP and forget all locos for this client if(heartBeatEnable && (millis()-heartBeat > HEARTBEAT_TIMEOUT*2000)) { - DIAG(F("WiThrottle hearbeat missed client=%d"),clientid); - for (int loco=0;loco