Cannot change pointer for port in main loop (arduino uno)

[Numex106]

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.

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.

I have tried using volatile uint16_t* for the next_port and current_port and that doesn't work, either.

What am I doing wrong?

Code: [Select]

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
}



[brucehoult]

The port must be volatile because it is hardware.

The pointer to the port must be volatile because it is set in your loop and read in the interrupt handler.

Code: [Select]

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


[abyrvalg]

Also there will be strange effects when the interrupt is fired right between next_pin and next_port assignments. Why not do that state advance straight in the ISR? It may look like you are minimizing ISR processing time freeing it up for next interrupts but in reality that problem with non-atomic assignments asks for interrupt disable/enable, which will reduce the time available for interrupts anyway.