Optimizing clock cycles?

JAJSentina:
Sir, I did look up on the progmem table examples. the serial.print is just to check if my program calls the right elements.

If you use the same access method as in your serial.print statement your check just failed.

I gave you an example how to access and print your data properly and a link to the documentation.

I will not read it to you.

JAJSentina:
Ps. Italics mess up my post so i used parenthesis instead of brackets

That is the reason why code should be enclosed in code tags, as described in
Read this before posting a programming question.

Sir thank you. using..

Serial.print(pgm_read_word_near(t1 + i))

...Did return the correct values.

Here is the new code. I have elimated the for loop statement. reduce the overall program. but the cycles did not significantly change. I dont know where I did something wrong :frowning:

Please see attachment for the code.

MyCode.txt (28 KB)

Hello, i have trimmed the coded such that the formulas have been eliminated. Thanks to the Progmem advice i was able to declare a large set of raw data.

Here is the code :

#include <avr/pgmspace.h>

const int switch_a_top = PG5;//PD4; //pin 4
const int switch_a_bot = PB7; //pin 13
const int switch_b_top = PH6;//pin 9
const int switch_b_bot = PB4;//pin 10
const int switch_c_top = PE4;//pin 2
const int switch_c_bot = PD3;//pin 5
uint32_t j = 0;
const uint16_t t1_load = 0;
const uint16_t t1_comp = 0;


const PROGMEM bool state_aT[] = {0, 0, 0, 1, 1, 1, 1, 1, 1,.............};
const PROGMEM bool state_bT[] = {1, 1, 1, 1, 1, 1, 0, 0, 0, ............};
const PROGMEM bool state_cT[] = {1, 0, 0, 1, 0, 0, 1, 1, 1, 1...........};


bool aT, bT, cT;

void setup() {
 //Serial.begin(2000000);
 DDRG |= (1 << switch_a_top);
 DDRB |= (1 << switch_a_bot);
 DDRH |= (1 << switch_b_top);
 DDRB |= (1 << switch_b_bot);
 DDRE |= (1 << switch_c_top);
 DDRD |= (1 << switch_c_bot);

 cli();// stop interrupts
 TCCR1A = 0; //reset the value
 TCCR1B = 0; //reset the value
 TCNT1 = 0; //reset the value
 OCR1A = 0; // compare match value
 TCCR1B = 0b00001001; //WGM12 bit is 1 and no prescaler

 TIMSK1 |= (1 << OCIE1A);

 sei();// enable interrupts
}// put your setup code here, to run once:


void loop() {
 // put your main code here, to run repeatedly:
}


ISR(TIMER1_COMPA_vect) {
 if (j >= 5760) {
   j = 0;
 }
 aT = pgm_read_word_near(state_aT + j);
 bT = pgm_read_word_near(state_bT + j);
 cT = pgm_read_word_near(state_cT + j);

 if (aT == 1) {
   PORTG |= (1 << switch_a_top);
   PORTB &= ~(1 << switch_a_bot);
   //Serial.print("hi");
 }
 else {
   PORTG &= ~(1 << switch_a_top);
   PORTB |= (1 << switch_a_bot);
 }
 if (bT == 1) {

   PORTH |= (1 << switch_b_top);
   PORTB &= ~(1 << switch_b_bot);
 }
 else
 {
   PORTH &= ~(1 << switch_b_top);
   PORTB |= (1 << switch_b_bot);
 }
 if (cT == 1)
 {
   PORTE |= (1 << switch_c_top);
   PORTD &= ~(1 << switch_c_bot);
 }
 else
 {
   PORTE &= ~(1 << switch_c_top);
   PORTD |= (1 << switch_c_bot);
 }
 j = j + 1;
}

I have the following results

My timing is off. 68ms instead of 16ms.

I tried to know when the state changes per loop and it took about 1792 clock cycles to run. Can i still reduce this clock cycle?

 aT = pgm_read_word_near(state_aT + j);

