Issue with 7 segment display code using RTC

[DRicc.2]

Currently, I am trying to display the time in hours and minutes via a 4 digit7-segment common cathode display however I am running into a problem. For example, when I try to display the time "4:09" the UART output is correct from the DS2321 RTC module I'm using but the 7-segment display only shows the last digit 9 across all 4 segments. My issue is with my function "void Display_7Segment where even when testing two segments at a time it still displays the same number for both segments despite writing to different pins (pin7 is digit 1, 8 is digit 2, etc.).

I'm using a Nucleo L476RG with an internal clock PLLCLK of 80mHZ, all segments and digits are set as GPIO pins, I2C is used for the RTC, and USART2 TX and RX is used for the PuTTy terminal output:

#include "main.h"
#include "uart.h"
#include <stdio.h>

#define DS3231_ADDRESS 0xD0 // Address of the DS3231, see page 16 of the datasheet.

I2C_HandleTypeDef hi2c1;

UART_HandleTypeDef huart2;


void SystemClock_Config(void);
static void MX_GPIO_Init(void);
static void MX_I2C1_Init(void);
static void MX_USART2_UART_Init(void);

char msg[100];

// 7-segment display encoding for digits 0-9 (7 bits for a-g segments)
uint8_t seg_digits[10] = {
    0x3F, // 0
    0x06, // 1
    0x5B, // 2
    0x4F, // 3
    0x66, // 4
    0x6D, // 5
    0x7D, // 6
    0x07, // 7
    0x7F, // 8
    0x6F  // 9
};

typedef struct { //This groups variables into one type of unit.
	uint8_t minutes; //Variable for minutes
	uint8_t hour; //Variable for hour
} TIME;

TIME time;


uint8_t decToBcd(int val)
{
    return (uint8_t)((val / 10 * 16) + (val % 10)); // Converts Decimal to Binary Coded Decimal
}

int bcdToDec(uint8_t val)
{
    return (int)((val / 16 * 10) + (val % 16)); // Converts Binary Coded Decimal to Decimal
}

void Set_Time(uint8_t min, uint8_t hour) // Set the time in hours and minutes
{
    uint8_t set_time[2];
    set_time[0] = decToBcd(min);
    set_time[1] = decToBcd(hour);
    HAL_I2C_Mem_Write(&hi2c1, DS3231_ADDRESS, 0x01, 1, set_time, 2, 1000);
}

void Get_Time(void)
{
    uint8_t get_time[2];
    HAL_I2C_Mem_Read(&hi2c1, DS3231_ADDRESS, 0x01, 1, get_time, 2, 1000);
    time.minutes = bcdToDec(get_time[0]);
    time.hour = bcdToDec(get_time[1]);
}

