From befb41ce981e8e8fcece4c00f242a143f3e2cf80 Mon Sep 17 00:00:00 2001 From: Harald Barth Date: Sun, 18 Jun 2023 09:48:15 +0200 Subject: [PATCH 1/5] check ADCee::init() return value --- MotorDriver.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/MotorDriver.cpp b/MotorDriver.cpp index 678a28a..e41e150 100644 --- a/MotorDriver.cpp +++ b/MotorDriver.cpp @@ -108,8 +108,13 @@ MotorDriver::MotorDriver(int16_t power_pin, byte signal_pin, byte signal_pin2, i } currentPin=current_pin; - if (currentPin!=UNUSED_PIN) - ADCee::init(currentPin); + if (currentPin!=UNUSED_PIN) { + int ret = ADCee::init(currentPin); + if (ret < -1010) { // XXX give value a name later + DIAG(F("ADCee::init error %d, disable current pin %d"), ret, currentPin); + currentPin = UNUSED_PIN; + } + } senseOffset=0; // value can not be obtained until waveform is activated if (fault_pin != UNUSED_PIN) { From 02669368758977411c83ffb3896556dd37b547e4 Mon Sep 17 00:00:00 2001 From: Harald Barth Date: Sun, 18 Jun 2023 19:26:38 +0200 Subject: [PATCH 2/5] STM32: Use mask as loop variable --- DCCTimerSTM32.cpp | 28 +++++++++++++++++++--------- GITHUB_SHA.h | 2 +- TrackManager.h | 12 +++++++++--- 3 files changed, 29 insertions(+), 13 deletions(-) diff --git a/DCCTimerSTM32.cpp b/DCCTimerSTM32.cpp index a45c4d4..7ed2b95 100644 --- a/DCCTimerSTM32.cpp +++ b/DCCTimerSTM32.cpp @@ -30,6 +30,10 @@ #ifdef ARDUINO_ARCH_STM32 #include "DCCTimer.h" +#ifdef DEBUG_ADC +#include "TrackManager.h" +#endif +#include "DIAG.h" #if defined(ARDUINO_NUCLEO_F411RE) // Nucleo-64 boards don't have Serial1 defined by default @@ -307,6 +311,8 @@ int ADCee::init(uint8_t pin) { analogchans[id] = adcchan; // Keep track of which ADC channel is used for reading this pin usedpins |= (1 << id); // This pin is now ready + DIAG(F("ADCee::init(): value=%d, channel=%d, id=%d"), value, adcchan, id); + return value; } @@ -340,11 +346,13 @@ void ADCee::scan() { // found value analogvals[id] = ADC1->DR; // advance at least one track - // for scope debug TrackManager::track[1]->setBrake(0); +#ifdef DEBUG_ADC + if (id == 1) TrackManager::track[1]->setBrake(0); +#endif waiting = false; id++; mask = mask << 1; - if (id == NUM_ADC_INPUTS+1) { + if (mask == 0) { // the 1 has been shifted out id = 0; mask = 1; } @@ -355,18 +363,20 @@ void ADCee::scan() { // look for a valid track to sample or until we are around while (true) { if (mask & usedpins) { - // start new ADC aquire on id + // start new ADC aquire on id ADC1->SQR3 = analogchans[id]; //1st conversion in regular sequence ADC1->CR2 |= (1 << 30); //Start 1st conversion SWSTART - // for scope debug TrackManager::track[1]->setBrake(1); - waiting = true; - return; +#ifdef DEBUG_ADC + if (id == 1) TrackManager::track[1]->setBrake(1); +#endif + waiting = true; + return; } id++; mask = mask << 1; - if (id == NUM_ADC_INPUTS+1) { - id = 0; - mask = 1; + if (mask == 0) { // the 1 has been shifted out + id = 0; + mask = 1; } } } diff --git a/GITHUB_SHA.h b/GITHUB_SHA.h index 7ef8904..8bd3812 100644 --- a/GITHUB_SHA.h +++ b/GITHUB_SHA.h @@ -1 +1 @@ -#define GITHUB_SHA "devel-202306031954Z" +#define GITHUB_SHA "devel-202306181725Z" diff --git a/TrackManager.h b/TrackManager.h index ef4a47c..19e756d 100644 --- a/TrackManager.h +++ b/TrackManager.h @@ -84,8 +84,15 @@ class TrackManager { static int16_t joinRelay; static bool progTrackSyncMain; // true when prog track is a siding switched to main - static bool progTrackBoosted; // true when prog track is not current limited - + static bool progTrackBoosted; // true when prog track is not current limited + +#ifdef DEBUG_ADC + public: +#else + private: +#endif + static MotorDriver* track[MAX_TRACKS]; + private: static void addTrack(byte t, MotorDriver* driver); static byte lastTrack; @@ -93,7 +100,6 @@ class TrackManager { static POWERMODE mainPowerGuess; static void applyDCSpeed(byte t); - static MotorDriver* track[MAX_TRACKS]; static TRACK_MODE trackMode[MAX_TRACKS]; static int16_t trackDCAddr[MAX_TRACKS]; // dc address if TRACK_MODE_DC or TRACK_MODE_DCX #ifdef ARDUINO_ARCH_ESP32 From 7783837545d1f6f937726083c79605cb23242603 Mon Sep 17 00:00:00 2001 From: Harald Barth Date: Sun, 18 Jun 2023 21:08:52 +0200 Subject: [PATCH 3/5] Back out this as it is bigger and slower This reverts commit efb26660603eb02d056b1d6c9c7a7b4124213269. --- DCCTimer.h | 14 ++++---- DCCTimerAVR.cpp | 91 ++++++++++++++++++++++++++----------------------- 2 files changed, 55 insertions(+), 50 deletions(-) diff --git a/DCCTimer.h b/DCCTimer.h index 7a9d940..29b8819 100644 --- a/DCCTimer.h +++ b/DCCTimer.h @@ -105,9 +105,14 @@ private: // that an offset can be initialized. class ADCee { public: - // init does add the pin to the list of scanned pins (if this + // begin is called for any setup that must be done before + // **init** can be called. On some architectures this involves ADC + // initialisation and clock routing, sampling times etc. + static void begin(); + // init adds the pin to the list of scanned pins (if this // platform's implementation scans pins) and returns the first - // read value. It is called before the regular scan is started. + // read value (which is why it required begin to have been called first!) + // It must be called before the regular scan is started. static int init(uint8_t pin); // read does read the pin value from the scanned cache or directly // if this is a platform that does not scan. fromISR is a hint if @@ -116,9 +121,6 @@ public: static int read(uint8_t pin, bool fromISR=false); // returns possible max value that the ADC can return static int16_t ADCmax(); - // begin is called for any setup that must be done before - // scan can be called. - static void begin(); private: // On platforms that scan, it is called from waveform ISR // only on a regular basis. @@ -127,8 +129,6 @@ private: static uint16_t usedpins; // cached analog values (malloc:ed to actual number of ADC channels) static int *analogvals; - // ids to scan (new way) - static byte *idarr; // friend so that we can call scan() and begin() friend class DCCWaveform; }; diff --git a/DCCTimerAVR.cpp b/DCCTimerAVR.cpp index 40ce0fb..9b16c47 100644 --- a/DCCTimerAVR.cpp +++ b/DCCTimerAVR.cpp @@ -123,14 +123,13 @@ void DCCTimer::reset() { } #if defined(ARDUINO_AVR_MEGA) || defined(ARDUINO_AVR_MEGA2560) -#define NUM_ADC_INPUTS 16 +#define NUM_ADC_INPUTS 15 #else -#define NUM_ADC_INPUTS 8 +#define NUM_ADC_INPUTS 7 #endif uint16_t ADCee::usedpins = 0; int * ADCee::analogvals = NULL; -byte *ADCee::idarr = NULL; -static bool ADCusesHighPort = false; +bool ADCusesHighPort = false; /* * Register a new pin to be scanned @@ -139,28 +138,16 @@ static bool ADCusesHighPort = false; */ int ADCee::init(uint8_t pin) { uint8_t id = pin - A0; - byte n; - if (id >= NUM_ADC_INPUTS) + if (id > NUM_ADC_INPUTS) return -1023; if (id > 7) ADCusesHighPort = true; pinMode(pin, INPUT); int value = analogRead(pin); - if (analogvals == NULL) { - analogvals = (int *)calloc(NUM_ADC_INPUTS, sizeof(int)); - for (n=0 ; n < NUM_ADC_INPUTS; n++) // set unreasonable value at startup as marker - analogvals[n] = -32768; // 16 bit int min value - idarr = (byte *)calloc(NUM_ADC_INPUTS+1, sizeof(byte)); // +1 for terminator value - for (n=0 ; n <= NUM_ADC_INPUTS; n++) - idarr[n] = 255; // set 255 as end of array marker - } - analogvals[id] = value; // store before enable by idarr[n] - for (n=0 ; n <= NUM_ADC_INPUTS; n++) { - if (idarr[n] == 255) { - idarr[n] = id; - break; - } - } + if (analogvals == NULL) + analogvals = (int *)calloc(NUM_ADC_INPUTS+1, sizeof(int)); + analogvals[id] = value; + usedpins |= (1<setBrake(0); waiting = false; + id++; + mask = mask << 1; + if (id == NUM_ADC_INPUTS+1) { + id = 0; + mask = 1; + } } if (!waiting) { - // cycle around in-use analogue pins - num++; - if (idarr[num] == 255) - num = 0; - // start new ADC aquire on id + if (usedpins == 0) // otherwise we would loop forever + return; + // look for a valid track to sample or until we are around + while (true) { + if (mask & usedpins) { + // start new ADC aquire on id #if defined(ADCSRB) && defined(MUX5) - if (ADCusesHighPort) { // if we ever have started to use high pins) - if (idarr[num] > 7) // if we use a high ADC pin - bitSet(ADCSRB, MUX5); // set MUX5 bit - else - bitClear(ADCSRB, MUX5); - } + if (ADCusesHighPort) { // if we ever have started to use high pins) + if (id > 7) // if we use a high ADC pin + bitSet(ADCSRB, MUX5); // set MUX5 bit + else + bitClear(ADCSRB, MUX5); + } #endif - ADMUX = (1 << REFS0) | (idarr[num] & 0x07); // select AVCC as reference and set MUX - bitSet(ADCSRA, ADSC); // start conversion - waiting = true; + ADMUX=(1<setBrake(1); + waiting = true; + return; + } + id++; + mask = mask << 1; + if (id == NUM_ADC_INPUTS+1) { + id = 0; + mask = 1; + } + } } } #pragma GCC pop_options @@ -231,4 +236,4 @@ void ADCee::begin() { //bitSet(ADCSRA, ADSC); //do not start the ADC yet. Done when we have set the MUX interrupts(); } -#endif \ No newline at end of file +#endif From 56fcb4e5f790086010c357a7da912ca066fb0532 Mon Sep 17 00:00:00 2001 From: Harald Barth Date: Mon, 19 Jun 2023 00:06:04 +0200 Subject: [PATCH 4/5] Optimize DCCTimerARV.cpp --- DCCTimer.h | 1 + DCCTimerAVR.cpp | 39 +++++++++++++++++++++++++-------------- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/DCCTimer.h b/DCCTimer.h index 29b8819..7402f16 100644 --- a/DCCTimer.h +++ b/DCCTimer.h @@ -127,6 +127,7 @@ private: static void scan(); // bit array of used pins (max 16) static uint16_t usedpins; + static uint8_t highestPin; // cached analog values (malloc:ed to actual number of ADC channels) static int *analogvals; // friend so that we can call scan() and begin() diff --git a/DCCTimerAVR.cpp b/DCCTimerAVR.cpp index 9b16c47..80cd245 100644 --- a/DCCTimerAVR.cpp +++ b/DCCTimerAVR.cpp @@ -1,6 +1,6 @@ /* * © 2021 Mike S - * © 2021-2022 Harald Barth + * © 2021-2023 Harald Barth * © 2021 Fred Decker * © 2021 Chris Harlow * © 2021 David Cutting @@ -29,6 +29,9 @@ #include #include #include "DCCTimer.h" +#ifdef DEBUG_ADC +#include "TrackManager.h" +#endif INTERRUPT_CALLBACK interruptHandler=0; // Arduino nano, uno, mega etc @@ -123,13 +126,14 @@ void DCCTimer::reset() { } #if defined(ARDUINO_AVR_MEGA) || defined(ARDUINO_AVR_MEGA2560) -#define NUM_ADC_INPUTS 15 +#define NUM_ADC_INPUTS 16 #else -#define NUM_ADC_INPUTS 7 +#define NUM_ADC_INPUTS 8 #endif uint16_t ADCee::usedpins = 0; +uint8_t ADCee::highestPin = 0; int * ADCee::analogvals = NULL; -bool ADCusesHighPort = false; +static bool ADCusesHighPort = false; /* * Register a new pin to be scanned @@ -138,16 +142,17 @@ bool ADCusesHighPort = false; */ int ADCee::init(uint8_t pin) { uint8_t id = pin - A0; - if (id > NUM_ADC_INPUTS) + if (id >= NUM_ADC_INPUTS) return -1023; if (id > 7) ADCusesHighPort = true; pinMode(pin, INPUT); int value = analogRead(pin); if (analogvals == NULL) - analogvals = (int *)calloc(NUM_ADC_INPUTS+1, sizeof(int)); + analogvals = (int *)calloc(NUM_ADC_INPUTS, sizeof(int)); analogvals[id] = value; usedpins |= (1< highestPin) highestPin = id; return value; } int16_t ADCee::ADCmax() { @@ -157,13 +162,15 @@ int16_t ADCee::ADCmax() { * Read function ADCee::read(pin) to get value instead of analogRead(pin) */ int ADCee::read(uint8_t pin, bool fromISR) { - (void)fromISR; // AVR does ignore this arg uint8_t id = pin - A0; if ((usedpins & (1<setBrake(0); +#ifdef DEBUG_ADC + if (id == 1) TrackManager::track[1]->setBrake(0); +#endif waiting = false; id++; mask = mask << 1; - if (id == NUM_ADC_INPUTS+1) { + if (id > highestPin) { // the 1 has been shifted out id = 0; mask = 1; } @@ -212,13 +221,15 @@ void ADCee::scan() { #endif ADMUX=(1<setBrake(1); +#ifdef DEBUG_ADC + if (id == 1) TrackManager::track[1]->setBrake(1); +#endif waiting = true; return; } id++; mask = mask << 1; - if (id == NUM_ADC_INPUTS+1) { + if (id > highestPin) { id = 0; mask = 1; } From d5dad767a42eed0730de8f288b7f0d5261151395 Mon Sep 17 00:00:00 2001 From: Harald Barth Date: Mon, 19 Jun 2023 00:09:27 +0200 Subject: [PATCH 5/5] version 4.2.55 --- GITHUB_SHA.h | 2 +- version.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/GITHUB_SHA.h b/GITHUB_SHA.h index 8bd3812..25e57bf 100644 --- a/GITHUB_SHA.h +++ b/GITHUB_SHA.h @@ -1 +1 @@ -#define GITHUB_SHA "devel-202306181725Z" +#define GITHUB_SHA "devel-202306182208Z" diff --git a/version.h b/version.h index c3d47d9..6355f6b 100644 --- a/version.h +++ b/version.h @@ -4,7 +4,8 @@ #include "StringFormatter.h" -#define VERSION "4.2.54" +#define VERSION "4.2.55" +// 4.2.55 - Optimize analog read for AVR // 4.2.54 - EX8874 shield in config.example.h // - Fix: Better warnings for pin number errors // - Fix: Default roster list possible in Withrottle and