Function pointers cause extremly wierd behavior of bare metal ATMega328PB

Question

I'm making a digital watch project on bare metal ATMega328PB. One of the tasks of uC is to drive 7-segmented display using pins from banks B and D as digital outputs. I'm uploading code using Arduino IDE with MiniCore and Arduino UNO as ISP programmer. I keep all the fuses at factory default (internal oscilator etc.) and I don't use any bootloader. Uploading code works fine, but in my code there are functions for driving the correct pins high to display correct digit. I need to assign execution for each function to corresponding integer, so I used function pointer array.

The moment I uploaded the code where function "display_write" was used, my uC stopped driving the display pins correctly and acted as if it was constantly resetting even where there is 10k pullup resistor on reset pin. I'm preety sure it's a software problem. I keep my project soldered on homemade PCB and I've checked multiple times that the power is beegin delivered (via osciloscope) and there are now unexpected events on any pins. ATMega in this state creates short impluses on some segments' anodes (connected to PD pins) but all cathodes (this is 5 digit common cathode display) are not pulled low (via NPN transistors).

In this state there is no communication with uC. However, when I disconnect some anode driving pins from the display, uC starts working normally until I connect them again and there are always the same 2 pins which after disconnecting return it to normal state temporarly. I've measured everything hardwarewise (segments draw less than 1mA form pins) and there is absoltutely nothing special about this configuration or whatever. This ONLY happens when I execute function that uses function pointers in my program, when i don't use function pointers everything works fine.

I honestly have no idea what is cousing this behavior. I modified the code to not use timer interrupt but it also didn;t change anything until i use "display_write" function.

This is the code that is cousing this after upload.

/*
Wiring:
PD0 - a
PD1 - b
PD2 - c
PD3 - d
PD4 - e
PD5 - f
PD6 - g
PD7 - dp

PB0 - CA1 - position 0
PB1 - CA2 - position 1
PB2 - CA3 - position 2
PB6 - CA4 - position 3
PB7 - CA5 - position 4
*/

#include <Wire.h> //Library needed for I2C communication with RTC module
#include <Arduino.h>

#define digits_count    5

//DS1337 RTC register addresses
#define rtc_address     0x68
#define second          0x00
#define minute          0x01
#define hour            0x02
#define day             0x03
#define date            0x04
#define month           0x05
#define year            0x06
#define alarm1_minute   0x08
#define alarm1_hour     0x09
#define alarm1_state    0x0A
#define alarm2_minute   0x0B
#define alarm2_hour     0x0C
#define alarm2_state    0x0D

//Curent position for display
volatile int current_position = 0;

//Current digits array with sign number and dot point status
volatile int current_display[digits_count][2];

//Position lookup table for PORT manipulation
volatile int position_LUT[digits_count] = {0b00000001, 0b00000010, 0b00000100, 0b01000000, 0b10000000};

//Pointers assigning integers to corresponding functions that display digits/signs they represent
volatile void (*fun_ptr[11])(int, bool);

//Interrupt service routine for Timer 1 to regularly update display
ISR(TIMER1_COMPA_vect) {
  display_write(current_position);
}

void setup() {
  //Set Timer 1 to interrupt and refresh screen periodacly
  TCCR1A  = 0b00000000; 
  TCCR1B  = 0b00001001; //No prescaler, CTC for OCR1A
  TCNT1   = 0;          //Initial value
  TIMSK1 |= 0b00000010; //Set interrupt on compare A
  OCR1A   = 5000;       //Display new digit every 5ms
  //Set D and B pins as outputs for driving the display
  DDRD =  0b11111111;
  DDRB =  0b11111111;
  //Default output state
  PORTD = 0b00000000;
  PORTB = 0b00000000;
  //Assigning function pointers to functions
  fun_ptr[0] = d0;
  fun_ptr[1] = d1;
  fun_ptr[2] = d2;
  fun_ptr[3] = d3;
  fun_ptr[4] = d4;
  fun_ptr[5] = d5;
  fun_ptr[6] = d6;
  fun_ptr[7] = d7;
  fun_ptr[8] = d8;
  fun_ptr[9] = d9;
  fun_ptr[10] = null;

  //Initial diplay values
  current_display[0][0] = 4;
  current_display[1][0] = 7;
  current_display[2][0] = 2;
  current_display[3][0] = 9;
  current_display[4][0] = 8;
  current_display[0][1] = 1;
  current_display[1][1] = 1;
  current_display[2][1] = 0;
  current_display[3][1] = 0;
  current_display[4][1] = 1;

  //Initialize I2C communication with RTC
  Wire.begin();
}