if (aT == 1) {

Make j an int rather than a long. Increment it immediately after reading pgmspace.
Make aT and etc local variables of the ISR.
Use pgm_read_byte() instead of pgm_read_word() (and modify the tables appropriately.)
That should result in better than 30% improvement of the ISR.
You could do even better by putting all the state table values into one byte and bit-testing instead of checking bools.
How often are you trying to change your pin states? (how many actual pin state transitions in your 16ms?)
Have you done any estimates that lead you to believe that it is actually possible?
5760 state changes in 16ms gives you about 44 AVR clock ticks for each one, and doing an ISR context switch plus 3 table reads plus 6 output pin changes in 44 cycles probably isn't possible. You current code is spending more than that just doing the context switch.

I don't know if this will help you, but this code generates a signal similar to your signal in the OP. It turns on a 38kHz signal on and off every 30ms on pin 11.

//interrupt for turning the 38Hz IR signal on/off
ISR (TIMER1_COMPA_vect)
{
  TCCR2A ^= _BV (COM2A0) ;  // Toggle OC2A on Compare Match  
  if ((TCCR2A & _BV (COM2A0)) == 0)
    digitalWrite (11, LOW);  // ensure off   
}  // end of TIMER1_COMPA_vect

void setup(){
    //timer for IR LEDs
    pinMode(11, OUTPUT); //OC2A pin
    TCCR1A = _BV(COM1A0); //CTC
    TCCR1B = _BV(WGM12) | _BV(CS12); //CTC, prescaler 256
    OCR1A = 1875; //Period of 30ms
    TIMSK1 = _BV (OCIE1A);   // enable Timer1 Interrupt
    //Set up timer 2 for 38kHz PWM. toggles on OCR1A value
    TCCR2A = _BV(WGM21) | _BV (COM2A0);    //CTC
    TCCR2B = _BV (CS20);   // No prescaler
    OCR2A =  209;          //frequency 38kHz
}

Have a look at #24 in thread: PWM for 3-phase Inverter - Motors, Mechanics, Power and CNC - Arduino Forum

Seems to me this is what you are trying to do?

westfw:
You could do even better by putting all the state table values into one byte and bit-testing instead of checking bools.

Sir, how do perform bit testing? From what i googled it involved bitread();

Metallor:

//interrupt for turning the 38Hz IR signal on/off

ISR (TIMER1_COMPA_vect)
{
  TCCR2A ^= _BV (COM2A0) ;  // Toggle OC2A on Compare Match 
  if ((TCCR2A & _BV (COM2A0)) == 0)
    digitalWrite (11, LOW);  // ensure off 
}  // end of TIMER1_COMPA_vect

void setup(){
    //timer for IR LEDs
    pinMode(11, OUTPUT); //OC2A pin
    TCCR1A = _BV(COM1A0); //CTC
    TCCR1B = _BV(WGM12) | _BV(CS12); //CTC, prescaler 256
    OCR1A = 1875; //Period of 30ms
    TIMSK1 = _BV (OCIE1A);  // enable Timer1 Interrupt
    //Set up timer 2 for 38kHz PWM. toggles on OCR1A value
    TCCR2A = _BV(WGM21) | _BV (COM2A0);    //CTC
    TCCR2B = _BV (CS20);  // No prescaler
    OCR2A =  209;          //frequency 38kHz
}

Thanks, I will try to test this program when I can.

MarkT:
Have a look at #24 in thread: PWM for 3-phase Inverter - Motors, Mechanics, Power and CNC - Arduino Forum

Sir, from what i understand this generates a nearly pure sine wave. What am am trying is to generate a modified PWM signal that averages as a sine wave.

Metallor:
I don't know if this will help you, but this code generates a signal similar to your signal in the OP. It turns on a 38kHz signal on and off every 30ms on pin 11.

//interrupt for turning the 38Hz IR signal on/off

ISR (TIMER1_COMPA_vect)
{
  TCCR2A ^= _BV (COM2A0) ;  // Toggle OC2A on Compare Match 
  if ((TCCR2A & _BV (COM2A0)) == 0)
    digitalWrite (11, LOW);  // ensure off 
}  // end of TIMER1_COMPA_vect

void setup(){
    //timer for IR LEDs
    pinMode(11, OUTPUT); //OC2A pin
    TCCR1A = _BV(COM1A0); //CTC
    TCCR1B = _BV(WGM12) | _BV(CS12); //CTC, prescaler 256
    OCR1A = 1875; //Period of 30ms
    TIMSK1 = _BV (OCIE1A);  // enable Timer1 Interrupt
    //Set up timer 2 for 38kHz PWM. toggles on OCR1A value
    TCCR2A = _BV(WGM21) | _BV (COM2A0);    //CTC
    TCCR2B = _BV (CS20);  // No prescaler
    OCR2A =  209;          //frequency 38kHz
}

Sir, I tried to compile the sketch. It says error compiling for Arduino board. Am I missing to include library?

This wasn't the complete code. You need a loop() function and also to include arduino.h at the top.

#include <Arduino.h>

//interrupt for turning the 38Hz IR signal on/off
ISR (TIMER1_COMPA_vect)
{
  TCCR2A ^= _BV (COM2A0) ;  // Toggle OC2A on Compare Match  
  if ((TCCR2A & _BV (COM2A0)) == 0)
    digitalWrite (11, LOW);  // ensure off   
}  // end of TIMER1_COMPA_vect

void setup(){
    //timer for IR LEDs
    pinMode(11, OUTPUT); //OC2A pin
    TCCR1A = _BV(COM1A0); //CTC
    TCCR1B = _BV(WGM12) | _BV(CS12); //CTC, prescaler 256
    OCR1A = 1875; //Period of 30ms
    TIMSK1 = _BV (OCIE1A);   // enable Timer1 Interrupt
    //Set up timer 2 for 38kHz PWM. toggles on OCR1A value
    TCCR2A = _BV(WGM21) | _BV (COM2A0);    //CTC
    TCCR2B = _BV (CS20);   // No prescaler
    OCR2A =  209;          //frequency 38kHz

void loop(){
}

I don't think this is exactly what you want, this always goes low between the PWM cycles, so you will need to modify it a bit if you find it suitable.

I am using this to drive an infrared LED at 38kHz, switching on and off at a period of 30ms. OCR1A and OCR2A will let you change the timing.

The idea was originally from this thread.

how do perform bit testing? From what i googled it involved bitread();

bitread() would work. The difficult part would be building your tables, probably.

/* Old code.  New code has the vertical columns of bits all encoded into
 *   just one byte
 * const PROGMEM bool state_aT[] = {0, 0, 0, 1,  1, 1, 1, 1, 1,.............};
 * const PROGMEM bool state_bT[] = {1, 1, 1, 1,  1, 1, 0, 0, 0, ............};
 * const PROGMEM bool state_cT[] = {1, 0, 0, 1,  0, 0, 1, 1, 1, 1...........};
*/

#define ASTATE 0  // a in bit 0
#define BSTATE 1  // b in bit 1
#define CSTART 2  // c in bit 2
// Convenient macro for defining the state bits
#define MAKEABC(A, B, C) ((A<<ASTATR)+(B<<BSTATE)+(C<<CSTATE))

const PROGMEM uint8_t states[] = {
    MAKEABC(0,1,1),   // 0
    MAKEABC(0,1,0),
    MAKEABC(0,1,0),
    MAKEABC(1,1,1),

    MAKEABC(1,1,0),  // 4
    MAKEABC(1,1,0),
    MAKEABC(1,0,1),
    MAKEABC(1,0,1),
//    :
};

ISR(TIMER1_COMPA_vect) {
    uint8_t T;
    
    if (j >= 5760) {
	j = 0;
    }
    T = pgm_read_byte_near(states + j);

    if (bitread(T, ASTATE)) {
	PORTG |= (1 << switch_a_top);
	PORTB &= ~(1 << switch_a_bot);
	//Serial.print("hi");
    } else {
	PORTG &= ~(1 << switch_a_top);
	PORTB |= (1 << switch_a_bot);
    }

    if (bitread(T, BSTATE)) {
	PORTH |= (1 << switch_b_top);
	PORTB &= ~(1 << switch_b_bot);
    } else {
	PORTH &= ~(1 << switch_b_top);
	PORTB |= (1 << switch_b_bot);
    }
// etc

What am am trying is to generate a modified PWM signal that averages as a sine wave.

You really haven't defined exactly what it is you're trying to do very well.
First of all, you seem to be manipulating at least 6 outputs.
Your original picture has weird banding, probably due to the resolution of the plot. It looks mostly like a sine wave, but there are "problems." It's not clear wheter you're varying pulse width, or pulse frequency, or what that gap is in the middle, for example. The code you posted doesn't have any meaningful comments.
I can "guess" that you're driving transistors for three phases of half-H-bridge circuits for a 3-phase motor, and that you want equal width pulses with the duty cycle varying like a sine function, but it's just a guess.
You haven't told us the pulse width, or how accurate you need things, or where 5760 came from.
Grr.

westfw:
You really haven't defined exactly what it is you're trying to do very well.
First of all, you seem to be manipulating at least 6 outputs.
Your original picture has weird banding, probably due to the resolution of the plot. It looks mostly like a sine wave, but there are "problems." It's not clear wheter you're varying pulse width, or pulse frequency, or what that gap is in the middle, for example. The code you posted doesn't have any meaningful comments.
I can "guess" that you're driving transistors for three phases of half-H-bridge circuits for a 3-phase motor, and that you want equal width pulses with the duty cycle varying like a sine function, but it's just a guess.
You haven't told us the pulse width, or how accurate you need things, or where 5760 came from.
Grr.

Hello sir, thank you for the code, I will try to test it whenever i can on the oscilloscope.

Basically i am trying to generate a modified sinusoidal signal using PWM. The signal is intended to average out as this, hence the gap. This is the intended average voltage:

from this PWM signal.

The other three signals are just the inverse

At first, I based my initial code using the formulas and methods of space vector PWM. but realized that previous program had calculation delays. After learning progmem, i generated the calculation results using matlab and just put the raw data in the variables. this data are switch states.

I am experimenting how much resolution i can get without compromising the speed.

The 5760 (360*16) was calculated that every 1 degree of the modified sine wave there are 16 different switch states.
I can adjust this number accordingly depending on how smooth the signal should be but as observed the frequency decrease with more raw data input

yes, i am using 6 outputs to drive a VSI 3 phase. I intend to drive the circuit below:

Ok. Thank you. That's MUCH better!

I'll need to think about it more...
One thing I've found useful in situations like this is to set a pin when you enter the ISR, and clear it when you exit the ISR. You can then monitor that pin with a scope (or even just an LED), to get an idea of how much time you are spending in the ISR vs the loop() code. If your ISR is taking longer than your timer tick, it will show up pretty clearly, because you'll only get a couple of instructions with the LED in the loop() state.

Probably having a full 360 values per cycle is overkill, and one thing you can do relatively easily is change the increment of j so that you're stepping 5 or 10 degrees at a time. I'm not quite sure how that will work with the current table of fully-decoded pin states - I guess somehow you need to get through a full position (16 steps.) Or just recalculate the table for the bigger step (and change the timer value?)

westfw:
Ok. Thank you. That's MUCH better!

I'll need to think about it more...
One thing I've found useful in situations like this is to set a pin when you enter the ISR, and clear it when you exit the ISR. You can then monitor that pin with a scope (or even just an LED), to get an idea of how much time you are spending in the ISR vs the loop() code. If your ISR is taking longer than your timer tick, it will show up pretty clearly, because you'll only get a couple of instructions with the LED in the loop() state.

Probably having a full 360 values per cycle is overkill, and one thing you can do relatively easily is change the increment of j so that you're stepping 5 or 10 degrees at a time. I'm not quite sure how that will work with the current table of fully-decoded pin states - I guess somehow you need to get through a full position (16 steps.) Or just recalculate the table for the bigger step (and change the timer value?)

Sir, thank you. I tested the code in the the oscilloscope and it took 8uS per loop. My guess that it took 128 clock cycles per loop. That is enough for me to run the code. Tnx to progmem and your bit testing approach, I was able to fit entire loop at the desired frequency at only 13kB of memory as shown by the compiler. :slight_smile:

Kudos to everyone who helped. :smiley: