diff --git a/extras/tests/simavr_based/test_issue208/expect.txt b/extras/tests/simavr_based/test_issue208/expect.txt index d36fdf3..eac920c 100644 --- a/extras/tests/simavr_based/test_issue208/expect.txt +++ b/extras/tests/simavr_based/test_issue208/expect.txt @@ -2,17 +2,17 @@ DirB: 1*L->H, 0*H->L EnableA: 0*L->H, 0*H->L EnableB: 0*L->H, 0*H->L - StepA: 235908*L->H, 235908*H->L, Max High=11us Total High=1237142us - StepB: 2*L->H, 2*H->L, Max High=13us Total High=17us -Position[A]=37462 + StepA: 235907*L->H, 235907*H->L, Max High=10us Total High=927140us + StepB: 2*L->H, 2*H->L, Max High=12us Total High=17us +Position[A]=37463 Position[B]=2 -Time in FillISR max=769 us, total=1237221 us +Time in FillISR max=768 us, total=1213765 us -Time in StepA max=11 us, total=1237142 us +Time in StepA max=10 us, total=927140 us -Time in StepB max=13 us, total=17 us +Time in StepB max=12 us, total=17 us -Time in StepISR max=7 us, total=1021992 us +Time in StepISR max=6 us, total=912200 us diff --git a/extras/tests/simavr_based/test_issue250_30us/.gitignore b/extras/tests/simavr_based/test_issue250_30us/.gitignore deleted file mode 100644 index 8eba6c8..0000000 --- a/extras/tests/simavr_based/test_issue250_30us/.gitignore +++ /dev/null @@ -1 +0,0 @@ -src/ diff --git a/extras/tests/simavr_based/test_issue250_30us/Makefile b/extras/tests/simavr_based/test_issue250_30us/Makefile deleted file mode 100644 index ba9091d..0000000 --- a/extras/tests/simavr_based/test_issue250_30us/Makefile +++ /dev/null @@ -1,131 +0,0 @@ -# -# In order to execute the test for one directory use: -# -# make -C test_sd_01b_328p -f ../Makefile.test - -SRC=$(wildcard ../../../src/*) $(wildcard src/*) - -# platformio should contain only one env section. -# This section states the dut name -# atmega168 -# atmega168p -# atmega328 -# atmega328p -# atmega2560_timer1 -# atmega2560_timer3 -# atmega2560_timer4 -# atmega2560_timer5 -# - -DUT=$(shell gawk '/env:/{print(substr($$1,6,length($$1)-6))}' platformio.ini) - -TRACES=-at StepISR=trace@0x25/0x08 # PB3 -TRACES+=-at FillISR=trace@0x25/0x10 # PB4 - -# -ifeq ($(DUT),atmega2560_timer1) -DEVICE=atmega2560 -TRACES+=-at StepA=trace@0x025/0x20 #OC1A PB5 11 ATMega2560 -TRACES+=-at StepB=trace@0x025/0x40 #OC1B PB6 12 ATMega2560 -TRACES+=-at StepC=trace@0x025/0x80 #OC1C PB7 13 ATMega2560 -# -else ifeq ($(DUT),atmega2560_timer3) -DEVICE=atmega2560 -TRACES+=-at StepA=trace@0x02e/0x08 #OC3A PE3 5 ATMega2560 -TRACES+=-at StepB=trace@0x02e/0x10 #OC3B PE4 2 ATMega2560 -TRACES+=-at StepC=trace@0x02e/0x20 #OC3C PE5 3 ATMega2560 -# -else ifeq ($(DUT),atmega2560_timer4) -DEVICE=atmega2560 -TRACES+=-at StepA=trace@0x102/0x08 #OC4A PH3 6 ATMega2560 -TRACES+=-at StepB=trace@0x102/0x10 #OC4B PH4 7 ATMega2560 -TRACES+=-at StepC=trace@0x102/0x20 #OC4C PH5 8 ATMega2560 -# -else ifeq ($(DUT),atmega2560_timer5) -DEVICE=atmega2560 -TRACES+=-at StepA=trace@0x10b/0x08 #OC5A PL3 46 ATMega2560 -TRACES+=-at StepB=trace@0x10b/0x10 #OC5B PL4 45 ATMega2560 -TRACES+=-at StepC=trace@0x10b/0x20 #OC5C PL5 44 ATMega2560 - -else ifeq ($(DUT),atmega168) -DEVICE=atmega168 -TRACES+=-at StepA=trace@0x25/0x02 #OC1A PB1 9 atmega168 -TRACES+=-at StepB=trace@0x25/0x04 #OC1B PB2 10 atmega168 - -else ifeq ($(DUT),atmega168p) -DEVICE=atmega168p -TRACES+=-at StepA=trace@0x25/0x02 #OC1A PB1 9 atmega168p -TRACES+=-at StepB=trace@0x25/0x04 #OC1B PB2 10 atmega168p - -else ifeq ($(DUT),atmega328) -DEVICE=atmega328 -TRACES+=-at StepA=trace@0x25/0x02 #OC1A PB1 9 ATMega328 -TRACES+=-at StepB=trace@0x25/0x04 #OC1B PB2 10 ATMega328 - -else ifeq ($(DUT),atmega328p) -DEVICE=atmega328p -TRACES+=-at StepA=trace@0x25/0x02 #OC1A PB1 9 ATMega328p -TRACES+=-at StepB=trace@0x25/0x04 #OC1B PB2 10 ATMega328p - -else ifeq ($(DUT),atmega32u4) -DEVICE=atmega32u4 -TRACES+=-at StepA=trace@0x025/0x20 #OC1A PB5 11 -TRACES+=-at StepB=trace@0x025/0x40 #OC1B PB6 12 -#TRACES+=-at StepC=trace@0x025/0x80 #OC1C PB7 13 - -endif - -ifeq ($(DEVICE),atmega2560) -TRACES+=-at DirA=trace@0x2b/0x01 # Pin 21 PD0 -TRACES+=-at DirB=trace@0x2b/0x02 # Pin 20 PD1 -TRACES+=-at DirC=trace@0x10b/0x80 # Pin 42 PL7 -TRACES+=-at EnableA=trace@0x2b/0x04 # Pin 19 PD2 -TRACES+=-at EnableB=trace@0x2b/0x08 # Pin 18 PD3 -TRACES+=-at EnableC=trace@0x10b/0x40 # Pin 43 PL6 - -else ifeq ($(DEVICE),$(filter $(DEVICE),atmega168 atmega168p atmega328 atmega328p)) -TRACES+=-at DirA=trace@0x2b/0x20 # Pin 5 PD5 -TRACES+=-at DirB=trace@0x2b/0x80 # Pin 7 PD7 -TRACES+=-at EnableA=trace@0x2b/0x40 # Pin 6 PD6 -TRACES+=-at EnableB=trace@0x25/0x01 # Pin 8 PB0 - -else ifeq ($(DUT),atmega32u4) -TRACES+=-at DirA=trace@0x25/0x10 # Pin 26 PB4 -TRACES+=-at DirB=trace@0x25/0x08 # Pin 14 PB3 -#TRACES+=-at DirC=trace@0x10b/0x80 # Pin 42 PL7 -TRACES+=-at EnableA=trace@0x25/0x04 # Pin 16 PB2 -TRACES+=-at EnableB=trace@0x25/0x02 # Pin 15 PB1 -#TRACES+=-at EnableC=trace@0x10b/0x40 # Pin 43 PL6 - -endif - -FIRMWARE=".pio/build/$(DUT)/firmware.elf" - -DIR=$(shell env pwd) - -ifndef SILENCE - SILENCE=0 -endif - -test: .tested - -.tested: result.txt ../judge_pos0.awk - echo DUT=$(DUT) - rm -f .tested - gawk -f ../judge_pos0.awk -v DIR=$(DIR) result.txt - test -f .tested - -result.txt: x.vcd - gawk -v SILENCE=$(SILENCE) -f ../eval.awk x.vcd >/dev/null - -x.vcd: $(SRC) ../run_avr platformio.ini src/Issue250.ino - ~/.platformio/penv/bin/pio run -e $(DUT) || ~/.local/bin/pio run -e $(DUT) || env pio run -e $(DUT) - ../run_avr $(FIRMWARE) -m $(DEVICE) -o x.vcd $(TRACES) - -src/Issue250.ino: - mkdir -p src - cd src; ln -s ../../../../../examples/Issue250/Issue250.ino . - -clean: - rm -fR .pio .tested x.vcd result.txt - diff --git a/extras/tests/simavr_based/test_issue250_30us/expect.txt b/extras/tests/simavr_based/test_issue250_30us/expect.txt deleted file mode 100644 index e69de29..0000000 diff --git a/extras/tests/simavr_based/test_issue250_30us/platformio.ini b/extras/tests/simavr_based/test_issue250_30us/platformio.ini deleted file mode 100644 index 4136b31..0000000 --- a/extras/tests/simavr_based/test_issue250_30us/platformio.ini +++ /dev/null @@ -1,32 +0,0 @@ -; PlatformIO Project Configuration File -; -; Build options: build flags, source filter -; Upload options: custom upload port, speed and extra flags -; Library options: dependencies, extra library storages -; Advanced options: extra scripting -; -; Please visit documentation for the other options and examples -; https://docs.platformio.org/page/projectconf.html - -[platformio] - -# There should be only one env section for the DUT under test. -# One of -# atmega168p -# atmega328p -# atmega2560_timer1 -# atmega2560_timer3 -# atmega2560_timer4 -# atmega2560_timer5 -# -[common] -# This is the line input to StepperDemo: -#build_flags = -D SIMULATOR -D SIMAVR_TIME_MEASUREMENT -D BLOCK_INTERRUPT_US=30 -build_flags = -D SIMULATOR -D SIMAVR_TIME_MEASUREMENT -D BLOCK_INTERRUPT_US=30 -D LOOPS=100 -D TOGGLE_DIRECTION=false -D USE_MOVETO=false - -[env:atmega328p] -platform = atmelavr -board = nanoatmega328 -framework = arduino -build_flags = -Werror -Wall ${common.build_flags} -lib_extra_dirs = ../../../../.. diff --git a/extras/tests/simavr_based/test_sd_04_timing_2560/expect.txt b/extras/tests/simavr_based/test_sd_04_timing_2560/expect.txt index 04aa065..559ecba 100644 --- a/extras/tests/simavr_based/test_sd_04_timing_2560/expect.txt +++ b/extras/tests/simavr_based/test_sd_04_timing_2560/expect.txt @@ -4,9 +4,9 @@ EnableA: 1*L->H, 1*H->L EnableB: 2*L->H, 1*H->L EnableC: 2*L->H, 1*H->L - StepA: 64000*L->H, 64000*H->L, Max High=25us Total High=396526us - StepB: 64000*L->H, 64000*H->L, Max High=35us Total High=418067us - StepC: 64000*L->H, 64000*H->L, Max High=41us Total High=635308us + StepA: 64000*L->H, 64000*H->L, Max High=26us Total High=479601us + StepB: 64000*L->H, 64000*H->L, Max High=30us Total High=368752us + StepC: 64000*L->H, 64000*H->L, Max High=36us Total High=379262us Position[A]=64000 Position[B]=64000 @@ -17,13 +17,13 @@ Time in EnableA max=233598 us, total=233598 us Time in EnableB max=246080 us, total=246080 us -Time in EnableC max=254236 us, total=254236 us +Time in EnableC max=254234 us, total=254234 us -Time in FillISR max=2866 us, total=2158540 us +Time in FillISR max=2805 us, total=2111191 us -Time in StepA max=25 us, total=396526 us +Time in StepA max=26 us, total=479601 us -Time in StepB max=35 us, total=418067 us +Time in StepB max=30 us, total=368752 us -Time in StepC max=41 us, total=635308 us +Time in StepC max=36 us, total=379262 us diff --git a/extras/tests/simavr_based/test_sd_04_timing_328p/expect.txt b/extras/tests/simavr_based/test_sd_04_timing_328p/expect.txt index 2a62855..573edde 100644 --- a/extras/tests/simavr_based/test_sd_04_timing_328p/expect.txt +++ b/extras/tests/simavr_based/test_sd_04_timing_328p/expect.txt @@ -2,21 +2,21 @@ DirB: 1*L->H, 0*H->L EnableA: 1*L->H, 1*H->L EnableB: 2*L->H, 1*H->L - StepA: 1000*L->H, 1000*H->L, Max High=15us Total High=5588us - StepB: 1000*L->H, 1000*H->L, Max High=15us Total High=5832us + StepA: 1000*L->H, 1000*H->L, Max High=14us Total High=4334us + StepB: 1000*L->H, 1000*H->L, Max High=14us Total High=4915us Position[A]=1000 Position[B]=1000 Time in EnableA max=225398 us, total=225398 us -Time in EnableB max=238117 us, total=238117 us +Time in EnableB max=238115 us, total=238115 us -Time in FillISR max=2645 us, total=47801 us +Time in FillISR max=2640 us, total=47588 us -Time in StepA max=15 us, total=5588 us +Time in StepA max=14 us, total=4334 us -Time in StepB max=15 us, total=5832 us +Time in StepB max=14 us, total=4915 us -Time in StepISR max=7 us, total=9366 us +Time in StepISR max=6 us, total=8679 us diff --git a/extras/tests/simavr_based/test_sd_04_timing_328p_37k/expect.txt b/extras/tests/simavr_based/test_sd_04_timing_328p_37k/expect.txt index 6b154c7..e6e67ac 100644 --- a/extras/tests/simavr_based/test_sd_04_timing_328p_37k/expect.txt +++ b/extras/tests/simavr_based/test_sd_04_timing_328p_37k/expect.txt @@ -2,15 +2,15 @@ DirB: 1*L->H, 0*H->L EnableA: 1*L->H, 1*H->L EnableB: 1*L->H, 0*H->L - StepA: 1000*L->H, 1000*H->L, Max High=10us Total High=5265us + StepA: 1000*L->H, 1000*H->L, Max High=11us Total High=3969us StepB: 0*L->H, 0*H->L, Max High=0us Total High=0us Position[A]=1000 Time in EnableA max=225399 us, total=225399 us -Time in FillISR max=2015 us, total=27562 us +Time in FillISR max=2013 us, total=27494 us -Time in StepA max=10 us, total=5265 us +Time in StepA max=11 us, total=3969 us -Time in StepISR max=7 us, total=4614 us +Time in StepISR max=5 us, total=4031 us diff --git a/src/StepperISR_avr.cpp b/src/StepperISR_avr.cpp index fec1612..1292acd 100644 --- a/src/StepperISR_avr.cpp +++ b/src/StepperISR_avr.cpp @@ -12,9 +12,10 @@ // Toggle is 0 1 // Disconnect is 0 0 // -#define Stepper_OneToZero(T, X) TCCR##T##A = TCCR##T##A & ~_BV(COM##T##X##0) #define Stepper_Zero(T, X) \ TCCR##T##A = (TCCR##T##A | _BV(COM##T##X##1)) & ~_BV(COM##T##X##0) +// Force compare of Stepper_Toggle appears to be broken in simavr +// In other words, use of Stepper_Toggle yields errors in simavr, for which root cause is unclear #define Stepper_Toggle(T, X) \ TCCR##T##A = (TCCR##T##A | _BV(COM##T##X##0)) & ~_BV(COM##T##X##1) #define Stepper_One(T, X) TCCR##T##A |= _BV(COM##T##X##1) | _BV(COM##T##X##0) @@ -133,78 +134,68 @@ void StepperQueue::init(uint8_t queue_num, uint8_t step_pin) { // Remark: Interrupt Flag is automatically cleared on ISR execution // // If reaching here without further commands, then the queue is done -#define AVR_STEPPER_ISR(T, CHANNEL) \ - ISR(TIMER##T##_COMP##CHANNEL##_vect) { \ - enterStepperISR(); \ - uint8_t rp = fas_queue_##CHANNEL.read_idx; \ - if (rp == fas_queue_##CHANNEL.next_write_idx) { \ - /* queue is empty => set to disconnect */ \ - Stepper_Disconnect(T, CHANNEL); \ - /* force compare to ensure disconnect */ \ - ForceCompare(T, CHANNEL); \ - /* disable compare interrupt */ \ - DisableCompareInterrupt(T, CHANNEL); \ - fas_queue_##CHANNEL._isRunning = false; \ - fas_queue_##CHANNEL._noMoreCommands = false; \ - exitStepperISR(); \ - return; \ - } \ - struct queue_entry* e = &fas_queue_##CHANNEL.entry[rp & QUEUE_LEN_MASK]; \ - /* There is a risk, that this new compare time is delayed by one cycle */ \ - OCR##T##CHANNEL += e->ticks; \ - if (Stepper_IsOneIfOutput(T, CHANNEL)) { \ - /* Clear output bit by another compare event */ \ - Stepper_OneToZero(T, CHANNEL); \ - ForceCompare(T, CHANNEL); \ - if (e->steps-- > 1) { \ - /* perform another step with this queue entry */ \ - Stepper_One(T, CHANNEL); \ - exitStepperISR(); \ - return; \ - } \ - } else if (fas_queue_##CHANNEL._noMoreCommands) { \ - /* new command received after running out of commands */ \ - /* if this new command requires a step, then this step would be lost \ - */ \ - fas_queue_##CHANNEL._noMoreCommands = false; \ - if (e->toggle_dir) { \ - Stepper_ToggleDirection(CHANNEL); \ - } \ - if (e->steps > 0) { \ - /* That's the problem, so generate a step */ \ - Stepper_One(T, CHANNEL); \ - ForceCompare(T, CHANNEL); \ - /* Use a high time of 3us */ \ - delayMicroseconds(3); \ - /* Clear output bit by another toggle */ \ - Stepper_OneToZero(T, CHANNEL); \ - ForceCompare(T, CHANNEL); \ - if (e->steps-- > 1) { \ - /* perform another step with this queue entry */ \ - Stepper_One(T, CHANNEL); \ - exitStepperISR(); \ - return; \ - } \ - } \ - } \ - if (TEST_NOT_REPEATING_ENTRY) { \ - rp++; \ - } \ - fas_queue_##CHANNEL.read_idx = rp; \ - if (rp != fas_queue_##CHANNEL.next_write_idx) { \ - /* command in queue */ \ - e = &fas_queue_##CHANNEL.entry[rp & QUEUE_LEN_MASK]; \ - if (e->toggle_dir) { \ - Stepper_ToggleDirection(CHANNEL); \ - } \ - if (e->steps != 0) { \ - Stepper_One(T, CHANNEL); \ - } \ - } else { \ - /* queue is empty, so wait this command to complete, then disconnect */ \ - fas_queue_##CHANNEL._noMoreCommands = true; \ - } \ - exitStepperISR(); \ +#define AVR_STEPPER_ISR(T, CHANNEL) \ + ISR(TIMER##T##_COMP##CHANNEL##_vect) { \ + enterStepperISR(); \ + uint8_t rp = fas_queue_##CHANNEL.read_idx; \ + uint8_t wp = fas_queue_##CHANNEL.next_write_idx; \ + if (rp == wp) { \ + /* queue is empty => set to disconnect */ \ + /* disable compare interrupt */ \ + DisableCompareInterrupt(T, CHANNEL); \ + Stepper_Disconnect(T, CHANNEL); \ + /* force compare to ensure disconnect */ \ + ForceCompare(T, CHANNEL); \ + fas_queue_##CHANNEL._isRunning = false; \ + fas_queue_##CHANNEL._noMoreCommands = false; \ + exitStepperISR(); \ + return; \ + } \ + struct queue_entry* e = &fas_queue_##CHANNEL.entry[rp & QUEUE_LEN_MASK]; \ + /* There is a risk, that this new compare time is delayed by one cycle */ \ + uint16_t ticks = e->ticks; \ + /* Set output to zero, this works in any case. In case of pause: no-op */ \ + Stepper_Zero(T, CHANNEL); \ + ForceCompare(T, CHANNEL); \ + if (e->steps > 1) { \ + /* perform another step with this queue entry, toggle mode is active */ \ + e->steps--; \ + Stepper_Toggle(T, CHANNEL); \ + } else { \ + /* either pause command or no more steps */ \ + if (fas_queue_##CHANNEL._noMoreCommands) { \ + /* new command received after running out of commands */ \ + /* if this new command requires a step, then this step would be lost */ \ + fas_queue_##CHANNEL._noMoreCommands = false; \ + if (e->steps != 0) { \ + /* New command needs steps, so do it immediately */ \ + ticks = 10; \ + } \ + } \ + else if (TEST_NOT_REPEATING_ENTRY) { \ + rp++; \ + fas_queue_##CHANNEL.read_idx = rp; \ + if (rp == wp) { \ + /* queue is empty, wait this command to complete, then disconnect */ \ + Stepper_Zero(T, CHANNEL); \ + fas_queue_##CHANNEL._noMoreCommands = true; \ + OCR##T##CHANNEL += ticks; \ + exitStepperISR(); \ + return; \ + } \ + e = &fas_queue_##CHANNEL.entry[rp & QUEUE_LEN_MASK]; \ + } \ + if (e->toggle_dir) { \ + Stepper_ToggleDirection(CHANNEL); \ + } \ + if (e->steps != 0) { \ + Stepper_Toggle(T, CHANNEL); \ + } else { \ + Stepper_Zero(T, CHANNEL); \ + } \ + } \ + OCR##T##CHANNEL += ticks; \ + exitStepperISR(); \ } #define AVR_STEPPER_ISR_GEN(T, CHANNEL) AVR_STEPPER_ISR(T, CHANNEL) @@ -250,7 +241,6 @@ AVR_CYCLIC_ISR_GEN(FAS_TIMER_MODULE) Stepper_Zero(T, CHANNEL); \ if (e->steps != 0) { \ Stepper_Toggle(T, CHANNEL); \ - e->steps--; \ } \ /* clear interrupt flag */ \ ClearInterruptFlag(T, CHANNEL); \