Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hardware SPI implementation on AVR #1954

Closed
dolence opened this issue Sep 12, 2022 · 28 comments
Closed

Hardware SPI implementation on AVR #1954

dolence opened this issue Sep 12, 2022 · 28 comments

Comments

@dolence
Copy link

dolence commented Sep 12, 2022

I've been trying to implement hardware SPI on AVR baremetal. IC is an Atmega64 running at 12 MHz. I even tried to hardcode SPI speed but nothing is being showed. This is what I did following the Arduino implementation and avr-libc example:

#include <avr/io.h>
#include <avr/interrupt.h>
#include <util/delay.h>
#include <u8g2.h>

#define DISPLAY_SCK_DIR DDRB
#define DISPLAY_SCK_PORT PORTB
#define DISPLAY_SCK_PIN 1

#define DISPLAY_MOSI_DIR DDRB
#define DISPLAY_MOSI_PORT PORTB
#define DISPLAY_MOSI_PIN 2

#define DISPLAY_CS_DIR DDRB
#define DISPLAY_CS_PORT PORTB
#define DISPLAY_CS_PIN 0

u8g2_t u8g2;

uint8_t my_u8x8_byte_avr_hw_spi (u8x8_t * u8x8, uint8_t msg, uint8_t arg_int, void *arg_ptr) {
	uint8_t *data;

	switch (msg) {
		
		case U8X8_MSG_BYTE_INIT:
		if ( u8x8->bus_clock == 0 ) 	/* issue 769 */
		u8x8->bus_clock = u8x8->display_info->sck_clock_hz;		
		/* disable chipselect */
		u8x8_gpio_SetCS(u8x8, u8x8->display_info->chip_disable_level);		
		DISPLAY_MOSI_DIR |= (1 << DISPLAY_MOSI_PIN);		
		DISPLAY_SCK_DIR |= (1 << DISPLAY_SCK_PIN);				
		SPCR = ((1 << SPE) | (1 << MSTR));
		switch (u8x8->display_info->spi_mode) {
			case 0:
			break;
			case 1:
			SPCR |= (1 << CPHA);
			break;
			case 2:
			SPCR |= (1 << CPOL);
			break;
			case 3:			
			SPCR |= (1 << CPOL);
			SPCR |= (1 << CPHA);
			break;
		};
		/*
		switch (F_CPU / u8x8->display_info->sck_clock_hz) {
			case 2:			
			SPSR |= (1 << SPI2X);
			break;
			case 4:			
			break;
			case 8:
			SPSR |= (1 << SPI2X);
			SPCR |= (1 << SPR0);
			break;
			case 16:		
			SPCR |= (1 << SPR0);
			break;
			case 32:			
			SPSR |= (1 << SPI2X);
			SPCR |= (1 << SPR1);
			break;
			case 64:			
			SPCR |= (1 << SPR1);
			break;
			case 128:			
			SPCR |= (1 << SPR1);
			SPCR |= (1 << SPR0);
			break;
		}
		*/
		SPCR |= (1 << SPR0);
		
		u8x8_gpio_SetCS(u8x8, u8x8->display_info->chip_disable_level);
		break;
		
		case U8X8_MSG_BYTE_SET_DC:
		u8x8_gpio_SetDC(u8x8, arg_int);
		break;
		
		case U8X8_MSG_BYTE_START_TRANSFER:
		u8x8_gpio_SetCS(u8x8, u8x8->display_info->chip_enable_level);
		u8x8->gpio_and_delay_cb(u8x8, U8X8_MSG_DELAY_NANO, u8x8->display_info->post_chip_enable_wait_ns, NULL);
		break;
		
		case U8X8_MSG_BYTE_SEND:
		data = (uint8_t *) arg_ptr;
		while (arg_int > 0) {
			SPDR = (uint8_t) * data;
			while (!(SPSR & (1 << SPIF)));
			data++;
			arg_int--;
		}
		break;
		
		case U8X8_MSG_BYTE_END_TRANSFER:
		u8x8->gpio_and_delay_cb(u8x8, U8X8_MSG_DELAY_NANO, u8x8->display_info->pre_chip_disable_wait_ns, NULL);
		u8x8_gpio_SetCS(u8x8, u8x8->display_info->chip_disable_level);
		break;
		default:
		return 0;
	}
	
	return 1;
}

uint8_t my_u8x8_gpio_and_delay (u8x8_t * u8x8, uint8_t msg, uint8_t arg_int, void *arg_ptr) {
	// Re-use library for delays
	//if (u8x8_avr_delay(u8x8, msg, arg_int, arg_ptr))
	//return 1;

	switch (msg) {
		// called once during init phase of u8g2/u8x8
		// can be used to setup pins
		case U8X8_MSG_GPIO_AND_DELAY_INIT:
		DISPLAY_CS_DIR |= (1 << DISPLAY_CS_PIN);
		//DC_DDR |= _BV(DC_BIT);
		//RESET_DDR |= _BV(RESET_BIT);
		break;
		// CS (chip select) pin: Output level in arg_int
		case U8X8_MSG_GPIO_CS:
		if (arg_int)
		DISPLAY_CS_DIR |= (1 << DISPLAY_CS_PIN);
		else
		DISPLAY_CS_DIR &= ~(1 << DISPLAY_CS_PIN);
		break;
		// DC (data/cmd, A0, register select) pin: Output level in arg_int
		case U8X8_MSG_GPIO_DC:
		if (arg_int)
		//DC_PORT |= _BV(DC_BIT);
		//else
		//DC_PORT &= ~_BV(DC_BIT);
		break;
		// Reset pin: Output level in arg_int
		case U8X8_MSG_GPIO_RESET:
		if (arg_int)
		//RESET_PORT |= _BV(RESET_BIT);
		//else
		//RESET_PORT &= ~_BV(RESET_BIT);
		break;
		default:
		u8x8_SetGPIOResult(u8x8, 1);
		break;
	}
	return 1;
}



int main(void)
{
	sei();
	DDRC |= (1 << 4);
	PORTC |= (1 << 4);
	
	
	
	u8g2_Setup_st7920_128x64_f(&u8g2, U8G2_R0, my_u8x8_byte_avr_hw_spi, my_u8x8_gpio_and_delay);	
	u8g2_InitDisplay(&u8g2);
	u8g2_SetPowerSave(&u8g2, 0);
	
	/* full buffer example, setup procedure ends in _f */
	u8g2_ClearBuffer(&u8g2);
	u8g2_SetFont(&u8g2, u8g2_font_ncenB14_tr);
	u8g2_DrawStr(&u8g2, 1, 18, "U8g2 on AVR");
	u8g2_SendBuffer(&u8g2);
	
	
    /* Replace with your application code */
    while (1) 
    {

    }
}

Any thoughts on this?

@olikraus
Copy link
Owner

my_u8x8_gpio_and_delay: Handling of the delay messages is missing: /~https://github.com/olikraus/u8g2/wiki/Porting-to-new-MCU-platform#template-for-the-gpio-and-delay-callback

I think at least DELAY_MILLI should be there, probably also the 100NANO delay.

@dolence
Copy link
Author

dolence commented Sep 13, 2022

Thank you. I missed the delay functions. I don't know if I did it right, but after implementing it still not working. I've made a swap on that 12 MHz crystal oscillator for an 16 MHz just to be sure it wasn't the case.
I'm sure wiring is fine because I have tried an old firmware using u8g and m2tklib and it worked as expected.
I made some clean up, mainly on DC and RESET pins I'm not using. Still missing something...

EDIT 1: this is the line where the program is halting.
while (!(SPSR & (1 << SPIF)));

EDIT 2: registers seen to be properly set
image

EDIT 3: delay received message is 32, which weirdly doesn't map to any delay message definition
image

#include <avr/io.h>
#include <avr/interrupt.h>
#include <util/delay.h>
#include <u8g2.h>


#define DISPLAY_SCK_DIR DDRB
#define DISPLAY_SCK_PORT PORTB
#define DISPLAY_SCK_PIN 1

#define DISPLAY_MOSI_DIR DDRB
#define DISPLAY_MOSI_PORT PORTB
#define DISPLAY_MOSI_PIN 2

#define DISPLAY_CS_DIR DDRB
#define DISPLAY_CS_PORT PORTB
#define DISPLAY_CS_PIN 0



u8g2_t u8g2;



#define P_CPU_NS (1000000000UL / F_CPU)

uint8_t my_u8x8_avr_delay(u8x8_t *u8x8, uint8_t msg, uint8_t arg_int, void *arg_ptr)
{
	uint8_t cycles;

	switch(msg)
	{
		case U8X8_MSG_DELAY_NANO:     // delay arg_int * 1 nano second
		// At 20Mhz, each cycle is 50ns, the call itself is slower.
		break;
		case U8X8_MSG_DELAY_100NANO:    // delay arg_int * 100 nano seconds
		// Approximate best case values...
		#define CALL_CYCLES 26UL
		#define CALC_CYCLES 4UL
		#define RETURN_CYCLES 4UL
		#define CYCLES_PER_LOOP 4UL

		cycles = (100UL * arg_int) / (P_CPU_NS * CYCLES_PER_LOOP);

		if(cycles > CALL_CYCLES + RETURN_CYCLES + CALC_CYCLES)
		break;

		__asm__ __volatile__ (
		"1: sbiw %0,1" "\n\t" // 2 cycles
		"brne 1b" : "=w" (cycles) : "0" (cycles) // 2 cycles
		);
		break;
		case U8X8_MSG_DELAY_10MICRO:    // delay arg_int * 10 micro seconds
		for(int i=0 ; i < arg_int ; i++)
		_delay_us(10);
		break;
		case U8X8_MSG_DELAY_MILLI:      // delay arg_int * 1 milli second
		for(int i=0 ; i < arg_int ; i++)
		_delay_ms(1);
		break;
		default:
		return 0;
	}
	return 1;
}