void loop() {
  current_display[0][1] = 0;
  current_display[1][1] = 1;
  current_display[2][1] = 0;
  current_display[3][1] = 1;
  current_display[4][1] = 0;
  rtc_read(hour, 0, 1, 1);
  rtc_read(minute, 2, 3, 1);
  rtc_read(second, 4, 4, 0);;
  delay(1);
}

//Read data from RTC and insert digits to positions in display array
void rtc_read(int address, int position1, int position2, bool position2_valid) {
  int data_bcd;
  int data_dec;
  Wire.beginTransmission(rtc_address);
  Wire.write(address);
  Wire.endTransmission();
  Wire.requestFrom(rtc_address,1);
  if (Wire.available()) {
    data_bcd = Wire.read();
  }
  data_dec = (data_bcd / 16 * 10) + (data_bcd % 16);  //Convert RTS's BCD format to decimal
  current_display[position1][0] = data_dec / 10;      //Write most siginificant digit to position 1
  if (position2_valid != 0) {
  current_display[position2][0] = data_dec % 10;      //Write least siginificant digit to position 2
  }
}

//Display array function
void display_write(int position) {
  (*fun_ptr[current_display[position][0]])(position, current_display[position][1]);
  current_position++;
  if (current_position == digits_count) {
    current_position = 0;
  }
}

//Digit/sign display functions

void d0(int position, bool dp_status) {
  PORTD = 0b00111111; // Set segment configuration
  PORTB = position_LUT[position]; //Disable all digits and add bit corresponding to position argument from LUT
  PORTD |= (dp_status << PD7); //Add dot point bit
}

void d1(int position, bool dp_status) {
  PORTD = 0b00000110;
  PORTB = position_LUT[position];
  PORTD |= (dp_status << PD7);
}

void d2(int position, bool dp_status) {
  PORTD = 0b01011011;
  PORTB = position_LUT[position];
  PORTD |= (dp_status << PD7);
}

void d3(int position, bool dp_status) {
  PORTD = 0b01001111;
  PORTB = position_LUT[position];
  PORTD |= (dp_status << PD7);
}

void d4(int position, bool dp_status) {
  PORTD = 0b01100110;
  PORTB = position_LUT[position];
  PORTD |= (dp_status << PD7);
}

void d5(int position, bool dp_status) {
  PORTD = 0b01101101;
  PORTB = position_LUT[position];
  PORTD |= (dp_status << PD7);
}

void d6(int position, bool dp_status) {
  PORTD = 0b01111101;
  PORTB = position_LUT[position];
  PORTD |= (dp_status << PD7);
}

void d7(int position, bool dp_status) {
  PORTD = 0b00000111;
  PORTB = position_LUT[position];
  PORTD |= (dp_status << PD7);
}

void d8(int position, bool dp_status) {
  PORTD = 0b01111111;
  PORTB = position_LUT[position];
  PORTD |= (dp_status << PD7);
}

void d9(int position, bool dp_status) {
  PORTD = 0b01101111;
  PORTB = position_LUT[position];
  PORTD |= (dp_status << PD7);
}

void null(int position, bool dp_status) {
  PORTD = 0b00000000;
  PORTB = position_LUT[position];
  PORTD |= (dp_status << PD7);
}

I tried all possible combinations of the code and the wierd behavior (constant reset and no communication) alwyas comes back when the "display_write" function is executed. I have checked all connections and hardware problems and measured all signals from uC and power (USB 5V).

I expected the code to regularly check the integer for each position in the "current_display" array and execute a function d0/d1...null corresponding to this integer via function pointer array.

Answer 1

You have to be very careful when working with pointers. When calling functions via pointers, you must be 100% sure that the correct pointers will be called. In your case, this is not guaranteed.

The following code allows a value greater than 10 to be entered into the array.

  data_dec = (data_bcd / 16 * 10) + (data_bcd % 16);  //Convert RTS's BCD format to decimal
  current_display[position1][0] = data_dec / 10;      //Write most siginificant digit to position 1

It is sufficient that, for example, the variable data_bcd contains the value 0xFF. The data_dec variable then has the value 165. And this results in the value 16 being stored in the array. Which results in calling a pointer from an array index that is not defined. Which automatically causes the code to crash.

To convert a number to a value of a seven-segment representation, you do not need such a neck-breaking program construction in any case.

The rest of the program contains many, many incorrect programming constructs, and it is possible that there is something else that can cause the crash also.