From 776a098a724f941001a1c7ec8239122a3a508818 Mon Sep 17 00:00:00 2001 From: Neil McKechnie Date: Thu, 19 Aug 2021 20:17:48 +0100 Subject: [PATCH 01/11] Bump EESTORE_ID version. TurnoutData struct size has been reduced by one byte during rewrite of Turnout class. Consequently, this renders any previous turnout definitions in EEPROM incompatible with the new format. For safety, the version is increased so that incompatible EEPROM contents are discarded. --- EEStore.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/EEStore.h b/EEStore.h index 247e30a..8fc98bd 100644 --- a/EEStore.h +++ b/EEStore.h @@ -29,7 +29,7 @@ extern ExternalEEPROM EEPROM; #include #endif -#define EESTORE_ID "DCC++0" +#define EESTORE_ID "DCC++1" struct EEStoreData{ char id[sizeof(EESTORE_ID)]; From fd36ca2b92abb54ac31bc121aaf757ee659e6574 Mon Sep 17 00:00:00 2001 From: Neil McKechnie Date: Thu, 19 Aug 2021 21:22:59 +0100 Subject: [PATCH 02/11] Restructure Turnout class. Turnout class split into a base class for common code and specific subclasses for Servo, DCC, VPIN and LCN turnouts. Interface further narrowed to reduce direct access to member variables. Turnout creation command handling has been moved into the DCCEXParser class. Turnout function and parameter names changed to make the Throw and Close functionality explicit. Turnout commands (close) and (throw) added. --- DCCEXParser.cpp | 66 +++++- LCN.cpp | 3 +- RMFT2.cpp | 6 +- Turnouts.cpp | 482 ++++++++++++------------------------------ Turnouts.h | 549 +++++++++++++++++++++++++++++++++++++++--------- WiThrottle.cpp | 7 +- 6 files changed, 645 insertions(+), 468 deletions(-) diff --git a/DCCEXParser.cpp b/DCCEXParser.cpp index 816cf85..4a27514 100644 --- a/DCCEXParser.cpp +++ b/DCCEXParser.cpp @@ -56,7 +56,8 @@ const int16_t HASH_KEYWORD_LCN = 15137; const int16_t HASH_KEYWORD_RESET = 26133; const int16_t HASH_KEYWORD_SPEED28 = -17064; const int16_t HASH_KEYWORD_SPEED128 = 25816; -const int16_t HASH_KEYWORD_SERVO = 27709; +const int16_t HASH_KEYWORD_SERVO=27709; +const int16_t HASH_KEYWORD_VPIN=-415; int16_t DCCEXParser::stashP[MAX_COMMAND_PARAMS]; bool DCCEXParser::stashBusy; @@ -658,7 +659,7 @@ bool DCCEXParser::parseT(Print *stream, int16_t params, int16_t p[]) case 0: // list turnout definitions { bool gotOne = false; - for (Turnout *tt = Turnout::firstTurnout; tt != NULL; tt = tt->nextTurnout) + for (Turnout *tt = Turnout::first(); tt != NULL; tt = tt->next()) { gotOne = true; tt->print(stream); @@ -672,17 +673,62 @@ bool DCCEXParser::parseT(Print *stream, int16_t params, int16_t p[]) StringFormatter::send(stream, F("\n")); return true; - case 2: // turnout 0=CLOSE,1=THROW - if (p[1]>1 || p[1]<0 ) return false; - if (!Turnout::setClosed(p[0],p[1]==0)) return false; + case 2: // + switch (p[1]) { +#ifdef TURNOUT_LEGACY_BEHAVIOUR + // turnout 1 or T=THROW, 0 or C=CLOSE + case 1: case 0x54: // 1 or T + if (!Turnout::setClosed(p[0], false)) return false; + break; + case 0: case 0x43: // 0 or C + if (!Turnout::setClosed(p[0], true)) return false; + break; +#else + // turnout 0 or T=THROW,1 or C=CLOSE + case 0: case 0x54: // 0 or T + if (!Turnout::setClosed(p[0], false)) return false; + break; + case 1: case 0x43: // 1 or C + if (!Turnout::setClosed(p[0], true)) return false; + break; +#endif + default: + return false; + } + // Send acknowledgement to caller, and to Serial. StringFormatter::send(stream, F("\n"), p[0], p[1]); + if (stream != &Serial) StringFormatter::send(Serial, F("\n"), p[0], p[1]); return true; - default: // Anything else is handled by Turnout class. - if (!Turnout::create(p[0], params-1, &p[1])) + default: // Anything else is some kind of create function. + if (p[1] == HASH_KEYWORD_SERVO) { // + if (params == 6) { + if (!ServoTurnout::create(p[0], (VPIN)p[2], (uint16_t)p[3], (uint16_t)p[4], (uint8_t)p[5])) return false; - StringFormatter::send(stream, F("\n")); - return true; + } else + return false; + } else + if (p[1] == HASH_KEYWORD_VPIN) { // + if (params==3) { + if (VpinTurnout::create(p[0], p[2])) return false; + } else + return false; + } else + if (p[1]==HASH_KEYWORD_DCC) { + if (params==4 && p[2]>0 && p[2]<=512 && p[3]>=0 && p[3]<4) { // + if (!DCCTurnout::create(p[0], p[2], p[3])) return false; + } else if (params==3 && p[2]>0 && p[2]<=512*4) { // + if (!DCCTurnout::create(p[0], (p[2]-1)/4+1, (p[2]-1)%4)) return false; + } else + return false; + } else if (params==3) { // for DCC or LCN + if (!DCCTurnout::create(p[0], p[1], p[2])) return false; + } + else if (params==3) { // legacy for Servo + if (!ServoTurnout::create(p[0], (VPIN)p[1], (uint16_t)p[2], (uint16_t)p[3], 1)) return false; + } + StringFormatter::send(stream, F("\n")); + return true; } } @@ -797,7 +843,7 @@ bool DCCEXParser::parseD(Print *stream, int16_t params, int16_t p[]) StringFormatter::send(stream, F("128 Speedsteps")); return true; - case HASH_KEYWORD_SERVO: + case HASH_KEYWORD_SERVO: // IODevice::writeAnalogue(p[1], p[2], params>3 ? p[3] : 0); break; diff --git a/LCN.cpp b/LCN.cpp index e9d0886..16b3f3f 100644 --- a/LCN.cpp +++ b/LCN.cpp @@ -48,8 +48,7 @@ void LCN::loop() { } else if (ch == 't' || ch == 'T') { // Turnout opcodes if (Diag::LCN) DIAG(F("LCN IN %d%c"),id,(char)ch); - Turnout * tt = Turnout::get(id); - if (!tt) tt=Turnout::createLCN(id); + if (!Turnout::exists(id)) LCNTurnout::create(id); Turnout::setClosedStateOnly(id,ch=='t'); Turnout::turnoutlistHash++; // signals ED update of turnout data id = 0; diff --git a/RMFT2.cpp b/RMFT2.cpp index 5e4fd62..8e155e4 100644 --- a/RMFT2.cpp +++ b/RMFT2.cpp @@ -77,7 +77,7 @@ byte RMFT2::flags[MAX_FLAGS]; VPIN id=GET_OPERAND(0); int addr=GET_OPERAND(1); byte subAddr=GET_OPERAND(2); - Turnout::createDCC(id,addr,subAddr); + DCCTurnout::create(id,addr,subAddr); continue; } @@ -87,14 +87,14 @@ byte RMFT2::flags[MAX_FLAGS]; int activeAngle=GET_OPERAND(2); int inactiveAngle=GET_OPERAND(3); int profile=GET_OPERAND(4); - Turnout::createServo(id,pin,activeAngle,inactiveAngle,profile); + ServoTurnout::create(id,pin,activeAngle,inactiveAngle,profile); continue; } if (opcode==OPCODE_PINTURNOUT) { int16_t id=GET_OPERAND(0); VPIN pin=GET_OPERAND(1); - Turnout::createVpin(id,pin); + VpinTurnout::create(id,pin); continue; } // other opcodes are not needed on this pass diff --git a/Turnouts.cpp b/Turnouts.cpp index c18337b..bd5fd5b 100644 --- a/Turnouts.cpp +++ b/Turnouts.cpp @@ -1,4 +1,5 @@ /* + * © 2021 Restructured Neil McKechnie * © 2013-2016 Gregg E. Berman * © 2020, Chris Harlow. All rights reserved. * © 2020, Harald Barth. @@ -19,17 +20,12 @@ * along with CommandStation. If not, see . */ -// >>>>>> ATTENTION: This class requires major cleaning. -// The public interface has been narrowed to avoid the ambuguity of "activated". - - -//#define EESTOREDEBUG #include "defines.h" -#include "Turnouts.h" #include "EEStore.h" #include "StringFormatter.h" #include "RMFT2.h" +#include "Turnouts.h" #ifdef EESTOREDEBUG #include "DIAG.h" #endif @@ -39,370 +35,150 @@ const int16_t HASH_KEYWORD_SERVO=27709; const int16_t HASH_KEYWORD_DCC=6436; const int16_t HASH_KEYWORD_VPIN=-415; -enum unit8_t { - TURNOUT_DCC = 1, - TURNOUT_SERVO = 2, - TURNOUT_VPIN = 3, - TURNOUT_LCN = 4, -}; + /* + * Protected static data + */ + Turnout *Turnout::_firstTurnout = 0; - - -/////////////////////////////////////////////////////////////////////////////// -// Static function to print all Turnout states to stream in form "" - -void Turnout::printAll(Print *stream){ - for (Turnout *tt = Turnout::firstTurnout; tt != NULL; tt = tt->nextTurnout) - StringFormatter::send(stream, F("\n"), tt->data.id, tt->data.active); -} // Turnout::printAll - -/////////////////////////////////////////////////////////////////////////////// -// Object method to print configuration of one Turnout to stream, in one of the following forms: -// -// -// -// - -void Turnout::print(Print *stream){ - uint8_t state = ((data.active) != 0); - uint8_t type = data.type; - switch (type) { - case TURNOUT_LCN: - // LCN Turnout - StringFormatter::send(stream, F("\n"), data.id, state); - break; - case TURNOUT_DCC: - // DCC Turnout - StringFormatter::send(stream, F("\n"), data.id, - (((data.dccAccessoryData.address-1) >> 2)+1), ((data.dccAccessoryData.address-1) & 3), state); - break; - case TURNOUT_VPIN: - // VPIN Digital output - StringFormatter::send(stream, F("\n"), data.id, data.vpinData.vpin, state); - break; - case TURNOUT_SERVO: - // Servo Turnout - StringFormatter::send(stream, F("\n"), data.id, data.servoData.vpin, - data.servoData.activePosition, data.servoData.inactivePosition, data.servoData.profile, state); - break; - default: - break; - } -} - - -// Public interface to turnout throw/close -bool Turnout::setClosed(int id, bool closed) { - // hides the internal activate argument to a single place - return activate(id, closed? false: true ); /// Needs cleaning up -} -bool Turnout::isClosed(int id) { - // hides the internal activate argument to a single place - return !isActive(id); /// Needs cleaning up -} -int Turnout::getId() { - return data.id; -} - -/////////////////////////////////////////////////////////////////////////////// -// Static function to activate/deactivate Turnout with ID 'n'. -// Returns false if turnout not found. - + /* + * Public static data + */ + int Turnout::turnoutlistHash = 0; -bool Turnout::activate(int n, bool state){ - Turnout * tt=get(n); - if (!tt) return false; - tt->activate(state); - turnoutlistHash++; - return true; -} + /* + * Protected static functions + */ -/////////////////////////////////////////////////////////////////////////////// -// Static function to check if the Turnout with ID 'n' is activated or not. -// Returns false if turnout not found. - -bool Turnout::isActive(int n){ - Turnout * tt=get(n); - if (!tt) return false; - return tt->isActive(); -} - - -/////////////////////////////////////////////////////////////////////////////// -// Object function to check the status of Turnout is activated or not. - -bool Turnout::isActive() { - return data.active; -} - -/////////////////////////////////////////////////////////////////////////////// -// Object method to activate or deactivate the Turnout. - -// activate is virtual here so that it can be overridden by a non-DCC turnout mechanism -void Turnout::activate(bool state) { -#ifdef EESTOREDEBUG - DIAG(F("Turnout::activate(%d)"),state); -#endif - if (data.type == TURNOUT_LCN) { - // A LCN turnout is transmitted to the LCN master. - LCN::send('T', data.id, state); - return; // The tStatus will be updated by a message from the LCN master, later. - } - data.active = state; - switch (data.type) { - case TURNOUT_DCC: - DCC::setAccessory((((data.dccAccessoryData.address-1) >> 2) + 1), - ((data.dccAccessoryData.address-1) & 3), state); - break; - case TURNOUT_SERVO: -#ifndef IO_NO_HAL - IODevice::write(data.servoData.vpin, state); -#endif - break; - case TURNOUT_VPIN: - IODevice::write(data.vpinData.vpin, state); - break; - } - // Save state if stored in EEPROM - if (EEStore::eeStore->data.nTurnouts > 0 && num > 0) - EEPROM.put(num, data.tStatus); - -#if defined(RMFT_ACTIVE) - RMFT2::turnoutEvent(data.id, !state); -#endif - -} - -/////////////////////////////////////////////////////////////////////////////// -// Static function to find Turnout object specified by ID 'n'. Return NULL if not found. - -Turnout* Turnout::get(int n){ - Turnout *tt; - for(tt=firstTurnout;tt!=NULL && tt->data.id!=n;tt=tt->nextTurnout); - return(tt); -} - -/////////////////////////////////////////////////////////////////////////////// -// Static function to delete Turnout object specified by ID 'n'. Return false if not found. - -bool Turnout::remove(int n){ - Turnout *tt,*pp=NULL; - - for(tt=firstTurnout;tt!=NULL && tt->data.id!=n;pp=tt,tt=tt->nextTurnout); - - if(tt==NULL) return false; - - if(tt==firstTurnout) - firstTurnout=tt->nextTurnout; - else - pp->nextTurnout=tt->nextTurnout; - - free(tt); - turnoutlistHash++; - return true; -} - -/////////////////////////////////////////////////////////////////////////////// -// Static function to load all Turnout definitions from EEPROM -// TODO: Consider transmitting the initial state of the DCC/LCN turnout here. -// (already done for servo turnouts and VPIN turnouts). - -void Turnout::load(){ - struct TurnoutData data; - Turnout *tt=NULL; - - for(uint16_t i=0;idata.nTurnouts;i++){ - // Retrieve data - EEPROM.get(EEStore::pointer(), data); - - int lastKnownState = data.active; - switch (data.type) { - case TURNOUT_DCC: - tt=createDCC(data.id, ((data.dccAccessoryData.address-1)>>2)+1, (data.dccAccessoryData.address-1)&3); // DCC-based turnout - break; - case TURNOUT_LCN: - // LCN turnouts are created when the remote device sends a message. - break; - case TURNOUT_SERVO: - tt=createServo(data.id, data.servoData.vpin, - data.servoData.activePosition, data.servoData.inactivePosition, data.servoData.profile, lastKnownState); - break; - case TURNOUT_VPIN: - tt=createVpin(data.id, data.vpinData.vpin, lastKnownState); // VPIN-based turnout - break; - - default: - tt=NULL; - } - if (tt) tt->num = EEStore::pointer() + offsetof(TurnoutData, tStatus); // Save pointer to tStatus byte within EEPROM - // Advance by the actual size of the individual turnout struct. - EEStore::advance(data.size); -#ifdef EESTOREDEBUG - if (tt) print(tt); -#endif - } -} - -/////////////////////////////////////////////////////////////////////////////// -// Static function to store all Turnout definitions to EEPROM - -void Turnout::store(){ - Turnout *tt; - - tt=firstTurnout; - EEStore::eeStore->data.nTurnouts=0; - - while(tt!=NULL){ - // LCN turnouts aren't saved to EEPROM - if (tt->data.type != TURNOUT_LCN) { -#ifdef EESTOREDEBUG - print(tt); -#endif - tt->num = EEStore::pointer() + offsetof(TurnoutData, tStatus); // Save pointer to tstatus byte within EEPROM - EEPROM.put(EEStore::pointer(),tt->data); - EEStore::advance(tt->data.size); - EEStore::eeStore->data.nTurnouts++; - } - tt=tt->nextTurnout; - } -} - -/////////////////////////////////////////////////////////////////////////////// -// Static function for creating a DCC-controlled Turnout. - -Turnout *Turnout::createDCC(int id, uint16_t add, uint8_t subAdd){ - if (add > 511 || subAdd > 3) return NULL; - Turnout *tt=create(id); - if (!tt) return(tt); - tt->data.type = TURNOUT_DCC; - tt->data.size = sizeof(tt->data.header) + sizeof(tt->data.dccAccessoryData); - tt->data.active = 0; - tt->data.dccAccessoryData.address = ((add-1) << 2) + subAdd + 1; - return(tt); -} - -/////////////////////////////////////////////////////////////////////////////// -// Static function for creating a LCN-controlled Turnout. - -Turnout *Turnout::createLCN(int id, uint8_t state) { - Turnout *tt=create(id); - if (!tt) return(tt); - tt->data.type = TURNOUT_LCN; - tt->data.size = sizeof(tt->data.header) + sizeof(tt->data.lcnData); - tt->data.active = (state != 0); - return(tt); -} - -/////////////////////////////////////////////////////////////////////////////// -// Static function for associating a Turnout id with a virtual pin in IODevice space. -// The actual creation and configuration of the pin must be done elsewhere, -// e.g. in mySetup.cpp during startup of the CS. - -Turnout *Turnout::createVpin(int id, VPIN vpin, uint8_t state){ - if (vpin > VPIN_MAX) return NULL; - Turnout *tt=create(id); - if(!tt) return(tt); - tt->data.type = TURNOUT_VPIN;; - tt->data.size = sizeof(tt->data.header) + sizeof(tt->data.vpinData); - tt->data.active = (state != 0); - tt->data.vpinData.vpin = vpin; - IODevice::write(vpin, state); // Set initial state of output. - return(tt); -} - -/////////////////////////////////////////////////////////////////////////////// -// Method for creating a Servo Turnout, e.g. connected to PCA9685 PWM device. - -Turnout *Turnout::createServo(int id, VPIN vpin, uint16_t activePosition, uint16_t inactivePosition, uint8_t profile, uint8_t state){ -#ifndef IO_NO_HAL - if (activePosition > 511 || inactivePosition > 511 || profile > 4) return NULL; - - Turnout *tt=create(id); - if (!tt) return(tt); - if (tt->data.type != TURNOUT_SERVO) tt->data.active = (state != 0); // Retain current state if it's an existing servo turnout. - tt->data.type = TURNOUT_SERVO; - tt->data.size = sizeof(tt->data.header) + sizeof(tt->data.servoData); - tt->data.servoData.vpin = vpin; - tt->data.servoData.activePosition = activePosition; - tt->data.servoData.inactivePosition = inactivePosition; - tt->data.servoData.profile = profile; - // Configure PWM interface device - int deviceParams[] = {(int)activePosition, (int)inactivePosition, profile, tt->data.active}; - if (!IODevice::configure(vpin, IODevice::CONFIGURE_SERVO, 4, deviceParams)) { - remove(id); + Turnout *Turnout::get(uint16_t id) { + // Find turnout object from list. + for (Turnout *tt = _firstTurnout; tt != NULL; tt = tt->_nextTurnout) + if (tt->_turnoutData.id == id) return tt; return NULL; } - return(tt); -#else - (void)id; (void)vpin; (void)activePosition; (void)inactivePosition; (void)profile; (void)state; // avoid compiler warnings - return NULL; -#endif -} -/////////////////////////////////////////////////////////////////////////////// -// Support for -// and -// and + // Add new turnout to end of chain + void Turnout::add(Turnout *tt) { + if (!_firstTurnout) + _firstTurnout = tt; + else { + // Find last object on chain + Turnout *ptr = _firstTurnout; + for ( ; ptr->_nextTurnout!=0; ptr=ptr->_nextTurnout) {} + // Line new object to last object. + ptr->_nextTurnout = tt; + } + turnoutlistHash++; + } + + // Remove nominated turnout from turnout linked list and delete the object. + bool Turnout::remove(uint16_t id) { + Turnout *tt,*pp=NULL; -Turnout *Turnout::create(int id, int params, int16_t p[]) { - if (p[0] == HASH_KEYWORD_SERVO) { // - if (params == 5) - return createServo(id, (VPIN)p[1], (uint16_t)p[2], (uint16_t)p[3], (uint8_t)p[4]); - else - return NULL; - } else - if (p[0] == HASH_KEYWORD_VPIN) { // - if (params==2) - return createVpin(id, p[1]); + for(tt=_firstTurnout; tt!=NULL && tt->_turnoutData.id!=id; pp=tt, tt=tt->_nextTurnout) {} + if (tt == NULL) return false; + + if (tt == _firstTurnout) + _firstTurnout = tt->_nextTurnout; else - return NULL; - } else - if (p[0]==HASH_KEYWORD_DCC) { - if (params==3 && p[1]>0 && p[1]<=512 && p[2]>=0 && p[2]<4) // - return createDCC(id, p[1], p[2]); - else if (params==2 && p[1]>0 && p[1]<=512*4) // - return createDCC(id, (p[1]-1)/4+1, (p[1]-1)%4); - else - return NULL; - } else if (params==2) { // for DCC or LCN - return createDCC(id, p[0], p[1]); + pp->_nextTurnout = tt->_nextTurnout; + + delete (ServoTurnout *)tt; + + turnoutlistHash++; + return true; } - else if (params==3) { // legacy for Servo - return createServo(id, (VPIN)p[0], (uint16_t)p[1], (uint16_t)p[2]); + + + /* + * Public static functions + */ + + bool Turnout::isClosed(uint16_t id) { + Turnout *tt = get(id); + if (tt) + return tt->isClosed(); + else + return false; } - return NULL; -} + // Static activate function is invoked from close(), throw() etc. to perform the + // common parts of the turnout operation. Code which is specific to a turnout + // type should be placed in the polymorphic virtual function activate(bool) which is + // called from here. + bool Turnout::activate(uint16_t id, bool closeFlag) { + #ifdef EESTOREDEBUG + if (closeFlag) + DIAG(F("Turnout::close(%d)"), id); + else + DIAG(F("Turnout::throw(%d)"), id); + #endif + Turnout *tt = Turnout::get(id); + if (!tt) return false; + bool ok = tt->activate(closeFlag); -/////////////////////////////////////////////////////////////////////////////// -// Create basic Turnout object. The details of what sort of object it is -// controlling are not set here. + // Write new closed/thrown state to EEPROM if required. Note that eepromAddress + // is always zero for LCN turnouts. + if (EEStore::eeStore->data.nTurnouts > 0 && tt->_eepromAddress) + EEPROM.put(tt->_eepromAddress, tt->_turnoutData.closed); -Turnout *Turnout::create(int id){ - Turnout *tt=get(id); - if (tt==NULL) { - tt=(Turnout *)calloc(1,sizeof(Turnout)); - if (!tt) return (tt); - tt->nextTurnout=firstTurnout; - firstTurnout=tt; - tt->data.id=id; + #if defined(RMFT_ACTIVE) + // TODO: Check that the inversion is correct here! + RMFT2::turnoutEvent(id, !closeFlag); + #endif + + return ok; } - turnoutlistHash++; - return tt; -} -/////////////////////////////////////////////////////////////////////////////// -// -// Object method to print debug info about the state of a Turnout object -// + // Load all turnout objects + void Turnout::load() { + for (uint16_t i=0; idata.nTurnouts; i++) { + Turnout::loadTurnout(); + } + } + + // Save all turnout objects + void Turnout::store() { + EEStore::eeStore->data.nTurnouts=0; + for (Turnout *tt = _firstTurnout; tt != 0; tt = tt->_nextTurnout) { + tt->save(); + EEStore::eeStore->data.nTurnouts++; + } + } + + // Load one turnout from EEPROM + Turnout *Turnout::loadTurnout () { + Turnout *tt; + // Read turnout type from EEPROM + struct TurnoutData turnoutData; + int eepromAddress = EEStore::pointer(); // Address of byte containing the _closed flag. + EEPROM.get(EEStore::pointer(), turnoutData); + EEStore::advance(sizeof(turnoutData)); + + switch (turnoutData.turnoutType) { + case TURNOUT_SERVO: + // Servo turnout + tt = ServoTurnout::load(&turnoutData); + break; + case TURNOUT_DCC: + // DCC Accessory turnout + tt = DCCTurnout::load(&turnoutData); + break; + case TURNOUT_VPIN: + // VPIN turnout + tt = VpinTurnout::load(&turnoutData); + break; + } + if (!tt) { + // Save EEPROM address in object. Note that LCN turnouts always have eepromAddress of zero. + tt->_eepromAddress = eepromAddress; + add(tt); + } + #ifdef EESTOREDEBUG -void Turnout::print(Turnout *tt) { - tt->print(StringFormatter::diagSerial); -} + printAll(&Serial); #endif + return tt; + } -/////////////////////////////////////////////////////////////////////////////// -Turnout *Turnout::firstTurnout=NULL; -int Turnout::turnoutlistHash=0; //bump on every change so clients know when to refresh their lists diff --git a/Turnouts.h b/Turnouts.h index db73214..152583e 100644 --- a/Turnouts.h +++ b/Turnouts.h @@ -1,4 +1,6 @@ /* + * © 2021 Restructured Neil McKechnie + * © 2013-2016 Gregg E. Berman * © 2020, Chris Harlow. All rights reserved. * * This file is part of Asbelos DCC API @@ -17,109 +19,464 @@ * along with CommandStation. If not, see . */ -/* - * Turnout data is stored in a structure whose length depends on the - * type of turnout. There is a common header of 3 bytes, followed by - * 2 bytes for DCC turnout, 5 bytes for servo turnout, 2 bytes for a - * VPIN turnout, or zero bytes for an LCN turnout. - * The variable length allows the limited space in EEPROM to be used effectively. - */ -#ifndef Turnouts_h -#define Turnouts_h +//#define EESTOREDEBUG +#include "defines.h" +#include "EEStore.h" +#include "StringFormatter.h" +#include "RMFT2.h" +#ifdef EESTOREDEBUG +#include "DIAG.h" +#endif -#include #include "DCC.h" #include "LCN.h" -#include "IODevice.h" - -const byte STATUS_ACTIVE=0x80; // Flag as activated in tStatus field -const byte STATUS_TYPE = 0x7f; // Mask for turnout type in tStatus field - -// The struct 'header' is used to determine the length of the -// overlaid data so must be at least as long as the anonymous fields it -// is overlaid with. -struct TurnoutData { - // Header common to all turnouts - union { - struct { - int id; - uint8_t tStatus; - uint8_t size; - } header; - - struct { - int id; - union { - uint8_t tStatus; - struct { - uint8_t active: 1; - uint8_t type: 5; - uint8_t :2; - }; - }; - uint8_t size; // set to actual total length of used structure - }; - }; - // Turnout-type-specific structure elements, different length depending - // on turnout type. This allows the data to be packed efficiently - // in the EEPROM. - union { - struct { - // DCC address (Address in bits 15-2, subaddress in bits 1-0 - uint16_t address; // CS currently supports linear address 1-2048 - // That's DCC accessory address 1-512 and subaddress 0-3. - } dccAccessoryData; - - struct { - VPIN vpin; - uint16_t activePosition : 12; // 0-4095 - uint16_t inactivePosition : 12; // 0-4095 - uint8_t profile; - } servoData; - - struct { - } lcnData; - - struct { - VPIN vpin; - } vpinData; - }; +// Turnout type definitions +enum { + TURNOUT_DCC = 1, + TURNOUT_SERVO = 2, + TURNOUT_VPIN = 3, + TURNOUT_LCN = 4, }; +/************************************************************************************* + * Turnout - Base class for turnouts. + * + *************************************************************************************/ + class Turnout { -public: - static Turnout *firstTurnout; - static int turnoutlistHash; - Turnout *nextTurnout; - static Turnout* get(int); - static bool remove(int); - static bool isClosed(int); - static bool setClosed(int n, bool closed); // return false if not found. - static void setClosedStateOnly(int n, bool closed); - int getId(); - static void load(); - static void store(); - static Turnout *createServo(int id , VPIN vpin , uint16_t activeAngle, uint16_t inactiveAngle, uint8_t profile=1, uint8_t initialState=0); - static Turnout *createVpin(int id, VPIN vpin, uint8_t initialState=0); - static Turnout *createDCC(int id, uint16_t address, uint8_t subAddress); - static Turnout *createLCN(int id, uint8_t initialState=0); - static Turnout *create(int id, int params, int16_t p[]); - static Turnout *create(int id); - static void printAll(Print *); - void print(Print *stream); -#ifdef EESTOREDEBUG - static void print(Turnout *tt); -#endif -private: - int num; // EEPROM address of tStatus in TurnoutData struct, or zero if not stored. - TurnoutData data; - static bool activate(int n, bool thrown); - static bool isActive(int); - bool isActive(); - void activate(bool state); - void setActive(bool state); - }; // Turnout +protected: + /* + * Object data + */ + + // The TurnoutData struct contains data common to all turnout types, that + // is written to EEPROM when the turnout is saved. + // The first byte of this struct contains the 'closed' flag which is + // updated whenever the turnout changes from thrown to closed and + // vice versa. If the turnout has been saved, then this byte is rewritten + // when changed in RAM. The 'closed' flag must be located in the first byte. + struct TurnoutData { + bool closed : 1; + bool _rfu: 2; + uint8_t turnoutType : 5; + uint16_t id; + } _turnoutData; // 3 bytes + + // Address in eeprom of first byte of the _turnoutData struct (containing the closed flag). + // Set to zero if the object has not been saved in EEPROM, e.g. for newly created Turnouts, and + // for all LCN turnouts. + uint16_t _eepromAddress = 0; + + // Pointer to next turnout on linked list. + Turnout *_nextTurnout = 0; + + /* + * Constructor + */ + Turnout(uint16_t id, uint8_t turnoutType, bool closed) { + _turnoutData.id = id; + _turnoutData.turnoutType = turnoutType; + _turnoutData.closed = closed; + add(this); + } + + /* + * Static data + */ + + static Turnout *_firstTurnout; + static int _turnoutlistHash; + + /* + * Virtual functions + */ + + virtual bool activate(bool close) = 0; // Mandatory in subclass + virtual void save() {} + /* + * Static functions + */ + + static Turnout *get(uint16_t id); + + static void add(Turnout *tt); + +public: + /* + * Static data + */ + static int turnoutlistHash; + + /* + * Public base class functions + */ + inline bool isClosed() { return _turnoutData.closed; }; + inline bool isThrown() { return !_turnoutData.closed; } + inline bool isType(uint8_t type) { return _turnoutData.turnoutType == type; } + inline uint16_t getId() { return _turnoutData.id; } + inline Turnout *next() { return _nextTurnout; } + /* + * Virtual functions + */ + virtual void print(Print *stream) {} + virtual ~Turnout() {} // Destructor + + /* + * Public static functions + */ + inline static bool exists(uint16_t id) { return get(id) != 0; } + + static bool remove(uint16_t id); + + static bool isClosed(uint16_t id); + + inline static bool isThrown(uint16_t id) { + return !isClosed(id); + } + + static bool activate(uint16_t id, bool closeFlag); + + inline static bool setClosed(uint16_t id) { + return activate(id, true); + } + + inline static bool setThrown(uint16_t id) { + return activate(id, false); + } + + inline static bool setClosed(uint16_t id, bool close) { + return activate(id, close); + } + + static bool setClosedStateOnly(uint16_t id, bool close) { + Turnout *tt = get(id); + if (tt) return false; + tt->_turnoutData.closed = close; + return true; + } + + inline static Turnout *first() { return _firstTurnout; } + + // Load all turnout definitions. + static void load(); + // Load one turnout definition + static Turnout *loadTurnout(); + // Save all turnout definitions + static void store(); + + static void printAll(Print *stream) { + for (Turnout *tt = _firstTurnout; tt != 0; tt = tt->_nextTurnout) + tt->print(stream); + } +}; + + +/************************************************************************************* + * ServoTurnout - Turnout controlled by servo device. + * + *************************************************************************************/ +class ServoTurnout : public Turnout { +private: + // ServoTurnoutData contains data specific to this subclass that is + // written to EEPROM when the turnout is saved. + struct ServoTurnoutData { + VPIN vpin; + uint16_t closedPosition : 12; + uint16_t thrownPosition : 12; + uint8_t profile; + } _servoTurnoutData; // 6 bytes + +public: + // Constructor + ServoTurnout(uint16_t id, VPIN vpin, uint16_t thrownPosition, uint16_t closedPosition, uint8_t profile, bool closed = true) : + Turnout(id, TURNOUT_SERVO, closed) + { + _servoTurnoutData.vpin = vpin; + _servoTurnoutData.thrownPosition = thrownPosition; + _servoTurnoutData.closedPosition = closedPosition; + _servoTurnoutData.profile = profile; + } + + // Create function + static Turnout *create(uint16_t id, VPIN vpin, uint16_t thrownPosition, uint16_t closedPosition, uint8_t profile, bool closed = true) { +#ifndef IO_NO_HAL + Turnout *tt = get(id); + if (!tt) { + // Object already exists, check if it is usable + if (tt->isType(TURNOUT_SERVO)) { + // Yes, so set parameters + ServoTurnout *st = (ServoTurnout *)tt; + st->_servoTurnoutData.vpin = vpin; + st->_servoTurnoutData.thrownPosition = thrownPosition; + st->_servoTurnoutData.closedPosition = closedPosition; + st->_servoTurnoutData.profile = profile; + // Don't touch the _closed parameter, retain the original value. + + // We don't really need to do the following, since a call to IODevice::_writeAnalogue + // will provide all the data that is required! + // int params[] = {(int)thrownPosition, (int)closedPosition, profile, closed}; + // IODevice::configure(vpin, IODevice::CONFIGURE_SERVO, 4, params); + + // Set position to saved position + IODevice::writeAnalogue(vpin, closed ? closedPosition : thrownPosition, PCA9685::Instant); + + return tt; + } else { + // Incompatible object, delete and recreate + remove(id); + } + } + tt = (Turnout *)new ServoTurnout(id, vpin, thrownPosition, closedPosition, profile, closed); + IODevice::writeAnalogue(vpin, closed ? closedPosition : thrownPosition, PCA9685::Instant); + return tt; +#else + return NULL; #endif + } + + bool activate(bool close) override { +#ifndef IO_NO_HAL + IODevice::writeAnalogue(_servoTurnoutData.vpin, + close ? _servoTurnoutData.closedPosition : _servoTurnoutData.thrownPosition, _servoTurnoutData.profile); + _turnoutData.closed = close; +#endif + return true; + } + + void save() override { + // Write turnout definition and current position to EEPROM + // First write common servo data, then + // write the servo-specific data + EEPROM.put(EEStore::pointer(), _turnoutData); + EEStore::advance(sizeof(_turnoutData)); + EEPROM.put(EEStore::pointer(), _servoTurnoutData); + EEStore::advance(sizeof(_servoTurnoutData)); + } + + void print(Print *stream) override { + StringFormatter::send(stream, F("\n"), _turnoutData.id, _servoTurnoutData.vpin, + _servoTurnoutData.thrownPosition, _servoTurnoutData.closedPosition, _servoTurnoutData.profile, _turnoutData.closed); + } + + // Load a Servo turnout definition from EEPROM. The common Turnout data has already been read at this point. + static Turnout *load(struct TurnoutData *turnoutData) { + ServoTurnoutData servoTurnoutData; + // Read class-specific data from EEPROM + EEPROM.get(EEStore::pointer(), servoTurnoutData); + EEStore::advance(sizeof(servoTurnoutData)); + + // Create new object + ServoTurnout *tt = new ServoTurnout(turnoutData->id, servoTurnoutData.vpin, servoTurnoutData.thrownPosition, + servoTurnoutData.closedPosition, servoTurnoutData.profile, turnoutData->closed); + + return tt; + } +}; + +/************************************************************************************* + * DCCTurnout - Turnout controlled by DCC Accessory Controller. + * + *************************************************************************************/ +class DCCTurnout : public Turnout { +private: + // DCCTurnoutData contains data specific to this subclass that is + // written to EEPROM when the turnout is saved. + struct DCCTurnoutData { + // DCC address (Address in bits 15-2, subaddress in bits 1-0 + uint16_t address; // CS currently supports linear address 1-2048 + // That's DCC accessory address 1-512 and subaddress 0-3. + } _dccTurnoutData; // 2 bytes + +public: + // Constructor + DCCTurnout(uint16_t id, uint16_t address, uint8_t subAdd) : + Turnout(id, TURNOUT_DCC, false) + { + _dccTurnoutData.address = ((address-1) << 2) + subAdd + 1; + } + + // Create function + static Turnout *create(uint16_t id, uint16_t add, uint8_t subAdd) { + Turnout *tt = get(id); + if (!tt) { + // Object already exists, check if it is usable + if (tt->isType(TURNOUT_DCC)) { + // Yes, so set parameters + DCCTurnout *dt = (DCCTurnout *)tt; + dt->_dccTurnoutData.address = ((add-1) << 2) + subAdd + 1; + // Don't touch the _closed parameter, retain the original value. + return tt; + } else { + // Incompatible object, delete and recreate + remove(id); + } + } + tt = (Turnout *)new DCCTurnout(id, add, subAdd); + return tt; + } + + bool activate(bool close) override { + DCC::setAccessory((((_dccTurnoutData.address-1) >> 2) + 1), + ((_dccTurnoutData.address-1) & 3), close); + _turnoutData.closed = close; + return true; + } + + void save() override { + // Write turnout definition and current position to EEPROM + // First write common servo data, then + // write the servo-specific data + EEPROM.put(EEStore::pointer(), _turnoutData); + EEStore::advance(sizeof(_turnoutData)); + EEPROM.put(EEStore::pointer(), _dccTurnoutData); + EEStore::advance(sizeof(_dccTurnoutData)); + } + + void print(Print *stream) override { + StringFormatter::send(stream, F("\n"), _turnoutData.id, + (((_dccTurnoutData.address-1) >> 2)+1), ((_dccTurnoutData.address-1) & 3), _turnoutData.closed); + } + + // Load a DCC turnout definition from EEPROM. The common Turnout data has already been read at this point. + static Turnout *load(struct TurnoutData *turnoutData) { + DCCTurnoutData dccTurnoutData; + // Read class-specific data from EEPROM + EEPROM.get(EEStore::pointer(), dccTurnoutData); + EEStore::advance(sizeof(dccTurnoutData)); + + // Create new object + DCCTurnout *tt = new DCCTurnout(turnoutData->id, (((dccTurnoutData.address-1) >> 2)+1), ((dccTurnoutData.address-1) & 3)); + + return tt; + } +}; + + +/************************************************************************************* + * VpinTurnout - Turnout controlled through a HAL vpin. + * + *************************************************************************************/ +class VpinTurnout : public Turnout { +private: + // VpinTurnoutData contains data specific to this subclass that is + // written to EEPROM when the turnout is saved. + struct VpinTurnoutData { + VPIN vpin; + } _vpinTurnoutData; // 2 bytes + +public: + // Constructor + VpinTurnout(uint16_t id, VPIN vpin, bool closed=true) : + Turnout(id, TURNOUT_VPIN, closed) + { + _vpinTurnoutData.vpin = vpin; + } + + // Create function + static Turnout *create(uint16_t id, VPIN vpin, bool closed=true) { + Turnout *tt = get(id); + if (!tt) { + // Object already exists, check if it is usable + if (tt->isType(TURNOUT_VPIN)) { + // Yes, so set parameters + VpinTurnout *vt = (VpinTurnout *)tt; + vt->_vpinTurnoutData.vpin = vpin; + // Don't touch the _closed parameter, retain the original value. + return tt; + } else { + // Incompatible object, delete and recreate + remove(id); + } + } + tt = (Turnout *)new VpinTurnout(id, vpin, closed); + return tt; + } + + bool activate(bool close) override { + IODevice::write(_vpinTurnoutData.vpin, close); + _turnoutData.closed = close; + return true; + } + + void save() override { + // Write turnout definition and current position to EEPROM + // First write common servo data, then + // write the servo-specific data + EEPROM.put(EEStore::pointer(), _turnoutData); + EEStore::advance(sizeof(_turnoutData)); + EEPROM.put(EEStore::pointer(), _vpinTurnoutData); + EEStore::advance(sizeof(_vpinTurnoutData)); + } + + void print(Print *stream) override { + StringFormatter::send(stream, F("\n"), _turnoutData.id, + _vpinTurnoutData.vpin, _turnoutData.closed); + } + + // Load a VPIN turnout definition from EEPROM. The common Turnout data has already been read at this point. + static Turnout *load(struct TurnoutData *turnoutData) { + VpinTurnoutData vpinTurnoutData; + // Read class-specific data from EEPROM + EEPROM.get(EEStore::pointer(), vpinTurnoutData); + EEStore::advance(sizeof(vpinTurnoutData)); + + // Create new object + VpinTurnout *tt = new VpinTurnout(turnoutData->id, vpinTurnoutData.vpin, turnoutData->closed); + + return tt; + } +}; + + +/************************************************************************************* + * LCNTurnout - Turnout controlled by Loconet + * + *************************************************************************************/ +class LCNTurnout : public Turnout { +private: + // LCNTurnout has no specific data, and in any case is not written to EEPROM! + // struct LCNTurnoutData { + // } _lcnTurnoutData; // 0 bytes + +public: + // Constructor + LCNTurnout(uint16_t id, bool closed=true) : + Turnout(id, TURNOUT_LCN, closed) + { } + + // Create function + static Turnout *create(uint16_t id, bool closed=true) { + Turnout *tt = get(id); + if (!tt) { + // Object already exists, check if it is usable + if (tt->isType(TURNOUT_LCN)) { + // Yes, so return this object + return tt; + } else { + // Incompatible object, delete and recreate + remove(id); + } + } + tt = (Turnout *)new LCNTurnout(id, closed); + return tt; + } + + bool activate(bool close) override { + LCN::send('T', _turnoutData.id, close); + // The _turnoutData.closed flag should be updated by a message from the LCN master, later. + return true; + } + + // LCN turnouts not saved to EEPROM. + //void save() override { } + //static Turnout *load(struct TurnoutData *turnoutData) { + + void print(Print *stream) override { + StringFormatter::send(stream, F("\n"), _turnoutData.id, _turnoutData.closed); + } + +}; + diff --git a/WiThrottle.cpp b/WiThrottle.cpp index e9cfdfd..07017ba 100644 --- a/WiThrottle.cpp +++ b/WiThrottle.cpp @@ -120,7 +120,7 @@ void WiThrottle::parse(RingStream * stream, byte * cmdx) { // Send turnout list if changed since last sent (will replace list on client) if (turnoutListHash != Turnout::turnoutlistHash) { StringFormatter::send(stream,F("PTL")); - for(Turnout *tt=Turnout::firstTurnout;tt!=NULL;tt=tt->nextTurnout){ + for(Turnout *tt=Turnout::first();tt!=NULL;tt=tt->next()){ int id=tt->getId(); StringFormatter::send(stream,F("]\\[%d}|{%d}|{%c"), id, id, Turnout::isClosed(id)?'2':'4'); } @@ -161,12 +161,11 @@ void WiThrottle::parse(RingStream * stream, byte * cmdx) { #endif else if (cmd[1]=='T' && cmd[2]=='A') { // PTA accessory toggle int id=getInt(cmd+4); - Turnout * tt=Turnout::get(id); - if (!tt) { + if (!Turnout::exists(id)) { // If turnout does not exist, create it int addr = ((id - 1) / 4) + 1; int subaddr = (id - 1) % 4; - Turnout::createDCC(id,addr,subaddr); + DCCTurnout::create(id,addr,subaddr); StringFormatter::send(stream, F("HmTurnout %d created\n"),id); } switch (cmd[3]) { From 7f6173825fa264b82d87e430adcbd27018278278 Mon Sep 17 00:00:00 2001 From: Neil McKechnie Date: Thu, 19 Aug 2021 21:43:55 +0100 Subject: [PATCH 03/11] Various corrections to Turnout code. --- DCCEXParser.cpp | 2 +- Turnouts.cpp | 14 ++++++++------ Turnouts.h | 9 +++++---- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/DCCEXParser.cpp b/DCCEXParser.cpp index 4a27514..388cf60 100644 --- a/DCCEXParser.cpp +++ b/DCCEXParser.cpp @@ -710,7 +710,7 @@ bool DCCEXParser::parseT(Print *stream, int16_t params, int16_t p[]) } else if (p[1] == HASH_KEYWORD_VPIN) { // if (params==3) { - if (VpinTurnout::create(p[0], p[2])) return false; + if (!VpinTurnout::create(p[0], p[2])) return false; } else return false; } else diff --git a/Turnouts.cpp b/Turnouts.cpp index bd5fd5b..0869a58 100644 --- a/Turnouts.cpp +++ b/Turnouts.cpp @@ -105,7 +105,7 @@ const int16_t HASH_KEYWORD_VPIN=-415; // Static activate function is invoked from close(), throw() etc. to perform the // common parts of the turnout operation. Code which is specific to a turnout - // type should be placed in the polymorphic virtual function activate(bool) which is + // type should be placed in the virtual function activate(bool) which is // called from here. bool Turnout::activate(uint16_t id, bool closeFlag) { #ifdef EESTOREDEBUG @@ -124,8 +124,7 @@ const int16_t HASH_KEYWORD_VPIN=-415; EEPROM.put(tt->_eepromAddress, tt->_turnoutData.closed); #if defined(RMFT_ACTIVE) - // TODO: Check that the inversion is correct here! - RMFT2::turnoutEvent(id, !closeFlag); + RMFT2::turnoutEvent(id, closeFlag); #endif return ok; @@ -149,10 +148,10 @@ const int16_t HASH_KEYWORD_VPIN=-415; // Load one turnout from EEPROM Turnout *Turnout::loadTurnout () { - Turnout *tt; + Turnout *tt = 0; // Read turnout type from EEPROM struct TurnoutData turnoutData; - int eepromAddress = EEStore::pointer(); // Address of byte containing the _closed flag. + int eepromAddress = EEStore::pointer(); // Address of byte containing the closed flag. EEPROM.get(EEStore::pointer(), turnoutData); EEStore::advance(sizeof(turnoutData)); @@ -169,11 +168,14 @@ const int16_t HASH_KEYWORD_VPIN=-415; // VPIN turnout tt = VpinTurnout::load(&turnoutData); break; + default: + // If we find anything else, then we don't know what it is or how long it is, + // so we can't go any further through the EEPROM! + return NULL; } if (!tt) { // Save EEPROM address in object. Note that LCN turnouts always have eepromAddress of zero. tt->_eepromAddress = eepromAddress; - add(tt); } #ifdef EESTOREDEBUG diff --git a/Turnouts.h b/Turnouts.h index 152583e..6f6c408 100644 --- a/Turnouts.h +++ b/Turnouts.h @@ -204,7 +204,7 @@ public: static Turnout *create(uint16_t id, VPIN vpin, uint16_t thrownPosition, uint16_t closedPosition, uint8_t profile, bool closed = true) { #ifndef IO_NO_HAL Turnout *tt = get(id); - if (!tt) { + if (tt) { // Object already exists, check if it is usable if (tt->isType(TURNOUT_SERVO)) { // Yes, so set parameters @@ -237,6 +237,7 @@ public: #endif } + // ServoTurnout-specific code for throwing or closing a servo turnout. bool activate(bool close) override { #ifndef IO_NO_HAL IODevice::writeAnalogue(_servoTurnoutData.vpin, @@ -301,7 +302,7 @@ public: // Create function static Turnout *create(uint16_t id, uint16_t add, uint8_t subAdd) { Turnout *tt = get(id); - if (!tt) { + if (tt) { // Object already exists, check if it is usable if (tt->isType(TURNOUT_DCC)) { // Yes, so set parameters @@ -378,7 +379,7 @@ public: // Create function static Turnout *create(uint16_t id, VPIN vpin, bool closed=true) { Turnout *tt = get(id); - if (!tt) { + if (tt) { // Object already exists, check if it is usable if (tt->isType(TURNOUT_VPIN)) { // Yes, so set parameters @@ -450,7 +451,7 @@ public: // Create function static Turnout *create(uint16_t id, bool closed=true) { Turnout *tt = get(id); - if (!tt) { + if (tt) { // Object already exists, check if it is usable if (tt->isType(TURNOUT_LCN)) { // Yes, so return this object From b4a3b503bcc3f6c5b6dd4ac3eeec5e8e65feb616 Mon Sep 17 00:00:00 2001 From: Neil McKechnie Date: Fri, 20 Aug 2021 00:07:50 +0100 Subject: [PATCH 04/11] Turnout notification handling enhanced. Ensure that the message is sent on the serial USB (to JMRI) whenever the turnout is closed or thrown, even if the request didn't originate on the serial USB. --- DCCEXParser.cpp | 10 +++++++--- Turnouts.cpp | 7 ++++++- Turnouts.h | 3 ++- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/DCCEXParser.cpp b/DCCEXParser.cpp index 388cf60..6779178 100644 --- a/DCCEXParser.cpp +++ b/DCCEXParser.cpp @@ -679,25 +679,29 @@ bool DCCEXParser::parseT(Print *stream, int16_t params, int16_t p[]) // turnout 1 or T=THROW, 0 or C=CLOSE case 1: case 0x54: // 1 or T if (!Turnout::setClosed(p[0], false)) return false; + p[1] = 1; break; case 0: case 0x43: // 0 or C if (!Turnout::setClosed(p[0], true)) return false; + p[1] = 0; break; #else // turnout 0 or T=THROW,1 or C=CLOSE case 0: case 0x54: // 0 or T if (!Turnout::setClosed(p[0], false)) return false; + p[1] = 0; break; case 1: case 0x43: // 1 or C if (!Turnout::setClosed(p[0], true)) return false; + p[1] = 1; break; #endif default: return false; } - // Send acknowledgement to caller, and to Serial. - StringFormatter::send(stream, F("\n"), p[0], p[1]); - if (stream != &Serial) StringFormatter::send(Serial, F("\n"), p[0], p[1]); + // Send acknowledgement to caller if the command was not received over Serial (acknowledgement + // messages on Serial are sent by the Turnout class). + if (stream != &Serial) StringFormatter::send(stream, F("\n"), p[0], p[1]); return true; default: // Anything else is some kind of create function. diff --git a/Turnouts.cpp b/Turnouts.cpp index 0869a58..8a77f57 100644 --- a/Turnouts.cpp +++ b/Turnouts.cpp @@ -125,7 +125,12 @@ const int16_t HASH_KEYWORD_VPIN=-415; #if defined(RMFT_ACTIVE) RMFT2::turnoutEvent(id, closeFlag); - #endif + #endif + + // Send message to JMRI etc. over Serial USB. This is done here + // to ensure that the message is sent when the turnout operation + // is not initiated by a Serial command. + StringFormatter::send(Serial, F("\n"), id, closeFlag); return ok; } diff --git a/Turnouts.h b/Turnouts.h index 6f6c408..6659c24 100644 --- a/Turnouts.h +++ b/Turnouts.h @@ -466,7 +466,8 @@ public: } bool activate(bool close) override { - LCN::send('T', _turnoutData.id, close); + // Assume that the LCN command still uses 1 for throw and 0 for close... + LCN::send('T', _turnoutData.id, !close); // The _turnoutData.closed flag should be updated by a message from the LCN master, later. return true; } From 482f4b1c79539d5d3615f00eb1979184888855be Mon Sep 17 00:00:00 2001 From: Neil McKechnie Date: Fri, 20 Aug 2021 14:36:18 +0100 Subject: [PATCH 05/11] Tidy up recent changes to Turnout class. --- DCCEXParser.cpp | 97 +++++++++++++++++++++++++------------------------ Turnouts.cpp | 27 ++++++++++---- Turnouts.h | 16 +++++--- 3 files changed, 79 insertions(+), 61 deletions(-) diff --git a/DCCEXParser.cpp b/DCCEXParser.cpp index 6779178..2c50ac1 100644 --- a/DCCEXParser.cpp +++ b/DCCEXParser.cpp @@ -58,6 +58,8 @@ const int16_t HASH_KEYWORD_SPEED28 = -17064; const int16_t HASH_KEYWORD_SPEED128 = 25816; const int16_t HASH_KEYWORD_SERVO=27709; const int16_t HASH_KEYWORD_VPIN=-415; +const int16_t HASH_KEYWORD_C=67; +const int16_t HASH_KEYWORD_T=84; int16_t DCCEXParser::stashP[MAX_COMMAND_PARAMS]; bool DCCEXParser::stashBusy; @@ -674,63 +676,62 @@ bool DCCEXParser::parseT(Print *stream, int16_t params, int16_t p[]) return true; case 2: // - switch (p[1]) { -#ifdef TURNOUT_LEGACY_BEHAVIOUR - // turnout 1 or T=THROW, 0 or C=CLOSE - case 1: case 0x54: // 1 or T - if (!Turnout::setClosed(p[0], false)) return false; - p[1] = 1; - break; - case 0: case 0x43: // 0 or C - if (!Turnout::setClosed(p[0], true)) return false; - p[1] = 0; - break; -#else - // turnout 0 or T=THROW,1 or C=CLOSE - case 0: case 0x54: // 0 or T - if (!Turnout::setClosed(p[0], false)) return false; - p[1] = 0; - break; - case 1: case 0x43: // 1 or C - if (!Turnout::setClosed(p[0], true)) return false; - p[1] = 1; - break; -#endif - default: - return false; - } - // Send acknowledgement to caller if the command was not received over Serial (acknowledgement - // messages on Serial are sent by the Turnout class). - if (stream != &Serial) StringFormatter::send(stream, F("\n"), p[0], p[1]); - return true; + { + bool state = false; + switch (p[1]) { + // By default turnout command uses 0=throw, 1=close, + // but legacy DCC++ behaviour is 1=throw, 0=close. + case 0: + state = Turnout::useLegacyTurnoutBehaviour; + break; + case 1: + state = !Turnout::useLegacyTurnoutBehaviour; + break; + case HASH_KEYWORD_C: + state = true; + break; + case HASH_KEYWORD_T: + state= false; + break; + default: + return false; + } + if (!Turnout::setClosed(p[0], state)) return false; - default: // Anything else is some kind of create function. - if (p[1] == HASH_KEYWORD_SERVO) { // - if (params == 6) { - if (!ServoTurnout::create(p[0], (VPIN)p[2], (uint16_t)p[3], (uint16_t)p[4], (uint8_t)p[5])) - return false; - } else + // Send acknowledgement to caller if the command was not received over Serial + // (acknowledgement messages on Serial are sent by the Turnout class). + if (stream != &Serial) Turnout::printState(p[0], stream); + return true; + } + + default: // Anything else is some kind of turnout create function. + if (params == 6 && p[1] == HASH_KEYWORD_SERVO) { // + if (!ServoTurnout::create(p[0], (VPIN)p[2], (uint16_t)p[3], (uint16_t)p[4], (uint8_t)p[5])) return false; } else - if (p[1] == HASH_KEYWORD_VPIN) { // - if (params==3) { - if (!VpinTurnout::create(p[0], p[2])) return false; - } else - return false; - } else - if (p[1]==HASH_KEYWORD_DCC) { - if (params==4 && p[2]>0 && p[2]<=512 && p[3]>=0 && p[3]<4) { // + if (params == 3 && p[1] == HASH_KEYWORD_VPIN) { // + if (!VpinTurnout::create(p[0], p[2])) return false; + } else + if (params >= 3 && p[1] == HASH_KEYWORD_DCC) { + if (params==4 && p[2]>0 && p[2]<=512 && p[3]>=0 && p[3]<4) { // if (!DCCTurnout::create(p[0], p[2], p[3])) return false; - } else if (params==3 && p[2]>0 && p[2]<=512*4) { // + } else if (params==3 && p[2]>0 && p[2]<=512*4) { // , 1<=nn<=2048 if (!DCCTurnout::create(p[0], (p[2]-1)/4+1, (p[2]-1)%4)) return false; } else return false; - } else if (params==3) { // for DCC or LCN - if (!DCCTurnout::create(p[0], p[1], p[2])) return false; + } else + if (params==3) { // legacy for DCC accessory + if (p[1]>0 && p[1]<=512 && p[2]>=0 && p[2]<4) { + if (!DCCTurnout::create(p[0], p[1], p[2])) return false; + } else + return false; } - else if (params==3) { // legacy for Servo + else + if (params==4) { // legacy for Servo if (!ServoTurnout::create(p[0], (VPIN)p[1], (uint16_t)p[2], (uint16_t)p[3], 1)) return false; - } + } else + return false; + StringFormatter::send(stream, F("\n")); return true; } diff --git a/Turnouts.cpp b/Turnouts.cpp index 8a77f57..86607f4 100644 --- a/Turnouts.cpp +++ b/Turnouts.cpp @@ -20,6 +20,14 @@ * along with CommandStation. If not, see . */ +#ifndef TURNOUTS_CPP +#define TURNOUTS_CPP + +// Set the following definition to true for = throw and = close +// or to false for = close and = throw (the original way). +#ifndef USE_LEGACY_TURNOUT_BEHAVIOUR +#define USE_LEGACY_TURNOUT_BEHAVIOUR false +#endif #include "defines.h" #include "EEStore.h" @@ -30,12 +38,6 @@ #include "DIAG.h" #endif -// Keywords used for turnout configuration. -const int16_t HASH_KEYWORD_SERVO=27709; -const int16_t HASH_KEYWORD_DCC=6436; -const int16_t HASH_KEYWORD_VPIN=-415; - - /* * Protected static data */ @@ -46,6 +48,7 @@ const int16_t HASH_KEYWORD_VPIN=-415; * Public static data */ int Turnout::turnoutlistHash = 0; + bool Turnout::useLegacyTurnoutBehaviour = USE_LEGACY_TURNOUT_BEHAVIOUR; /* * Protected static functions @@ -130,8 +133,7 @@ const int16_t HASH_KEYWORD_VPIN=-415; // Send message to JMRI etc. over Serial USB. This is done here // to ensure that the message is sent when the turnout operation // is not initiated by a Serial command. - StringFormatter::send(Serial, F("\n"), id, closeFlag); - + printState(id, &Serial); return ok; } @@ -189,3 +191,12 @@ const int16_t HASH_KEYWORD_VPIN=-415; return tt; } + // Display, on the specified stream, the current state of the turnout (1 or 0). + void Turnout::printState(uint16_t id, Print *stream) { + Turnout *tt = get(id); + if (!tt) + StringFormatter::send(stream, F("\n"), + id, tt->_turnoutData.closed ^ useLegacyTurnoutBehaviour); + } + +#endif \ No newline at end of file diff --git a/Turnouts.h b/Turnouts.h index 6659c24..32a8a94 100644 --- a/Turnouts.h +++ b/Turnouts.h @@ -109,6 +109,7 @@ public: * Static data */ static int turnoutlistHash; + static bool useLegacyTurnoutBehaviour; /* * Public base class functions @@ -171,6 +172,8 @@ public: for (Turnout *tt = _firstTurnout; tt != 0; tt = tt->_nextTurnout) tt->print(stream); } + + static void printState(uint16_t id, Print *stream); }; @@ -259,7 +262,8 @@ public: void print(Print *stream) override { StringFormatter::send(stream, F("\n"), _turnoutData.id, _servoTurnoutData.vpin, - _servoTurnoutData.thrownPosition, _servoTurnoutData.closedPosition, _servoTurnoutData.profile, _turnoutData.closed); + _servoTurnoutData.thrownPosition, _servoTurnoutData.closedPosition, _servoTurnoutData.profile, + _turnoutData.closed ^ useLegacyTurnoutBehaviour); } // Load a Servo turnout definition from EEPROM. The common Turnout data has already been read at this point. @@ -338,7 +342,8 @@ public: void print(Print *stream) override { StringFormatter::send(stream, F("\n"), _turnoutData.id, - (((_dccTurnoutData.address-1) >> 2)+1), ((_dccTurnoutData.address-1) & 3), _turnoutData.closed); + (((_dccTurnoutData.address-1) >> 2)+1), ((_dccTurnoutData.address-1) & 3), + _turnoutData.closed ^ useLegacyTurnoutBehaviour); } // Load a DCC turnout definition from EEPROM. The common Turnout data has already been read at this point. @@ -413,8 +418,8 @@ public: } void print(Print *stream) override { - StringFormatter::send(stream, F("\n"), _turnoutData.id, - _vpinTurnoutData.vpin, _turnoutData.closed); + StringFormatter::send(stream, F("\n"), _turnoutData.id, _vpinTurnoutData.vpin, + _turnoutData.closed ^ useLegacyTurnoutBehaviour); } // Load a VPIN turnout definition from EEPROM. The common Turnout data has already been read at this point. @@ -477,7 +482,8 @@ public: //static Turnout *load(struct TurnoutData *turnoutData) { void print(Print *stream) override { - StringFormatter::send(stream, F("\n"), _turnoutData.id, _turnoutData.closed); + StringFormatter::send(stream, F("\n"), _turnoutData.id, + _turnoutData.closed ^ useLegacyTurnoutBehaviour); } }; From 133c65bc4246236889eccffa18e73820c433eb8a Mon Sep 17 00:00:00 2001 From: Neil McKechnie Date: Fri, 20 Aug 2021 15:43:03 +0100 Subject: [PATCH 06/11] Report Turnout configuration in old and new formats. JMRI currently isn't aware of the newer types of turnout in DCC++EX, so when it receives the definitions of turnouts it barfs on them. It still knows a turnout exists, but isn't able to display its full configuration. For DCC Accessory turnouts, the configuration message has changed so that it includes the DCC string (to distinguish them from other types of turnout). To enable current and older versions of JMRI to continue working with DCC turnouts, CS now reports the old and new formats, i.e. and . It currently accepts the first one and ignores the second one, but in the fullness of time it might accept the second one too. --- Turnouts.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Turnouts.h b/Turnouts.h index 32a8a94..0a05dca 100644 --- a/Turnouts.h +++ b/Turnouts.h @@ -344,6 +344,10 @@ public: StringFormatter::send(stream, F("\n"), _turnoutData.id, (((_dccTurnoutData.address-1) >> 2)+1), ((_dccTurnoutData.address-1) & 3), _turnoutData.closed ^ useLegacyTurnoutBehaviour); + // Also report using classic DCC++ syntax for DCC accessory turnouts + StringFormatter::send(stream, F("\n"), _turnoutData.id, + (((_dccTurnoutData.address-1) >> 2)+1), ((_dccTurnoutData.address-1) & 3), + _turnoutData.closed ^ useLegacyTurnoutBehaviour); } // Load a DCC turnout definition from EEPROM. The common Turnout data has already been read at this point. From d8366f33c89521d451d39d77d0ca12e13314d20d Mon Sep 17 00:00:00 2001 From: Neil McKechnie Date: Sat, 21 Aug 2021 00:25:00 +0100 Subject: [PATCH 07/11] Make output turnout state rather than full turnout definition. command currently prints the current states for outputs and for sensors, but prints the full configuration of turnouts. This change makes the turnout output consistent, i.e. just is output for each turnout. The command still outputs the full turnout definition. --- Turnouts.cpp | 4 +--- Turnouts.h | 6 +++++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/Turnouts.cpp b/Turnouts.cpp index 86607f4..2647e1a 100644 --- a/Turnouts.cpp +++ b/Turnouts.cpp @@ -194,9 +194,7 @@ // Display, on the specified stream, the current state of the turnout (1 or 0). void Turnout::printState(uint16_t id, Print *stream) { Turnout *tt = get(id); - if (!tt) - StringFormatter::send(stream, F("\n"), - id, tt->_turnoutData.closed ^ useLegacyTurnoutBehaviour); + if (!tt) tt->printState(stream); } #endif \ No newline at end of file diff --git a/Turnouts.h b/Turnouts.h index 0a05dca..5ab7b07 100644 --- a/Turnouts.h +++ b/Turnouts.h @@ -119,6 +119,10 @@ public: inline bool isType(uint8_t type) { return _turnoutData.turnoutType == type; } inline uint16_t getId() { return _turnoutData.id; } inline Turnout *next() { return _nextTurnout; } + inline void printState(Print *stream) { + StringFormatter::send(stream, F("\n"), + _turnoutData.id, _turnoutData.closed ^ useLegacyTurnoutBehaviour); + } /* * Virtual functions */ @@ -170,7 +174,7 @@ public: static void printAll(Print *stream) { for (Turnout *tt = _firstTurnout; tt != 0; tt = tt->_nextTurnout) - tt->print(stream); + tt->printState(stream); } static void printState(uint16_t id, Print *stream); From 071389a04b4d9abba92d58d3770da4af7ef63382 Mon Sep 17 00:00:00 2001 From: Neil McKechnie Date: Sat, 21 Aug 2021 00:34:28 +0100 Subject: [PATCH 08/11] Remove compiler warnings in Turnout.h --- Turnouts.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Turnouts.h b/Turnouts.h index 5ab7b07..258c6d8 100644 --- a/Turnouts.h +++ b/Turnouts.h @@ -126,7 +126,9 @@ public: /* * Virtual functions */ - virtual void print(Print *stream) {} + virtual void print(Print *stream) { + (void)stream; // avoid compiler warnings. + } virtual ~Turnout() {} // Destructor /* @@ -240,6 +242,8 @@ public: IODevice::writeAnalogue(vpin, closed ? closedPosition : thrownPosition, PCA9685::Instant); return tt; #else + (void)id; (void)vpin; (void)thrownPosition; (void)closedPosition; + (void)profile; (void)closed; // avoid compiler warnings. return NULL; #endif } @@ -250,6 +254,8 @@ public: IODevice::writeAnalogue(_servoTurnoutData.vpin, close ? _servoTurnoutData.closedPosition : _servoTurnoutData.thrownPosition, _servoTurnoutData.profile); _turnoutData.closed = close; +#else + (void)close; // avoid compiler warnings #endif return true; } From dbabfdca80c4bb27cca3c121112784a8c16e2dc6 Mon Sep 17 00:00:00 2001 From: Neil McKechnie Date: Sat, 21 Aug 2021 23:13:34 +0100 Subject: [PATCH 09/11] Improvements to PCA9685 operation Rationalise duplicated code; improve initialisation; --- IODevice.h | 2 +- IO_PCA9685.cpp | 61 ++++++++++++++++++++------------------------------ 2 files changed, 25 insertions(+), 38 deletions(-) diff --git a/IODevice.h b/IODevice.h index a542f56..eaffec8 100644 --- a/IODevice.h +++ b/IODevice.h @@ -274,7 +274,7 @@ private: uint8_t profile; // Config parameter uint8_t stepNumber; // Index of current step (starting from 0) uint8_t numSteps; // Number of steps in animation, or 0 if none in progress. - int8_t state; + uint8_t currentProfile; // profile being used for current animation. }; // 12 bytes per element, i.e. per pin in use struct ServoData *_servoData [16]; diff --git a/IO_PCA9685.cpp b/IO_PCA9685.cpp index 1c24335..9e97234 100644 --- a/IO_PCA9685.cpp +++ b/IO_PCA9685.cpp @@ -53,19 +53,20 @@ bool PCA9685::_configure(VPIN vpin, ConfigTypeEnum configType, int paramCount, i int8_t pin = vpin - _firstVpin; struct ServoData *s = _servoData[pin]; - if (!s) { + if (s == NULL) { _servoData[pin] = (struct ServoData *)calloc(1, sizeof(struct ServoData)); s = _servoData[pin]; if (!s) return false; // Check for failed memory allocation } s->activePosition = params[0]; - s->currentPosition = s->inactivePosition = params[1]; + s->inactivePosition = params[1]; s->profile = params[2]; - - // Position servo to initial state - s->state = -1; // Set unknown state, to force reposition - _write(vpin, params[3]); + int state = params[3]; + if (state != -1) { + // Position servo to initial state + _writeAnalogue(vpin, state ? s->activePosition : s->inactivePosition, Instant); + } return true; } @@ -75,6 +76,8 @@ PCA9685::PCA9685(VPIN firstVpin, int nPins, uint8_t I2CAddress) { _firstVpin = firstVpin; _nPins = min(nPins, 16); _I2CAddress = I2CAddress; + // To save RAM, space for servo configuration is not allocated unless a pin is used. + // Initialise the pointers to NULL. for (int i=0; i<_nPins; i++) _servoData[i] = NULL; @@ -113,30 +116,12 @@ void PCA9685::_write(VPIN vpin, int value) { if (value) value = 1; struct ServoData *s = _servoData[pin]; - if (!s) { + if (s == NULL) { // Pin not configured, just write default positions to servo controller - if (value) - writeDevice(pin, _defaultActivePosition); - else - writeDevice(pin, _defaultInactivePosition); + writeDevice(pin, value ? _defaultActivePosition : _defaultInactivePosition); } else { // Use configured parameters for advanced transitions - uint8_t profile = s->profile; - // If current position not known, go straight to selected position. - if (s->state == -1) profile = Instant; - - // Animated profile. Initiate the appropriate action. - s->numSteps = profile==Fast ? 10 : - profile==Medium ? 20 : - profile==Slow ? 40 : - profile==Bounce ? sizeof(_bounceProfile)-1 : - 1; - s->state = value; - s->stepNumber = 0; - - // Update new from/to positions to initiate or change animation. - s->fromPosition = s->currentPosition; - s->toPosition = s->state ? s->activePosition : s->inactivePosition; + _writeAnalogue(vpin, value ? s->activePosition : s->inactivePosition, s->profile); } } @@ -150,16 +135,18 @@ void PCA9685::_writeAnalogue(VPIN vpin, int value, int profile) { else if (value < 0) value = 0; struct ServoData *s = _servoData[pin]; - - if (!s) { - // Servo pin not configured, so configure now. + if (s == NULL) { + // Servo pin not configured, so configure now using defaults s = _servoData[pin] = (struct ServoData *) calloc(sizeof(struct ServoData), 1); + if (s == NULL) return; // Check for memory allocation failure s->activePosition = _defaultActivePosition; s->inactivePosition = _defaultInactivePosition; - s->currentPosition = value; // Don't know where we're moving from. + s->currentPosition = value; + s->profile = Instant; } - s->profile = profile; + // Animated profile. Initiate the appropriate action. + s->currentProfile = profile; s->numSteps = profile==Fast ? 10 : profile==Medium ? 20 : profile==Slow ? 40 : @@ -175,7 +162,7 @@ void PCA9685::_writeAnalogue(VPIN vpin, int value, int profile) { bool PCA9685::_isActive(VPIN vpin) { int pin = vpin - _firstVpin; struct ServoData *s = _servoData[pin]; - if (!s) + if (s == NULL) return false; // No structure means no animation! else return (s->numSteps != 0); @@ -194,16 +181,16 @@ void PCA9685::_loop(unsigned long currentMicros) { // TODO: Could calculate step number from elapsed time, to allow for erratic loop timing. void PCA9685::updatePosition(uint8_t pin) { struct ServoData *s = _servoData[pin]; - if (!s) return; + if (s == NULL) return; // No pin configuration/state data if (s->numSteps == 0) return; // No animation in progress if (s->stepNumber == 0 && s->fromPosition == s->toPosition) { - // No movement required, so go straight to final step - s->stepNumber = s->numSteps; + // Go straight to end of sequence, output final position. + s->stepNumber = s->numSteps-1; } if (s->stepNumber < s->numSteps) { // Animation in progress, reposition servo s->stepNumber++; - if (s->profile == Bounce) { + if (s->currentProfile == Bounce) { // Retrieve step positions from array in flash byte profileValue = GETFLASH(&_bounceProfile[s->stepNumber]); s->currentPosition = map(profileValue, 0, 100, s->fromPosition, s->toPosition); From 39a69e340eead439b20127f86cdcbea93404002e Mon Sep 17 00:00:00 2001 From: Neil McKechnie Date: Sat, 21 Aug 2021 23:16:52 +0100 Subject: [PATCH 10/11] Turnout EEPROM improvements. Ensure state is saved and restored from EEPROM as expected. Make constructors for turnouts private. Otherwise, a statically created turnout may be initialising itself before the underlying HAL device has been created. By requiring the create() call be used, there is more control over the timing of the turnout object's creation. --- Turnouts.cpp | 26 +++++----- Turnouts.h | 138 +++++++++++++++++++++++++++------------------------ 2 files changed, 86 insertions(+), 78 deletions(-) diff --git a/Turnouts.cpp b/Turnouts.cpp index 2647e1a..3c8c35a 100644 --- a/Turnouts.cpp +++ b/Turnouts.cpp @@ -121,19 +121,21 @@ if (!tt) return false; bool ok = tt->activate(closeFlag); - // Write new closed/thrown state to EEPROM if required. Note that eepromAddress - // is always zero for LCN turnouts. - if (EEStore::eeStore->data.nTurnouts > 0 && tt->_eepromAddress) - EEPROM.put(tt->_eepromAddress, tt->_turnoutData.closed); + if (ok) { + // Write byte containing new closed/thrown state to EEPROM if required. Note that eepromAddress + // is always zero for LCN turnouts. + if (EEStore::eeStore->data.nTurnouts > 0 && tt->_eepromAddress > 0) + EEPROM.put(tt->_eepromAddress, *((uint8_t *) &tt->_turnoutData)); - #if defined(RMFT_ACTIVE) - RMFT2::turnoutEvent(id, closeFlag); - #endif + #if defined(RMFT_ACTIVE) + RMFT2::turnoutEvent(id, closeFlag); + #endif - // Send message to JMRI etc. over Serial USB. This is done here - // to ensure that the message is sent when the turnout operation - // is not initiated by a Serial command. - printState(id, &Serial); + // Send message to JMRI etc. over Serial USB. This is done here + // to ensure that the message is sent when the turnout operation + // is not initiated by a Serial command. + printState(id, &Serial); + } return ok; } @@ -180,7 +182,7 @@ // so we can't go any further through the EEPROM! return NULL; } - if (!tt) { + if (tt) { // Save EEPROM address in object. Note that LCN turnouts always have eepromAddress of zero. tt->_eepromAddress = eepromAddress; } diff --git a/Turnouts.h b/Turnouts.h index 258c6d8..11754a4 100644 --- a/Turnouts.h +++ b/Turnouts.h @@ -198,7 +198,6 @@ private: uint8_t profile; } _servoTurnoutData; // 6 bytes -public: // Constructor ServoTurnout(uint16_t id, VPIN vpin, uint16_t thrownPosition, uint16_t closedPosition, uint8_t profile, bool closed = true) : Turnout(id, TURNOUT_SERVO, closed) @@ -209,6 +208,7 @@ public: _servoTurnoutData.profile = profile; } +public: // Create function static Turnout *create(uint16_t id, VPIN vpin, uint16_t thrownPosition, uint16_t closedPosition, uint8_t profile, bool closed = true) { #ifndef IO_NO_HAL @@ -229,7 +229,7 @@ public: // int params[] = {(int)thrownPosition, (int)closedPosition, profile, closed}; // IODevice::configure(vpin, IODevice::CONFIGURE_SERVO, 4, params); - // Set position to saved position + // Set position directly to specified position - we don't know where it is moving from. IODevice::writeAnalogue(vpin, closed ? closedPosition : thrownPosition, PCA9685::Instant); return tt; @@ -248,6 +248,26 @@ public: #endif } + // Load a Servo turnout definition from EEPROM. The common Turnout data has already been read at this point. + static Turnout *load(struct TurnoutData *turnoutData) { + ServoTurnoutData servoTurnoutData; + // Read class-specific data from EEPROM + EEPROM.get(EEStore::pointer(), servoTurnoutData); + EEStore::advance(sizeof(servoTurnoutData)); + + // Create new object + Turnout *tt = ServoTurnout::create(turnoutData->id, servoTurnoutData.vpin, servoTurnoutData.thrownPosition, + servoTurnoutData.closedPosition, servoTurnoutData.profile, turnoutData->closed); + return tt; + } + + void print(Print *stream) override { + StringFormatter::send(stream, F("\n"), _turnoutData.id, _servoTurnoutData.vpin, + _servoTurnoutData.thrownPosition, _servoTurnoutData.closedPosition, _servoTurnoutData.profile, + _turnoutData.closed ^ useLegacyTurnoutBehaviour); + } + +protected: // ServoTurnout-specific code for throwing or closing a servo turnout. bool activate(bool close) override { #ifndef IO_NO_HAL @@ -269,26 +289,6 @@ public: EEPROM.put(EEStore::pointer(), _servoTurnoutData); EEStore::advance(sizeof(_servoTurnoutData)); } - - void print(Print *stream) override { - StringFormatter::send(stream, F("\n"), _turnoutData.id, _servoTurnoutData.vpin, - _servoTurnoutData.thrownPosition, _servoTurnoutData.closedPosition, _servoTurnoutData.profile, - _turnoutData.closed ^ useLegacyTurnoutBehaviour); - } - - // Load a Servo turnout definition from EEPROM. The common Turnout data has already been read at this point. - static Turnout *load(struct TurnoutData *turnoutData) { - ServoTurnoutData servoTurnoutData; - // Read class-specific data from EEPROM - EEPROM.get(EEStore::pointer(), servoTurnoutData); - EEStore::advance(sizeof(servoTurnoutData)); - - // Create new object - ServoTurnout *tt = new ServoTurnout(turnoutData->id, servoTurnoutData.vpin, servoTurnoutData.thrownPosition, - servoTurnoutData.closedPosition, servoTurnoutData.profile, turnoutData->closed); - - return tt; - } }; /************************************************************************************* @@ -305,7 +305,6 @@ private: // That's DCC accessory address 1-512 and subaddress 0-3. } _dccTurnoutData; // 2 bytes -public: // Constructor DCCTurnout(uint16_t id, uint16_t address, uint8_t subAdd) : Turnout(id, TURNOUT_DCC, false) @@ -313,6 +312,7 @@ public: _dccTurnoutData.address = ((address-1) << 2) + subAdd + 1; } +public: // Create function static Turnout *create(uint16_t id, uint16_t add, uint8_t subAdd) { Turnout *tt = get(id); @@ -333,9 +333,36 @@ public: return tt; } + // Load a DCC turnout definition from EEPROM. The common Turnout data has already been read at this point. + static Turnout *load(struct TurnoutData *turnoutData) { + DCCTurnoutData dccTurnoutData; + // Read class-specific data from EEPROM + EEPROM.get(EEStore::pointer(), dccTurnoutData); + EEStore::advance(sizeof(dccTurnoutData)); + + // Create new object + DCCTurnout *tt = new DCCTurnout(turnoutData->id, (((dccTurnoutData.address-1) >> 2)+1), ((dccTurnoutData.address-1) & 3)); + + return tt; + } + + void print(Print *stream) override { + StringFormatter::send(stream, F("\n"), _turnoutData.id, + (((_dccTurnoutData.address-1) >> 2)+1), ((_dccTurnoutData.address-1) & 3), + _turnoutData.closed ^ useLegacyTurnoutBehaviour); + // Also report using classic DCC++ syntax for DCC accessory turnouts + StringFormatter::send(stream, F("\n"), _turnoutData.id, + (((_dccTurnoutData.address-1) >> 2)+1), ((_dccTurnoutData.address-1) & 3), + _turnoutData.closed ^ useLegacyTurnoutBehaviour); + } + +protected: bool activate(bool close) override { + // DCC++ Classic behaviour is that Throw writes a 1 in the packet, + // and Close writes a 0. + // RCN-214 specifies that Throw is 0 and Close is 1. DCC::setAccessory((((_dccTurnoutData.address-1) >> 2) + 1), - ((_dccTurnoutData.address-1) & 3), close); + ((_dccTurnoutData.address-1) & 3), close ^ useLegacyTurnoutBehaviour); _turnoutData.closed = close; return true; } @@ -350,28 +377,6 @@ public: EEStore::advance(sizeof(_dccTurnoutData)); } - void print(Print *stream) override { - StringFormatter::send(stream, F("\n"), _turnoutData.id, - (((_dccTurnoutData.address-1) >> 2)+1), ((_dccTurnoutData.address-1) & 3), - _turnoutData.closed ^ useLegacyTurnoutBehaviour); - // Also report using classic DCC++ syntax for DCC accessory turnouts - StringFormatter::send(stream, F("\n"), _turnoutData.id, - (((_dccTurnoutData.address-1) >> 2)+1), ((_dccTurnoutData.address-1) & 3), - _turnoutData.closed ^ useLegacyTurnoutBehaviour); - } - - // Load a DCC turnout definition from EEPROM. The common Turnout data has already been read at this point. - static Turnout *load(struct TurnoutData *turnoutData) { - DCCTurnoutData dccTurnoutData; - // Read class-specific data from EEPROM - EEPROM.get(EEStore::pointer(), dccTurnoutData); - EEStore::advance(sizeof(dccTurnoutData)); - - // Create new object - DCCTurnout *tt = new DCCTurnout(turnoutData->id, (((dccTurnoutData.address-1) >> 2)+1), ((dccTurnoutData.address-1) & 3)); - - return tt; - } }; @@ -387,7 +392,6 @@ private: VPIN vpin; } _vpinTurnoutData; // 2 bytes -public: // Constructor VpinTurnout(uint16_t id, VPIN vpin, bool closed=true) : Turnout(id, TURNOUT_VPIN, closed) @@ -395,6 +399,7 @@ public: _vpinTurnoutData.vpin = vpin; } +public: // Create function static Turnout *create(uint16_t id, VPIN vpin, bool closed=true) { Turnout *tt = get(id); @@ -415,6 +420,25 @@ public: return tt; } + // Load a VPIN turnout definition from EEPROM. The common Turnout data has already been read at this point. + static Turnout *load(struct TurnoutData *turnoutData) { + VpinTurnoutData vpinTurnoutData; + // Read class-specific data from EEPROM + EEPROM.get(EEStore::pointer(), vpinTurnoutData); + EEStore::advance(sizeof(vpinTurnoutData)); + + // Create new object + VpinTurnout *tt = new VpinTurnout(turnoutData->id, vpinTurnoutData.vpin, turnoutData->closed); + + return tt; + } + + void print(Print *stream) override { + StringFormatter::send(stream, F("\n"), _turnoutData.id, _vpinTurnoutData.vpin, + _turnoutData.closed ^ useLegacyTurnoutBehaviour); + } + +protected: bool activate(bool close) override { IODevice::write(_vpinTurnoutData.vpin, close); _turnoutData.closed = close; @@ -430,24 +454,6 @@ public: EEPROM.put(EEStore::pointer(), _vpinTurnoutData); EEStore::advance(sizeof(_vpinTurnoutData)); } - - void print(Print *stream) override { - StringFormatter::send(stream, F("\n"), _turnoutData.id, _vpinTurnoutData.vpin, - _turnoutData.closed ^ useLegacyTurnoutBehaviour); - } - - // Load a VPIN turnout definition from EEPROM. The common Turnout data has already been read at this point. - static Turnout *load(struct TurnoutData *turnoutData) { - VpinTurnoutData vpinTurnoutData; - // Read class-specific data from EEPROM - EEPROM.get(EEStore::pointer(), vpinTurnoutData); - EEStore::advance(sizeof(vpinTurnoutData)); - - // Create new object - VpinTurnout *tt = new VpinTurnout(turnoutData->id, vpinTurnoutData.vpin, turnoutData->closed); - - return tt; - } }; @@ -461,12 +467,12 @@ private: // struct LCNTurnoutData { // } _lcnTurnoutData; // 0 bytes -public: // Constructor LCNTurnout(uint16_t id, bool closed=true) : Turnout(id, TURNOUT_LCN, closed) { } +public: // Create function static Turnout *create(uint16_t id, bool closed=true) { Turnout *tt = get(id); From 0875d27b0a009babccf89d3b476691bd02402ff8 Mon Sep 17 00:00:00 2001 From: Neil McKechnie Date: Sun, 22 Aug 2021 14:07:16 +0100 Subject: [PATCH 11/11] Remove 'activate' functions from turnout classes. Remove the static 'activate' function and rename the virtual 'activate' function to 'setClosedInternal'. --- Turnouts.cpp | 8 ++++---- Turnouts.h | 20 ++++++++------------ 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/Turnouts.cpp b/Turnouts.cpp index 3c8c35a..22bc175 100644 --- a/Turnouts.cpp +++ b/Turnouts.cpp @@ -106,11 +106,11 @@ return false; } - // Static activate function is invoked from close(), throw() etc. to perform the + // Static setClosed function is invoked from close(), throw() etc. to perform the // common parts of the turnout operation. Code which is specific to a turnout - // type should be placed in the virtual function activate(bool) which is + // type should be placed in the virtual function setClosedInternal(bool) which is // called from here. - bool Turnout::activate(uint16_t id, bool closeFlag) { + bool Turnout::setClosed(uint16_t id, bool closeFlag) { #ifdef EESTOREDEBUG if (closeFlag) DIAG(F("Turnout::close(%d)"), id); @@ -119,7 +119,7 @@ #endif Turnout *tt = Turnout::get(id); if (!tt) return false; - bool ok = tt->activate(closeFlag); + bool ok = tt->setClosedInternal(closeFlag); if (ok) { // Write byte containing new closed/thrown state to EEPROM if required. Note that eepromAddress diff --git a/Turnouts.h b/Turnouts.h index 11754a4..bb2c57e 100644 --- a/Turnouts.h +++ b/Turnouts.h @@ -93,7 +93,7 @@ protected: * Virtual functions */ - virtual bool activate(bool close) = 0; // Mandatory in subclass + virtual bool setClosedInternal(bool close) = 0; // Mandatory in subclass virtual void save() {} /* @@ -144,18 +144,14 @@ public: return !isClosed(id); } - static bool activate(uint16_t id, bool closeFlag); + static bool setClosed(uint16_t id, bool closeFlag); inline static bool setClosed(uint16_t id) { - return activate(id, true); + return setClosed(id, true); } inline static bool setThrown(uint16_t id) { - return activate(id, false); - } - - inline static bool setClosed(uint16_t id, bool close) { - return activate(id, close); + return setClosed(id, false); } static bool setClosedStateOnly(uint16_t id, bool close) { @@ -269,7 +265,7 @@ public: protected: // ServoTurnout-specific code for throwing or closing a servo turnout. - bool activate(bool close) override { + bool setClosedInternal(bool close) override { #ifndef IO_NO_HAL IODevice::writeAnalogue(_servoTurnoutData.vpin, close ? _servoTurnoutData.closedPosition : _servoTurnoutData.thrownPosition, _servoTurnoutData.profile); @@ -357,7 +353,7 @@ public: } protected: - bool activate(bool close) override { + bool setClosedInternal(bool close) override { // DCC++ Classic behaviour is that Throw writes a 1 in the packet, // and Close writes a 0. // RCN-214 specifies that Throw is 0 and Close is 1. @@ -439,7 +435,7 @@ public: } protected: - bool activate(bool close) override { + bool setClosedInternal(bool close) override { IODevice::write(_vpinTurnoutData.vpin, close); _turnoutData.closed = close; return true; @@ -490,7 +486,7 @@ public: return tt; } - bool activate(bool close) override { + bool setClosedInternal(bool close) override { // Assume that the LCN command still uses 1 for throw and 0 for close... LCN::send('T', _turnoutData.id, !close); // The _turnoutData.closed flag should be updated by a message from the LCN master, later.