uint8_t my_u8x8_byte_avr_hw_spi (u8x8_t * u8x8, uint8_t msg, uint8_t arg_int, void *arg_ptr) {
	uint8_t *data;

	switch (msg) {
		
		case U8X8_MSG_BYTE_INIT:
			if ( u8x8->bus_clock == 0 ) 	/* issue 769 */
			u8x8->bus_clock = u8x8->display_info->sck_clock_hz;		
			/* disable chipselect */
			u8x8_gpio_SetCS(u8x8, u8x8->display_info->chip_disable_level);		
			DISPLAY_MOSI_DIR |= (1 << DISPLAY_MOSI_PIN);		
			DISPLAY_SCK_DIR |= (1 << DISPLAY_SCK_PIN);				
			SPCR = ((1 << SPE) | (1 << MSTR));
			switch (u8x8->display_info->spi_mode) {
				case 0:
				break;
				case 1:
				SPCR |= (1 << CPHA);
				break;
				case 2:
				SPCR |= (1 << CPOL);
				break;
				case 3:			
				SPCR |= (1 << CPOL);
				SPCR |= (1 << CPHA);
				break;
			};
			
			switch (F_CPU / u8x8->display_info->sck_clock_hz) {
				case 2:			
				SPSR |= (1 << SPI2X);
				break;
				case 4:			
				break;
				case 8:
				SPSR |= (1 << SPI2X);
				SPCR |= (1 << SPR0);
				break;
				case 16:				
				SPCR |= (1 << SPR0);
				break;
				case 32:			
				SPSR |= (1 << SPI2X);
				SPCR |= (1 << SPR1);
				break;
				case 64:			
				SPCR |= (1 << SPR1);
				break;
				case 128:			
				SPCR |= (1 << SPR1);
				SPCR |= (1 << SPR0);
				break;
			}
			
	
			u8x8_gpio_SetCS(u8x8, u8x8->display_info->chip_disable_level);
			break;
		
		case U8X8_MSG_BYTE_START_TRANSFER:
			u8x8_gpio_SetCS(u8x8, u8x8->display_info->chip_enable_level);
			u8x8->gpio_and_delay_cb(u8x8, U8X8_MSG_DELAY_NANO, u8x8->display_info->post_chip_enable_wait_ns, NULL);
			break;
		
		case U8X8_MSG_BYTE_SEND:
			data = (uint8_t *) arg_ptr;
			while (arg_int > 0) {
				SPDR = (uint8_t) * data;
				while (!(SPSR & (1 << SPIF)));
				data++;
				arg_int--;
			}
			break;
		
		case U8X8_MSG_BYTE_END_TRANSFER:
			u8x8->gpio_and_delay_cb(u8x8, U8X8_MSG_DELAY_NANO, u8x8->display_info->pre_chip_disable_wait_ns, NULL);
			u8x8_gpio_SetCS(u8x8, u8x8->display_info->chip_disable_level);
			break;
			
		default:
			if (my_u8x8_avr_delay(u8x8, msg, arg_int, arg_ptr))	// check for any delay msgs
			return 1;
			u8x8_SetGPIOResult(u8x8, 1);      // default return value
			break;
	}
	
	return 1;
}

uint8_t my_u8x8_gpio_and_delay (u8x8_t * u8x8, uint8_t msg, uint8_t arg_int, void *arg_ptr) {
	// Re-use library for delays
	//if (u8x8_avr_delay(u8x8, msg, arg_int, arg_ptr))
	//return 1;

	switch (msg) {
		// called once during init phase of u8g2/u8x8
		// can be used to setup pins
		case U8X8_MSG_GPIO_AND_DELAY_INIT:
		DISPLAY_CS_DIR |= (1 << DISPLAY_CS_PIN);

		break;
		// CS (chip select) pin: Output level in arg_int
		case U8X8_MSG_GPIO_CS:
		if (arg_int)
		DISPLAY_CS_DIR |= (1 << DISPLAY_CS_PIN);
		else
		DISPLAY_CS_DIR &= ~(1 << DISPLAY_CS_PIN);
		break;
		default:
		u8x8_SetGPIOResult(u8x8, 1);
		break;
	}
	return 1;
}



