From 6d12e6cd3f5f520020d49946652a94c1e3473f6b Mon Sep 17 00:00:00 2001 From: Alex Shepherd Date: Sat, 25 May 2019 15:52:55 +1200 Subject: [PATCH 1/9] added conditional compilation for ESP8266 to add ICACHE_RAM_ATTR to ExternalInterruptHandler changed storage for Micros to unsigned long --- NmraDcc.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/NmraDcc.cpp b/NmraDcc.cpp index 6adfc6e..fa2561a 100644 --- a/NmraDcc.cpp +++ b/NmraDcc.cpp @@ -310,6 +310,8 @@ DCC_PROCESSOR_STATE DccProcState ; portMUX_TYPE mux = portMUX_INITIALIZER_UNLOCKED; void IRAM_ATTR ExternalInterruptHandler(void) +#elif defined(ESP8266) +void ICACHE_RAM_ATTR ExternalInterruptHandler(void) #else void ExternalInterruptHandler(void) #endif @@ -340,9 +342,9 @@ void ExternalInterruptHandler(void) // Bit evaluation without Timer 0 ------------------------------ uint8_t DccBitVal; static int8_t bit1, bit2 ; - static word lastMicros; + static unsigned long lastMicros = 0; static byte halfBit, DCC_IrqRunning; - unsigned int actMicros, bitMicros; + unsigned long actMicros, bitMicros; if ( DCC_IrqRunning ) { // nested DCC IRQ - obviously there are glitches // ignore this interrupt and increment glitchcounter From 6a7e206032b7f056c1d11131b5ed40cc91def61b Mon Sep 17 00:00:00 2001 From: Alex Shepherd Date: Mon, 5 Aug 2019 21:30:39 +1200 Subject: [PATCH 2/9] reverted changes around lastMicros --- NmraDcc.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/NmraDcc.cpp b/NmraDcc.cpp index fa2561a..4034f53 100644 --- a/NmraDcc.cpp +++ b/NmraDcc.cpp @@ -342,9 +342,9 @@ void ExternalInterruptHandler(void) // Bit evaluation without Timer 0 ------------------------------ uint8_t DccBitVal; static int8_t bit1, bit2 ; - static unsigned long lastMicros = 0; + static word lastMicros = 0; static byte halfBit, DCC_IrqRunning; - unsigned long actMicros, bitMicros; + unsigned int actMicros, bitMicros; if ( DCC_IrqRunning ) { // nested DCC IRQ - obviously there are glitches // ignore this interrupt and increment glitchcounter From e06f6b3bce9277de9f9b3b6e51b7bf91329f8c5a Mon Sep 17 00:00:00 2001 From: Alex Shepherd Date: Mon, 5 Aug 2019 21:34:48 +1200 Subject: [PATCH 3/9] bumped version to 2.0.2 --- library.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library.properties b/library.properties index 2622dbd..ac7eec3 100644 --- a/library.properties +++ b/library.properties @@ -1,5 +1,5 @@ name=NmraDcc -version=2.0.1 +version=2.0.2 author=Alex Shepherd, Wolfgang Kuffer, Geoff Bunza, Martin Pischky, Franz-Peter Müller, Sven (littleyoda), Hans Tanner maintainer=Alex Shepherd sentence=Enables NMRA DCC Communication From f3a2b87693e724663a43959c922738c6f1dcb6ac Mon Sep 17 00:00:00 2001 From: Alex Shepherd Date: Tue, 6 Aug 2019 00:22:04 +1200 Subject: [PATCH 4/9] split out ServiceMode ackCV from Ops Mode AdvancedCVAck as doing a ackCV in Ops Mode is wrong and adds 6ms busy delay add cache of CV29 value --- NmraDcc.cpp | 35 ++++++++++++++++++++--------------- NmraDcc.h | 14 +++++++++++++- 2 files changed, 33 insertions(+), 16 deletions(-) diff --git a/NmraDcc.cpp b/NmraDcc.cpp index 4034f53..334de28 100644 --- a/NmraDcc.cpp +++ b/NmraDcc.cpp @@ -295,7 +295,8 @@ typedef struct uint8_t ExtIntNum; uint8_t ExtIntPinNum; int16_t myDccAddress; // Cached value of DCC Address from CVs - uint8_t inAccDecDCCAddrNextReceivedMode; + uint8_t inAccDecDCCAddrNextReceivedMode; + uint8_t cv29Value; #ifdef DCC_DEBUG uint8_t IntCount; uint8_t TickCount; @@ -604,6 +605,13 @@ void ackCV(void) notifyCVAck() ; } +void ackAdvancedCV(void) +{ + if( notifyAdvancedCVAck && (DccProcState.cv29Value & CV29_ADV_ACK) ) + notifyAdvancedCVAck() ; +} + + uint8_t readEEPROM( unsigned int CV ) { return EEPROM.read(CV) ; } @@ -663,6 +671,7 @@ uint8_t writeCV( unsigned int CV, uint8_t Value) { case CV_29_CONFIG: // copy addressmode Bit to Flags + DccProcState.cv29Value = Value; DccProcState.Flags = ( DccProcState.Flags & ~FLAGS_CV29_BITS) | (Value & FLAGS_CV29_BITS); // no break, because myDccAdress must also be reset case CV_ACCESSORY_DECODER_ADDRESS_LSB: // Also same CV for CV_MULTIFUNCTION_PRIMARY_ADDRESS @@ -691,23 +700,19 @@ uint8_t writeCV( unsigned int CV, uint8_t Value) uint16_t getMyAddr(void) { - uint8_t CV29Value ; - if( DccProcState.myDccAddress != -1 ) // See if we can return the cached value return( DccProcState.myDccAddress ); - CV29Value = readCV( CV_29_CONFIG ) ; - - if( CV29Value & CV29_ACCESSORY_DECODER ) // Accessory Decoder? + if( DccProcState.cv29Value & CV29_ACCESSORY_DECODER ) // Accessory Decoder? { - if( CV29Value & CV29_OUTPUT_ADDRESS_MODE ) + if( DccProcState.cv29Value & CV29_OUTPUT_ADDRESS_MODE ) DccProcState.myDccAddress = ( readCV( CV_ACCESSORY_DECODER_ADDRESS_MSB ) << 8 ) | readCV( CV_ACCESSORY_DECODER_ADDRESS_LSB ); else DccProcState.myDccAddress = ( ( readCV( CV_ACCESSORY_DECODER_ADDRESS_MSB ) & 0b00000111) << 6 ) | ( readCV( CV_ACCESSORY_DECODER_ADDRESS_LSB ) & 0b00111111) ; } else // Multi-Function Decoder? { - if( CV29Value & CV29_EXT_ADDRESSING ) // Two Byte Address? + if( DccProcState.cv29Value & CV29_EXT_ADDRESSING ) // Two Byte Address? DccProcState.myDccAddress = ( ( readCV( CV_MULTIFUNCTION_EXTENDED_ADDRESS_MSB ) - 192 ) << 8 ) | readCV( CV_MULTIFUNCTION_EXTENDED_ADDRESS_LSB ) ; else @@ -728,7 +733,7 @@ void processDirectOpsOperation( uint8_t Cmd, uint16_t CVAddr, uint8_t Value ) if( validCV( CVAddr, 1 ) ) { if( writeCV( CVAddr, Value ) == Value ) - ackCV(); + ackAdvancedCV(); } } @@ -737,7 +742,7 @@ void processDirectOpsOperation( uint8_t Cmd, uint16_t CVAddr, uint8_t Value ) if( validCV( CVAddr, 0 ) ) { if( readCV( CVAddr ) == Value ) - ackCV(); + ackAdvancedCV(); } } } @@ -762,7 +767,7 @@ void processDirectOpsOperation( uint8_t Cmd, uint16_t CVAddr, uint8_t Value ) tempValue &= ~BitMask ; // Turn the Bit Off if( writeCV( CVAddr, tempValue ) == tempValue ) - ackCV() ; + ackAdvancedCV() ; } } @@ -774,12 +779,12 @@ void processDirectOpsOperation( uint8_t Cmd, uint16_t CVAddr, uint8_t Value ) if( BitValue ) { if( tempValue & BitMask ) - ackCV() ; + ackAdvancedCV() ; } else { if( !( tempValue & BitMask) ) - ackCV() ; + ackAdvancedCV() ; } } } @@ -873,7 +878,7 @@ void processMultiFunctionMessage( uint16_t Addr, DCC_ADDR_TYPE AddrType, uint8_t case 0b01100000: //TODO should we cache this info in DCC_PROCESSOR_STATE.Flags ? #ifdef NMRA_DCC_ENABLE_14_SPEED_STEP_MODE - speedSteps = (readCV( CV_29_CONFIG ) & CV29_F0_LOCATION) ? SPEED_STEP_28 : SPEED_STEP_14 ; + speedSteps = (DccProcState.cv29Value & CV29_F0_LOCATION) ? SPEED_STEP_28 : SPEED_STEP_14 ; #else speedSteps = SPEED_STEP_28 ; #endif @@ -1409,7 +1414,7 @@ void NmraDcc::init( uint8_t ManufacturerId, uint8_t VersionId, uint8_t Flags, ui // Set the Bits that control Multifunction or Accessory behaviour // and if the Accessory decoder optionally handles Output Addressing // we need to peal off the top two bits - writeCV( CV_29_CONFIG, ( readCV( CV_29_CONFIG ) & ~FLAGS_CV29_BITS ) | (Flags & FLAGS_CV29_BITS) ) ; + DccProcState.cv29Value = writeCV( CV_29_CONFIG, ( readCV( CV_29_CONFIG ) & ~FLAGS_CV29_BITS ) | (Flags & FLAGS_CV29_BITS) ) ; uint8_t doAutoFactoryDefault = 0; if((Flags & FLAGS_AUTO_FACTORY_DEFAULT) && (readCV(CV_VERSION_ID) == 255) && (readCV(CV_MANUFACTURER_ID) == 255)) diff --git a/NmraDcc.h b/NmraDcc.h index 55bf4b4..cfc8f51 100644 --- a/NmraDcc.h +++ b/NmraDcc.h @@ -97,7 +97,7 @@ typedef struct #define CV_29_CONFIG 29 #if defined(ESP32) - #include + #include #define MAXCV SPI_FLASH_SEC_SIZE #elif defined(ESP8266) #include @@ -698,6 +698,18 @@ extern void notifyCVResetFactoryDefault(void) __attribute__ ((weak)); * None */ extern void notifyCVAck(void) __attribute__ ((weak)); +/*+ + * notifyAdvancedCVAck() Called when a CV write must be acknowledged. + * This callback must increase the current drawn by this + * decoder by at least 60mA for 6ms +/- 1ms. + * + * Inputs: + * None + * * + * Returns: + * None + */ +extern void notifyAdvancedCVAck(void) __attribute__ ((weak)); /*+ * notifyServiceMode(bool) Called when state of 'inServiceMode' changes * From 71bb657e3a7d80edc096094564f5d279e8a1fb69 Mon Sep 17 00:00:00 2001 From: Franz-Peter Date: Mon, 5 Aug 2019 15:14:58 +0200 Subject: [PATCH 5/9] Esp32 iram attr (#26) * changed the version to 201 in the header * added conditional compilation for ESP8266 to add ICACHE_RAM_ATTR to ExternalInterruptHandler changed storage for Micros to unsigned long * some tuning to bit recognition and nested IRQ (STM32) --- NmraDcc.cpp | 40 +++++++++++++++++++++++++--------------- NmraDcc.h | 7 ++++++- 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/NmraDcc.cpp b/NmraDcc.cpp index 6adfc6e..e16d05e 100644 --- a/NmraDcc.cpp +++ b/NmraDcc.cpp @@ -93,7 +93,7 @@ #define MAX_ONEBITHALF 82 #define MIN_ONEBITFULL 82 #define MIN_ONEBITHALF 35 -#define MAX_BITDIFF 18 +#define MAX_BITDIFF 24 // Debug-Ports @@ -217,18 +217,12 @@ #define MODE_TP2 #define SET_TP2 #define CLR_TP2 - //#define MODE_TP2 DDRC |= (1<<2) // A2 - //#define SET_TP2 PORTC |= (1<<2) - //#define CLR_TP2 PORTC &= ~(1<<2) #define MODE_TP3 #define SET_TP3 #define CLR_TP3 #define MODE_TP4 #define SET_TP4 #define CLR_TP4 - //#define MODE_TP4 DDRC |= (1<<4) //A4 - //#define SET_TP4 PORTC |= (1<<4) - //#define CLR_TP4 PORTC &= ~(1<<4) #endif #ifdef DEBUG_PRINT @@ -310,6 +304,8 @@ DCC_PROCESSOR_STATE DccProcState ; portMUX_TYPE mux = portMUX_INITIALIZER_UNLOCKED; void IRAM_ATTR ExternalInterruptHandler(void) +#elif defined(ESP8266) +void ICACHE_RAM_ATTR ExternalInterruptHandler(void) #else void ExternalInterruptHandler(void) #endif @@ -340,9 +336,10 @@ void ExternalInterruptHandler(void) // Bit evaluation without Timer 0 ------------------------------ uint8_t DccBitVal; static int8_t bit1, bit2 ; - static word lastMicros; + static unsigned int lastMicros = 0; static byte halfBit, DCC_IrqRunning; unsigned int actMicros, bitMicros; + #ifdef ALLOW_NESTED_IRQ if ( DCC_IrqRunning ) { // nested DCC IRQ - obviously there are glitches // ignore this interrupt and increment glitchcounter @@ -353,6 +350,7 @@ void ExternalInterruptHandler(void) SET_TP3; return; //>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> abort IRQ } + #endif SET_TP3; actMicros = micros(); bitMicros = actMicros-lastMicros; @@ -364,11 +362,12 @@ void ExternalInterruptHandler(void) } DccBitVal = ( bitMicros < bitMax ); lastMicros = actMicros; - #ifdef debug - if(DccBitVal) {SET_TP2;} else {CLR_TP2;}; - #endif + + #ifdef ALLOW_NESTED_IRQ DCC_IrqRunning = true; interrupts(); // time critical is only the micros() command,so allow nested irq's + #endif + #ifdef DCC_DEBUG DccProcState.TickCount++; #endif @@ -429,13 +428,10 @@ void ExternalInterruptHandler(void) DccRx.BitCount++; if( abs(bit2-bit1) > MAX_BITDIFF ) { // the length of the 2 halfbits differ too much -> wrong protokoll - CLR_TP2; - CLR_TP3; DccRx.State = WAIT_PREAMBLE; bitMax = MAX_PRAEAMBEL; bitMin = MIN_ONEBITFULL; DccRx.BitCount = 0; - SET_TP4; #if defined ( __STM32F1__ ) detachInterrupt( DccProcState.ExtIntNum ); @@ -562,6 +558,7 @@ void ExternalInterruptHandler(void) { CLR_TP3; DccRx.State = WAIT_PREAMBLE ; + DccRx.BitCount = 0 ; bitMax = MAX_PRAEAMBEL; bitMin = MIN_ONEBITFULL; #ifdef ESP32 @@ -591,9 +588,11 @@ void ExternalInterruptHandler(void) DccRx.TempByte = 0 ; } } + #ifdef ALLOW_NESTED_IRQ + DCC_IrqRunning = false; + #endif CLR_TP1; CLR_TP3; - DCC_IrqRunning = false; } void ackCV(void) @@ -1355,6 +1354,17 @@ void NmraDcc::pin( uint8_t ExtIntNum, uint8_t ExtIntPinNum, uint8_t EnablePullup #if defined ( __STM32F1__ ) // with STM32F1 the interuptnumber is equal the pin number DccProcState.ExtIntNum = ExtIntPinNum; + // because STM32F1 has a NVIC we must set interuptpriorities + const nvic_irq_num irqNum2nvic[] = { NVIC_EXTI0, NVIC_EXTI1, NVIC_EXTI2, NVIC_EXTI3, NVIC_EXTI4, + NVIC_EXTI_9_5, NVIC_EXTI_9_5, NVIC_EXTI_9_5, NVIC_EXTI_9_5, NVIC_EXTI_9_5, + NVIC_EXTI_15_10, NVIC_EXTI_15_10, NVIC_EXTI_15_10, NVIC_EXTI_15_10, NVIC_EXTI_15_10, NVIC_EXTI_15_10 }; + exti_num irqNum = (exti_num)(PIN_MAP[ExtIntPinNum].gpio_bit); + +// DCC-Input IRQ must be able to interrupt other long low priority ( level15 ) IRQ's + nvic_irq_set_priority ( irqNum2nvic[irqNum], PRIO_DCC_IRQ); + +// Systic must be able to interrupt DCC-IRQ to always get correct micros() values + nvic_irq_set_priority(NVIC_SYSTICK, PRIO_SYSTIC); #else DccProcState.ExtIntNum = ExtIntNum; #endif diff --git a/NmraDcc.h b/NmraDcc.h index d1834c7..38c4b4f 100644 --- a/NmraDcc.h +++ b/NmraDcc.h @@ -49,10 +49,12 @@ #ifndef NMRADCC_IS_IN #define NMRADCC_IS_IN -#define NMRADCC_VERSION 200 // Version 2.0.0 +#define NMRADCC_VERSION 201 // Version 2.0.1 #define MAX_DCC_MESSAGE_LEN 6 // including XOR-Byte +//#define ALLOW_NESTED_IRQ // uncomment to enable nested IRQ's ( only for AVR! ) + typedef struct { uint8_t Size ; @@ -105,6 +107,9 @@ typedef struct #elif defined( __STM32F1__) #define MAXCV (EEPROM_PAGE_SIZE/4 - 1) // number of storage places (CV address could be larger // because STM32 uses virtual adresses) + #undef ALLOW_NESTED_IRQ // This is done with NVIC on STM32 + #define PRIO_DCC_IRQ 9 + #define PRIO_SYSTIC 8 // MUST be higher priority than DCC Irq #else #define MAXCV E2END // the upper limit of the CV value currently defined to max memory. #endif From 462025b9fe1fd980f4f20a6aec9e1a1199e40e59 Mon Sep 17 00:00:00 2001 From: Bob Jacobsen Date: Mon, 5 Aug 2019 16:10:21 -0500 Subject: [PATCH 6/9] Comment update (#27) Incredibly trivial update to comment noticed while reading code. --- NmraDcc.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NmraDcc.cpp b/NmraDcc.cpp index 4034f53..9e0295c 100644 --- a/NmraDcc.cpp +++ b/NmraDcc.cpp @@ -1101,7 +1101,7 @@ void execDccProcessor( DCC_MSG * pDccMsg ) DccProcState.DuplicateCount = 0 ; memcpy( &DccProcState.LastMsg, pDccMsg, sizeof( DCC_MSG ) ) ; } - // Wait until you see 2 identicle packets before acting on a Service Mode Packet + // Wait until you see 2 identical packets before acting on a Service Mode Packet else { DccProcState.DuplicateCount++ ; From 828b1feaba372d86c7292a0575403ae78b180361 Mon Sep 17 00:00:00 2001 From: Alex Shepherd Date: Fri, 9 Aug 2019 08:19:53 +1200 Subject: [PATCH 7/9] corrected method description --- NmraDcc.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/NmraDcc.h b/NmraDcc.h index cfc8f51..aa0adb7 100644 --- a/NmraDcc.h +++ b/NmraDcc.h @@ -699,9 +699,8 @@ extern void notifyCVResetFactoryDefault(void) __attribute__ ((weak)); */ extern void notifyCVAck(void) __attribute__ ((weak)); /*+ - * notifyAdvancedCVAck() Called when a CV write must be acknowledged. - * This callback must increase the current drawn by this - * decoder by at least 60mA for 6ms +/- 1ms. + * notifyAdvancedCVAck() Called when a CV write must be acknowledged via Advanced Acknowledgement. + * This callback must send the Advanced Acknowledgement via RailComm. * * Inputs: * None From 794128fe4b6cae966e3889748cbf134bb072cbb8 Mon Sep 17 00:00:00 2001 From: Roeland Date: Mon, 19 Aug 2019 08:59:29 +0200 Subject: [PATCH 8/9] Update NmraDcc.cpp (#28) Changed the Service Mode duplicate packet comparison to only compare the packet Size and Data fields and ignore the number of Preamble bits as some command stations appear to send different numbers of preamble bits for otherwise duplicated packets, so we detecting the required two identical packets in sequence and performing the ServiceMode Operation --- NmraDcc.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/NmraDcc.cpp b/NmraDcc.cpp index 9e0295c..66470fe 100644 --- a/NmraDcc.cpp +++ b/NmraDcc.cpp @@ -1096,7 +1096,8 @@ void execDccProcessor( DCC_MSG * pDccMsg ) { resetServiceModeTimer( 1 ) ; - if( memcmp( pDccMsg, &DccProcState.LastMsg, sizeof( DCC_MSG ) ) ) + //Check if size and data content of the DCC match with previous packed + if(pDccMsg->Size != DccProcState.LastMsg.Size || memcmp( pDccMsg->Data, &DccProcState.LastMsg.Data, pDccMsg->Size ) != 0 ) { DccProcState.DuplicateCount = 0 ; memcpy( &DccProcState.LastMsg, pDccMsg, sizeof( DCC_MSG ) ) ; From d8fb99ed919790838efbca3aee294b28ce7e2f66 Mon Sep 17 00:00:00 2001 From: Alex Shepherd Date: Mon, 19 Aug 2019 19:04:51 +1200 Subject: [PATCH 9/9] Update NmraDcc.cpp Added some more comments to the ServiceMode programming DCC Packet comparison logic --- NmraDcc.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NmraDcc.cpp b/NmraDcc.cpp index 66470fe..978da60 100644 --- a/NmraDcc.cpp +++ b/NmraDcc.cpp @@ -1096,7 +1096,7 @@ void execDccProcessor( DCC_MSG * pDccMsg ) { resetServiceModeTimer( 1 ) ; - //Check if size and data content of the DCC match with previous packed + //Only check the DCC Packet "Size" and "Data" fields and ignore the "PreambleBits" as they can be different to the previous packet if(pDccMsg->Size != DccProcState.LastMsg.Size || memcmp( pDccMsg->Data, &DccProcState.LastMsg.Data, pDccMsg->Size ) != 0 ) { DccProcState.DuplicateCount = 0 ;