void Display_7Segment(uint8_t digit1, uint8_t digit2, uint8_t digit3, uint8_t digit4)
{
	// Define pins for controlling the segments (PC0 to PC6 for segments a-g)
	    // and PC7 to PC10 for controlling the 4 digits
	    // Assuming these are already initialized by STM32CubeMX for GPIO PC0-PC10.

	    // Digit 1: PC7, Digit 2: PC8, Digit 3: PC9, Digit 4: PC10

	    // Clear all digits
	    HAL_GPIO_WritePin(GPIOC, GPIO_PIN_7, GPIO_PIN_RESET);
	    HAL_GPIO_WritePin(GPIOC, GPIO_PIN_8, GPIO_PIN_RESET);
	    HAL_GPIO_WritePin(GPIOC, GPIO_PIN_9, GPIO_PIN_RESET);
	    HAL_GPIO_WritePin(GPIOC, GPIO_PIN_10, GPIO_PIN_RESET);

	    // Display digit 1
	    GPIOC->ODR = (GPIOC->ODR & 0xFF80) | seg_digits[digit1];
	    HAL_GPIO_WritePin(GPIOC, GPIO_PIN_7, GPIO_PIN_RESET); // Enable digit 1
	    HAL_Delay(1); // Short delay for multiplexing
	    HAL_GPIO_WritePin(GPIOC, GPIO_PIN_7, GPIO_PIN_SET); // Disable digit 1

	    // Display digit 2
	    GPIOC->ODR = (GPIOC->ODR & 0xFF80) | seg_digits[digit2];
	    HAL_GPIO_WritePin(GPIOC, GPIO_PIN_8, GPIO_PIN_RESET); // Enable digit 2
	    HAL_Delay(1);
	    HAL_GPIO_WritePin(GPIOC, GPIO_PIN_8, GPIO_PIN_SET);

	    // Display digit 3
	    GPIOC->ODR = (GPIOC->ODR & 0xFF80) | seg_digits[digit3];
	    HAL_GPIO_WritePin(GPIOC, GPIO_PIN_9, GPIO_PIN_RESET); // Enable digit 3
	    HAL_Delay(1);
	    HAL_GPIO_WritePin(GPIOC, GPIO_PIN_9, GPIO_PIN_SET);

	    // Display digit 4
	    GPIOC->ODR = (GPIOC->ODR & 0xFF80) | seg_digits[digit4];
	    HAL_GPIO_WritePin(GPIOC, GPIO_PIN_10, GPIO_PIN_RESET); // Enable digit 4
	    HAL_Delay(1);
	    HAL_GPIO_WritePin(GPIOC, GPIO_PIN_10, GPIO_PIN_SET);
}

int main(void)
{


  HAL_Init();


  SystemClock_Config();


  MX_GPIO_Init();
  MX_I2C1_Init();
  MX_USART2_UART_Init();

  //Set_Time(04, 4); //This sets the current time of the RTC module in hours and minutes. Upload the code
  //With this line first, then comment it out and upload the code again.
  while (1)
  {
	  Get_Time(); // Fetch the current time from DS3231

	  uint8_t hour_tens = time.hour / 10;  // Tens place of the hour
	  uint8_t hour_ones = time.hour % 10;  // Ones place of the hour
	  uint8_t min_tens = time.minutes / 10; // Tens place of the minutes
	  uint8_t min_ones = time.minutes % 10; // Ones place of the minutes

	  Display_7Segment(hour_tens, hour_ones, min_tens, min_ones); // Display the time on the 7-segment display
	  sprintf(msg, "%02d:%02d\r\n", time.hour, time.minutes); //Prints the time via UART/
	  uart_send_str(msg);
	  HAL_Delay(1000); // Update every second
  }

}

[Andrew Neil]

Show your schematic with the LED displays.

As it's just the LED part that's not working, simplify your code to concentrate on just that - get rid of the RTC stuff for now.

[TDK]

> // Clear all digits
> HAL_GPIO_WritePin(GPIOC, GPIO_PIN_7, GPIO_PIN_RESET);
> HAL_GPIO_WritePin(GPIOC, GPIO_PIN_8, GPIO_PIN_RESET);
> HAL_GPIO_WritePin(GPIOC, GPIO_PIN_9, GPIO_PIN_RESET);
> HAL_GPIO_WritePin(GPIOC, GPIO_PIN_10, GPIO_PIN_RESET);

> HAL_GPIO_WritePin(GPIOC, GPIO_PIN_7, GPIO_PIN_SET); // Disable digit 1

These are inconsistent.

If disabling the digit has the pin high, then then clearing all digits should set all pins high, not low.

Edit: For common cathode displays, setting the command cathode pin high turns it off, so first four statements should use GPIO_PIN_SET (or nothing at all, since they are already disabled from the last pass through).

[Andrew Neil]

Have you stepped the code in the debugger?

You should be able to watch what's happening ...

[DRicc.2]

Fixed the code comments for this and realized I had it wrong, I slowed down the delay per segment and saw how the first digit displays a 5, the second displays two 3s, the third displays three 3s, and the fourth digit displays 4 9s.