int main(void)
{
	sei();
	DDRC |= (1 << 4);
	PORTC |= (1 << 4);
	
	
	
	u8g2_Setup_st7920_128x64_f(&u8g2, U8G2_R0, my_u8x8_byte_avr_hw_spi, my_u8x8_gpio_and_delay);	
	u8g2_InitDisplay(&u8g2);
	u8g2_SetPowerSave(&u8g2, 0);
	
	/* full buffer example, setup procedure ends in _f */
	u8g2_ClearBuffer(&u8g2);
	u8g2_SetFont(&u8g2, u8g2_font_ncenB14_tr);
	u8g2_DrawStr(&u8g2, 1, 18, "U8g2 on AVR");
	u8g2_SendBuffer(&u8g2);
	
	
    /* Replace with your application code */
    while (1) 
    {

    }
}

@dolence
Copy link
Author

dolence commented Sep 16, 2022

I've checked the initialization of SPI and all the registers seem to be right but unfortunately the display doesn't initialize. Could it be related to the fact my display doesn't use all the four wires as in #1949 ? It doesn't use the DC pin. I could give the Arduino version a try and see if this display work in 3-wire mode...

@olikraus
Copy link
Owner

while (!(SPSR & (1 << SPIF)));

Not sure whether I can help here. This is uC specific.

delay received message is 32

I don't exactly which messages are used in your case, but you should probably implement all delay messages listed in the code template to be on the safe side.

It doesn't use the DC pin. I could give the Arduino version a try and see if this display work in 3-wire mode...

I am a little bit confused. Let me start from the beginning:
There are different protocols with your display two of them are: 3-wire and 4-wire SPI.

It you choose one of them, you need to:

  1. Select the proper protocol on the PCB of the display itself. It will require to solder a specific protocol, which is the fixed with your display
  2. U8g2 then need to follow that protocol, which is from now on expected by your display. You can not just change the protocol on software side. Instead software has to fit to your display hardware configuration.
  3. 3-wire vs 4-wire differes in the usage of the DC line. and the size of one token (8 bits vs 9 bits)
  4. 3-wire is not supported in the c interface, it is only there with the Arduino C++ interface

You have actually asked for 3wire, so I pointed out the code in the C++ part of u8g2 (see #1949), but your above code doesn't seem to include the code. So your code above looks more like a 4-wire code.

As a starting point, my suggestion is this: Configure your display for 4-wire communication and then use the existing 4-wire communication in u8g2.

@dolence
Copy link
Author

dolence commented Sep 16, 2022

Oli, thank you for the clarification!

  1. Display is set to use SPI on hardware side, it was tested and working with the former version of the library.
  2. I undertood this, thanks.
    3-4) My display only have 3 pins for SPI usage. They are RS, R/W and E.
    image

@olikraus
Copy link
Owner

Oh, it looks like you want to use a ST7920 in serial mode. I think this wasn't mentioned before.
ST7920 is special: You need to use 4-wire protocol (this means: a very normal byte procedure), however the ST7920 indeed does not have a DC line. So your byte procedure should look like a 4-wire byte procedure.

@dolence
Copy link
Author

dolence commented Sep 21, 2022

Ok, so I got everything working following this tutorial. It was very easy. No delay functions to be implemented, just a few pin definitions and a couple of messages that later I made some cleanup removing uneeded code for my setup. Maybe it would be the case of updating documentation to reflect this.

A simple hello world code ended like this:

#include <avr/io.h>
#include <util/delay.h>
#include <avr/power.h>
#include "u8g2.h"
#include "u8x8_avr.h"

#define CS_DDR DDRB
#define CS_PORT PORTB
#define CS_BIT 0

uint8_t u8x8_gpio_and_delay(u8x8_t * u8x8, uint8_t msg, uint8_t arg_int, void *arg_ptr) {
	// Re-use library for delays
	if (u8x8_avr_delay(u8x8, msg, arg_int, arg_ptr))
	return 1;

	switch (msg) {
		// called once during init phase of u8g2/u8x8
		// can be used to setup pins
		case U8X8_MSG_GPIO_AND_DELAY_INIT:
			CS_DDR |= _BV(CS_BIT);		
			break;
		
		// CS (chip select) pin: Output level in arg_int
		case U8X8_MSG_GPIO_CS:
			if (arg_int)
				CS_PORT |= _BV(CS_BIT);
			else
				CS_PORT &= ~_BV(CS_BIT);
			break;

		default:
		u8x8_SetGPIOResult(u8x8, 1);
		break;
	}
	return 1;
}

u8g2_t u8g2;
int main (void) {
	u8g2_Setup_st7920_s_128x64_f(&u8g2, U8G2_R0, u8x8_byte_avr_hw_spi,	u8x8_gpio_and_delay);
	u8g2_InitDisplay(&u8g2);
	u8g2_SetPowerSave(&u8g2, 0);
	
	u8g2_ClearBuffer(&u8g2);
	u8g2_SetFont(&u8g2, u8g2_font_ncenB14_tr);
	u8g2_DrawStr(&u8g2, 0, 15, "HELLO!");
	u8g2_SendBuffer(&u8g2);

	while (1) {
		
	}
}

