Cannot change pointer for port in main loop

I am testing on an Arduino UNO. I want to use timer1 and interrupts to create various LED patterns. The LEDs may span multiple ports i.e PORTB and PORTD. I want to use a "volatile uint8_t*" pointer to store whether to toggle a pin on PORTB or PORTD i.e have the pointer store &PORTB or &POINTD. And then use it as necessary. However, once set, I cannot seem to be able to change the pointer in the loop.

int red_led = 9;    //PORTB 1
int green_led = 10; //PORTB 2

int led3 = 5;       //PORTD 5

volatile uint8_t current_led = 1; //port number of led
volatile uint8_t next_led = 1; 

volatile uint8_t *current_port = &PORTB; 
volatile uint8_t *next_port = &PORTB;    

int counter = 0; 
volatile int change_led_flag = 0;
volatile uint8_t led_state = 1;

uint16_t ocr1a_val = 10000-1; //determines period. 16MHz clock, Prescaler=8, 10000-1 ticks = 5ms period (200Hz)
uint16_t ocr1b_val = 9900-1; //determines duty cycle (99% for 10000 ticks)


void setup() {
  pinMode(red_led, OUTPUT);
  pinMode(green_led, OUTPUT);
  pinMode(led3, OUTPUT);

  TCCR1A = 0;
  TCCR1B = 0;

  //set CTC mode
  TCCR1B |= (1<<WGM12);

  //set OCR1A value
  OCR1A = ocr1a_val;
  //set OCR1B value
  OCR1B = ocr1b_val;

  //set pre-scaler to 8
  TCCR1B |= (1 << CS11);

  //enable output compare interrupts
  TIMSK1 |= (1 << OCIE1A);
  TIMSK1 |= (1 << OCIE1B);

  sei();
}

void loop() {
  if(change_led_flag == 1){
  change_led_flag = 0;
  current_led = next_led; //set the current led to what was just turned on
  current_port = next_port; //this statement doesn't do anything.
  switch (led_state) {
    case 1:
      led_state = 2;
      next_led = 2;
      next_port = &PORTB; //this statement doesn't do anything
      break;
    case 2:
      led_state = 3;
      next_led = 5;
      next_port = &PORTD; //this statement doesn't do anything
      break;
    case 3:
      led_state = 1;
      next_led = 1;
      next_port = &PORTB; //this statement doesn't do anything
      break;
    } //end switch statement
  } //end if
  
} //end loop()


//turn on next_led and set change_led_flag
ISR(TIMER1_COMPA_vect){
  *next_port |= (1 << next_led); //turn on led
  change_led_flag = 1; //set flag to update next led
}

//turn off current_led
ISR(TIMER1_COMPB_vect){
  *current_port &= ~(1 << current_led); //turn off led
}

Some more details:
I'm using timer1 in CTC mode (clear on compare match) and making use of both OCR1A and OCR1B interrupts.
When output compare A interrupt matches, the led is turned on and a flag is set for the main loop to change leds.
When output compare B interrupt matches, the led is turned off.
The switch statement works and the led pins can be changed i.e next_led and current_led, however, the next_port and current_port statements are ignored in the main loop.
The led pin change works when changing from case 3 to case 1 (when they are both PORTB), but when changing the next_port variable to PORTD this is completely ignored.

At the very beginning of the code, if I initialize current_port and next_port with &PORTD, then just PORTD led will light up and the main loop will not change the current_port and next_port variable to &PORTB.

What am I doing wrong?

Why?

So that I can select which led to turn on/off right when the interrupt fires. What would you suggest as an alternative?
Set two flags in each of the interrupts and handle everything else with if statements in the main loop?

Yup. That is very likely as-performant or better than using pointers and easier to get correct. A good example is the code you posted. It has a race condition / lacks critical sections. Your code is very likely to fail in very painful ways.

My gut tells me the threshold is around four ports. But even that is likely to have a marginal difference.

1 Like

You make some good points, thank you for your suggestions!

1 Like