I also had to define some symbols, most important is AVR_USE_HW_SPI and some SPI pin definitions:
image

And that is it!

Now I'm facing some problems with the C version of MUI. I'm trying a basic example following the MUIMinimal.ino as a guide but MUIF list is throwing an "initializer element is not constant" error and Form Definition String fds_t do not appear to be a recognized symbol. I'm doing something wrong or it is just untested in C? Should I stick to the Arduino version to use MUI?
image

@olikraus
Copy link
Owner

olikraus commented Sep 21, 2022

... updating documentation to reflect this

/~https://github.com/olikraus/u8g2/wiki/Porting-to-new-MCU-platform#atmel-avr

"initializer element is not constant" error and Form Definition String fds_t do not appear to be a recognized symbol. I'm doing something wrong or it is just untested in C?

In fact the development of MUI happend in C only. Did you include both mui.h and mui_u8g2.h?
See also here: /~https://github.com/olikraus/u8g2/blob/master/sys/sdl/mui_blink/main.c

@dolence
Copy link
Author

dolence commented Sep 21, 2022

In fact the development of MUI happend in C only. Did you include both mui.h and mui_u8g2.h? See also here:

Yes, both are included. I'm trying a very minimal test. It's a form with just a label on it:

muif_t muif_list[] MUI_PROGMEM = {
MUIF_LABEL(mui_u8g2_draw_text)
};

fds_t fds[] MUI_PROGMEM =
MUI_FORM(1)
MUI_LABEL(5,12, "A label")
;

This is the line where the error message is pointing to:
image

@dolence
Copy link
Author

dolence commented Sep 23, 2022

Oli, I'm still stuck with this initializer element is not constant error. I double checked everything. I have both mui_u8g2.h and mui.h included on my main.c, right above u8g2.h and u8x8_avr.h. Is there anything wrong in here? It was tested on baremetal AVR or just the C on Arduino? I think will test it under Arduino interface and see what happens...

muif_t muif_list[] MUI_PROGMEM = {
	MUIF_LABEL(mui_u8g2_draw_text)
};

fds_t fds[] MUI_PROGMEM =
MUI_FORM(1)
MUI_LABEL(5,12, "A label")
;

@olikraus
Copy link
Owner

I have created a test and it works as expected:

u8g2_t u8g2;
mui_t ui;
muif_t muif_list[] MUI_PROGMEM = {
MUIF_LABEL(mui_u8g2_draw_text)
};
fds_t fds[] MUI_PROGMEM =
MUI_FORM(1)
MUI_LABEL(5,12, "A label")
;

I am not sure whether MUI_PROGMEM is required. I thought that I have moved this into the struct typedef. Maybe try without MUI_PROGMEM. For the SDL code above MUI_PROGMEM is anyways disabled, so the problem might be indeed MUI_PROGMEM.

@dolence
Copy link
Author

dolence commented Sep 26, 2022

I've tried with and without MUI_PROGMEM and even setup an entire new environment in a different machine. After installing Microchip Studio from scratch and creating an entire new project, including the library files etc, still the same error.

		"C:\Program Files (x86)\Atmel\Studio\7.0\toolchain\avr8\avr8-gnu-toolchain\bin\avr-gcc.exe"  -x c -funsigned-char -funsigned-bitfields -DF_CPU=16000000UL -DAVR_USE_HW_SPI -DSCK_DDR=DDRB -DSCK_BIT=1 -DMOSI_DDR=DDRB -DMOSI_BIT=2 -DDEBUG  -I"C:\Program Files (x86)\Atmel\Studio\7.0\Packs\atmel\ATmega_DFP\1.7.374\include" -I".." -I"../csrc" -I"../lib"  -Og -ffunction-sections -fdata-sections -fpack-struct -fshort-enums -g2 -Wall -mmcu=atmega64 -B "C:\Program Files (x86)\Atmel\Studio\7.0\Packs\atmel\ATmega_DFP\1.7.374\gcc\dev\atmega64" -c -std=gnu99 -MD -MP -MF "lib/u8x8_avr.d" -MT"lib/u8x8_avr.d" -MT"lib/u8x8_avr.o"   -o "lib/u8x8_avr.o" "../lib/u8x8_avr.c" 
		Finished building: ../lib/u8x8_avr.c
		Building file: .././main.c
		Invoking: AVR/GNU C Compiler : 5.4.0
		In file included from .././main.c:11:0:
D:\DESENVOLVIMENTO\AVR\LQ-01_u8g2_mui\LQ-01_u8g2_mui\csrc\mui.h(213,29): error: initializer element is not constant
		 #define MUIF_LABEL(cb) MUIF(".L",0, 0,cb)
		                             ^
D:\DESENVOLVIMENTO\AVR\LQ-01_u8g2_mui\LQ-01_u8g2_mui\csrc\mui.h(210,35): info: in definition of macro 'MUIF'
		 #define MUIF(id,cflags,data,cb) { id[0], id[1], cflags, 0, data, cb} 
		                                   ^
D:\DESENVOLVIMENTO\AVR\LQ-01_u8g2_mui\LQ-01_u8g2_mui\main.c(23,1): info: in expansion of macro 'MUIF_LABEL'
		 MUIF_LABEL(mui_u8g2_draw_text)
		 ^
D:\DESENVOLVIMENTO\AVR\LQ-01_u8g2_mui\LQ-01_u8g2_mui\csrc\mui.h(213,29): info: (near initialization for 'muif_list[0].id0')
		 #define MUIF_LABEL(cb) MUIF(".L",0, 0,cb)
		                             ^
D:\DESENVOLVIMENTO\AVR\LQ-01_u8g2_mui\LQ-01_u8g2_mui\csrc\mui.h(210,35): info: in definition of macro 'MUIF'
		 #define MUIF(id,cflags,data,cb) { id[0], id[1], cflags, 0, data, cb} 
		                                   ^
D:\DESENVOLVIMENTO\AVR\LQ-01_u8g2_mui\LQ-01_u8g2_mui\main.c(23,1): info: in expansion of macro 'MUIF_LABEL'
		 MUIF_LABEL(mui_u8g2_draw_text)
		 ^
D:\DESENVOLVIMENTO\AVR\LQ-01_u8g2_mui\LQ-01_u8g2_mui\csrc\mui.h(213,29): error: initializer element is not constant
		 #define MUIF_LABEL(cb) MUIF(".L",0, 0,cb)
		                             ^
		"C:\Program Files (x86)\Atmel\Studio\7.0\toolchain\avr8\avr8-gnu-toolchain\bin\avr-gcc.exe"  -x c -funsigned-char -funsigned-bitfields -DF_CPU=16000000UL -DAVR_USE_HW_SPI -DSCK_DDR=DDRB -DSCK_BIT=1 -DMOSI_DDR=DDRB -DMOSI_BIT=2 -DDEBUG  -I"C:\Program Files (x86)\Atmel\Studio\7.0\Packs\atmel\ATmega_DFP\1.7.374\include" -I".." -I"../csrc" -I"../lib"  -Og -ffunction-sections -fdata-sections -fpack-struct -fshort-enums -g2 -Wall -mmcu=atmega64 -B "C:\Program Files (x86)\Atmel\Studio\7.0\Packs\atmel\ATmega_DFP\1.7.374\gcc\dev\atmega64" -c -std=gnu99 -MD -MP -MF "main.d" -MT"main.d" -MT"main.o"   -o "main.o" ".././main.c" 
D:\DESENVOLVIMENTO\AVR\LQ-01_u8g2_mui\LQ-01_u8g2_mui\Debug\Makefile(1458,1): error: recipe for target 'main.o' failed
D:\DESENVOLVIMENTO\AVR\LQ-01_u8g2_mui\LQ-01_u8g2_mui\csrc\mui.h(210,42): info: in definition of macro 'MUIF'
		 #define MUIF(id,cflags,data,cb) { id[0], id[1], cflags, 0, data, cb} 
		                                          ^
D:\DESENVOLVIMENTO\AVR\LQ-01_u8g2_mui\LQ-01_u8g2_mui\main.c(23,1): info: in expansion of macro 'MUIF_LABEL'
		 MUIF_LABEL(mui_u8g2_draw_text)
		 ^
D:\DESENVOLVIMENTO\AVR\LQ-01_u8g2_mui\LQ-01_u8g2_mui\csrc\mui.h(213,29): info: (near initialization for 'muif_list[0].id1')
		 #define MUIF_LABEL(cb) MUIF(".L",0, 0,cb)
		                             ^
D:\DESENVOLVIMENTO\AVR\LQ-01_u8g2_mui\LQ-01_u8g2_mui\csrc\mui.h(210,42): info: in definition of macro 'MUIF'
		 #define MUIF(id,cflags,data,cb) { id[0], id[1], cflags, 0, data, cb} 
		                                          ^
D:\DESENVOLVIMENTO\AVR\LQ-01_u8g2_mui\LQ-01_u8g2_mui\main.c(23,1): info: in expansion of macro 'MUIF_LABEL'
		 MUIF_LABEL(mui_u8g2_draw_text)
		 ^
		make: *** [main.o] Error 1
		make: *** Waiting for unfinished jobs....
		Building file: ../csrc/u8x8_fonts.c
		Invoking: AVR/GNU C Compiler : 5.4.0
		"C:\Program Files (x86)\Atmel\Studio\7.0\toolchain\avr8\avr8-gnu-toolchain\bin\avr-gcc.exe"  -x c -funsigned-char -funsigned-bitfields -DF_CPU=16000000UL -DAVR_USE_HW_SPI -DSCK_DDR=DDRB -DSCK_BIT=1 -DMOSI_DDR=DDRB -DMOSI_BIT=2 -DDEBUG  -I"C:\Program Files (x86)\Atmel\Studio\7.0\Packs\atmel\ATmega_DFP\1.7.374\include" -I".." -I"../csrc" -I"../lib"  -Og -ffunction-sections -fdata-sections -fpack-struct -fshort-enums -g2 -Wall -mmcu=atmega64 -B "C:\Program Files (x86)\Atmel\Studio\7.0\Packs\atmel\ATmega_DFP\1.7.374\gcc\dev\atmega64" -c -std=gnu99 -MD -MP -MF "csrc/u8x8_fonts.d" -MT"csrc/u8x8_fonts.d" -MT"csrc/u8x8_fonts.o"   -o "csrc/u8x8_fonts.o" "../csrc/u8x8_fonts.c" 
		Finished building: ../csrc/u8x8_fonts.c
		Building file: ../csrc/u8g2_fonts.c
		Invoking: AVR/GNU C Compiler : 5.4.0
		"C:\Program Files (x86)\Atmel\Studio\7.0\toolchain\avr8\avr8-gnu-toolchain\bin\avr-gcc.exe"  -x c -funsigned-char -funsigned-bitfields -DF_CPU=16000000UL -DAVR_USE_HW_SPI -DSCK_DDR=DDRB -DSCK_BIT=1 -DMOSI_DDR=DDRB -DMOSI_BIT=2 -DDEBUG  -I"C:\Program Files (x86)\Atmel\Studio\7.0\Packs\atmel\ATmega_DFP\1.7.374\include" -I".." -I"../csrc" -I"../lib"  -Og -ffunction-sections -fdata-sections -fpack-struct -fshort-enums -g2 -Wall -mmcu=atmega64 -B "C:\Program Files (x86)\Atmel\Studio\7.0\Packs\atmel\ATmega_DFP\1.7.374\gcc\dev\atmega64" -c -std=gnu99 -MD -MP -MF "csrc/u8g2_fonts.d" -MT"csrc/u8g2_fonts.d" -MT"csrc/u8g2_fonts.o"   -o "csrc/u8g2_fonts.o" "../csrc/u8g2_fonts.c" 
		Finished building: ../csrc/u8g2_fonts.c
	Done executing task "RunCompilerTask" -- FAILED.
Done building target "CoreBuild" in project "LQ-01_u8g2_mui.cproj" -- FAILED.
Done building project "LQ-01_u8g2_mui.cproj" -- FAILED.

Build FAILED.
========== Rebuild All: 0 succeeded, 1 failed, 0 skipped ==========

@olikraus
Copy link
Owner

Hmm... difficult to tell. Not sure how to continue from here. MUI works in unix/SDL and Arduino environment.

@dolence
Copy link
Author

dolence commented Oct 3, 2022

Could it be something specific to AVR-GCC? I could upload a simple project in case someone else want to test it. Anyway, I've just made to switch to the Arduino version until I got it working on Studio.

@olikraus
Copy link
Owner

olikraus commented Oct 3, 2022

Could it be something specific to AVR-GCC?

Hmm... MUI also works without problems on Arduino UNO

@dolence
Copy link
Author

dolence commented Oct 3, 2022

Different compiler versions, maybe? Or am I barking up the wrong tree?

@olikraus
Copy link
Owner

olikraus commented Oct 3, 2022

I think Arduino 1.8.4 or 1.8.10 did work for me.

@kasitoru
Copy link
Contributor

I have the same problem. After spending some time, I found the reason. When using PROGMEM, all values should be constants, but in some defines this is not the case. Example:

#define MUIF(id,cflags,data,cb) { id[0], id[1], cflags, 0, data, cb}

The problem is in id[0] and id[1]. There are several other similar definitions.

@olikraus
Copy link
Owner

Usually all provided data are constant. Moreover: This error didn't happen to me.

@kasitoru
Copy link
Contributor

However, as a temporary solution, @dolence can make the following changes to the mui.h file (lines 158-172):

//#if defined(__GNUC__) && defined(__AVR__)
//#  define muif_get_id0(muif) mui_pgm_read(&((muif)->id0))
//#  define muif_get_id1(muif) mui_pgm_read(&((muif)->id1))
//#  define muif_get_cflags(muif) mui_pgm_read(&((muif)->cflags))
//#  define muif_get_extra(muif) mui_pgm_read(&((muif)->extra))
//#  define muif_get_data(muif) ((void *)mui_pgm_wread(&((muif)->data)))
//#  define muif_get_cb(muif) ((muif_cb)mui_pgm_wread(&((muif)->cb)))
//#else
#  define muif_get_id0(muif) ((muif)->id0)
#  define muif_get_id1(muif) ((muif)->id1)
#  define muif_get_cflags(muif) ((muif)->cflags)
#  define muif_get_extra(muif) ((muif)->extra)
#  define muif_get_data(muif) ((muif)->data)
#  define muif_get_cb(muif) ((muif)->cb)
//#endif

And accordingly remove MUI_PROGMEM for muif_list. It worked for me.

@olikraus
Copy link
Owner

I am even more confused. I neither understand the original problem nor do I understand how the changes from @kasitoru are related to the original problem.

@kasitoru
Copy link
Contributor

kasitoru commented Oct 12, 2022

As I understand it, the problem is that some versions of the compiler for avr (in this case, we are talking about the official toolchain from Microchip) consider it unacceptable to use calculated values (such as id[0] from ".L") for PROGMEM variables.
You can add an additional define (for example MUIF_DISABLE_PGM) for these problematic compilers:

/* assumes that pointers are 16 bit so encapusalte the wread i another ifdef __AVR__ */
#if defined(__GNUC__) && defined(__AVR__) && !defined(MUIF_DISABLE_PGM)
#  define muif_get_id0(muif) mui_pgm_read(&((muif)->id0))
#  define muif_get_id1(muif) mui_pgm_read(&((muif)->id1))
#  define muif_get_cflags(muif) mui_pgm_read(&((muif)->cflags))
#  define muif_get_extra(muif) mui_pgm_read(&((muif)->extra))
#  define muif_get_data(muif) ((void *)mui_pgm_wread(&((muif)->data)))
#  define muif_get_cb(muif) ((muif_cb)mui_pgm_wread(&((muif)->cb)))
#else
#  define muif_get_id0(muif) ((muif)->id0)
#  define muif_get_id1(muif) ((muif)->id1)
#  define muif_get_cflags(muif) ((muif)->cflags)
#  define muif_get_extra(muif) ((muif)->extra)
#  define muif_get_data(muif) ((muif)->data)
#  define muif_get_cb(muif) ((muif)->cb)
#endif

When compiling, pass this argument to GCC:

avr-gcc.exe $(CCFLAGS) -DMUIF_DISABLE_PGM -c u8g2/csrc/*.c

@olikraus
Copy link
Owner

But the above get macros are not used in the errors reported by @dolence

@kasitoru
Copy link
Contributor

kasitoru commented Oct 12, 2022

Part of his error messages:

D:\DESENVOLVIMENTO\AVR\LQ-01_u8g2_mui\LQ-01_u8g2_mui\csrc\mui.h(213,29): error: initializer element is not constant
		 #define MUIF_LABEL(cb) MUIF(".L",0, 0,cb)

This is because he set MUI_PROGMEM for muif_list. If remove this, there will be no errors, but the display will be empty (because the reading will still come from the progmem area, but is no data there). That's why need to disable this macros.

@olikraus
Copy link
Owner

Ok, understand. But this still doesn't explain the root problem. 🤔

@kasitoru
Copy link
Contributor

I don't understand it either. I think the problem is in the avr-gcc compiler. Because the code looks good.

@kasitoru
Copy link
Contributor

It was my mistake. It was necessary to move the functions that are used in muif_list to the global scope.

@qianfan-Zhao
Copy link

#include <stdio.h>

int a = "1234567"[0];

int main(void)
{
	printf("%d\n", a);
	return 0;
}

this code can not compiled before gcc-8, it will report errors in my gcc-7:

$  gcc 1.c       
1.c:3:9: error: initializer element is not constant
 int a = "1234567"[0];
         ^~~~~~~~~
$ gcc -v 
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/7/lto-wrapper
OFFLOAD_TARGET_NAMES=nvptx-none
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu 7.5.0-6ubuntu2' --with-bugurl=file:///usr/share/doc/gcc-7/README.Bugs --enable-languages=c,ada,c++,go,brig,d,fortran,objc,ob
j-c++ --prefix=/usr --with-gcc-major-version-only --program-suffix=-7 --program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-
gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --enable-bootstrap --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new
 --enable-gnu-unique-object --disable-vtable-verify --enable-libmpx --enable-plugin --enable-default-pie --with-system-zlib --with-target-system-zlib --enable-objc-gc=auto --enable-multia
rch --disable-werror --with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-offload-targets=nvptx-none --without-cuda-driver -
-enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
gcc version 7.5.0 (Ubuntu 7.5.0-6ubuntu2)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants