Searching \ for '[PIC] Simple GCC question' in subject line. ()
Make payments with PayPal - it's fast, free and secure! Help us get a faster server
FAQ page: www.piclist.com/techref/microchip/devices.htm?key=pic
Search entire site for: 'Simple GCC question'.

Exact match. Not showing close matches.
PICList Thread
'[PIC] Simple GCC question'
2020\07\30@122530 by Bob Blick

face
flavicon
face
I'm using a nice little HD44780 LCD library in an STM32 project, it works great but I'm stuck with a variable scope problem that I can only seem to make worse because GCC is a bit stricter than I want it to be at this point. Sorry if the code example doesn't wordwrap properly.

If, somewhere in main() I create an instance of the LCD like this:
 Lcd_PortType ports[] = {LCD_D4_GPIO_Port, LCD_D5_GPIO_Port, LCD_D6_GPIO_Port, LCD_D7_GPIO_Port};
 Lcd_PinType pins[] = {LCD_D4_Pin, LCD_D5_Pin, LCD_D6_Pin, LCD_D7_Pin};
 Lcd_HandleTypeDef lcd = Lcd_create(ports, pins, LCD_RS_GPIO_Port, LCD_RS_Pin, LCD_EN_GPIO_Port, LCD_EN_Pin, LCD_4_BIT_MODE);

then I can use it like this:
 Lcd_cursor(&lcd, 1,6);

but of course I can't do that from within another function since "lcd" only exists in main()

If I try to make it all global by creating lcd outside of main(), GCC doesn't like that because initializer elements must be constants and it doesn't consider "ports" and "pins" to be constants.

Not being a real C programmer this has stopped me in my tracks. If I was stuck on a deserted island I would use this library as a basis for something with ports and pins hardcoded, but I'm hoping a real expert would give me some gentle help, which will certainly educate me about doing things the correct way and also get me past this little hurdle.

Thanks, Bob
-- http://www.piclist.com/techref/piclist PIC/SX FAQ & list archive
View/change your membership options at
mailman.mit.edu/mailman/listinfo/piclist
.

2020\07\30@123738 by Sean Breheny

face picon face
Hi Bob,

Do you have a link to the library you are using? I don't understand what
the type of LCD_D4_GPIO_Port, etc. is.

Sean


On Thu, Jul 30, 2020 at 12:27 PM Bob Blick <spam_OUTbobblickTakeThisOuTspamoutlook.com> wrote:

{Quote hidden}

-- http://www.piclist.com/techref/piclist PIC/SX FAQ & list archive
View/change your membership options at
mailman.mit.edu/mailman/listinfo/piclist
.

2020\07\30@124906 by Bob Blick

face
flavicon
face
Hi Sean,

Here it is:

https://github.com/4ilo/HD44780-Stm32HAL

Thanks, Bob

________________________________________
From: .....piclist-bouncesKILLspamspam@spam@mit.edu <piclist-bouncesspamKILLspammit.edu> on behalf of Sean Breheny Sent: Thursday, July 30, 2020 9:38 AM
To: Microcontroller discussion list - Public.
Subject: Re: [PIC] Simple GCC question

Hi Bob,

Do you have a link to the library you are using? I don't understand what
the type of LCD_D4_GPIO_Port, etc. is.

Sean


On Thu, Jul 30, 2020 at 12:27 PM Bob Blick  wrote:

{Quote hidden}

-- http://www.piclist.com/techref/piclist PIC/SX FAQ & list archive
View/change your membership options at
mailman.mit.edu/mailman/listinfo/piclist
.

2020\07\30@125428 by Manu Abraham

picon face
On Thu, Jul 30, 2020 at 9:57 PM Bob Blick <.....bobblickKILLspamspam.....outlook.com> wrote:
>
> I'm using a nice little HD44780 LCD library in an STM32 project, it works great but I'm stuck with a variable scope problem that I can only seem to make worse because GCC is a bit stricter than I want it to be at this point.. Sorry if the code example doesn't wordwrap properly.
>
> If, somewhere in main() I create an instance of the LCD like this:
>   Lcd_PortType ports[] = {LCD_D4_GPIO_Port, LCD_D5_GPIO_Port, LCD_D6_GPIO_Port, LCD_D7_GPIO_Port};
>   Lcd_PinType pins[] = {LCD_D4_Pin, LCD_D5_Pin, LCD_D6_Pin, LCD_D7_Pin};
>   Lcd_HandleTypeDef lcd = Lcd_create(ports, pins, LCD_RS_GPIO_Port, LCD_RS_Pin, LCD_EN_GPIO_Port, LCD_EN_Pin, LCD_4_BIT_MODE);
>

Why I dislike opaque datatypes...

Instead of copying objects to Lcd_create(), a pointer could have been used,
storage could have been global, made static ..
the variables appear to be for configuration purposes ?

One way to do is make the lcd object global.

Another way is to pass a lcd object pointer to whatever function you
are calling,
unless you need to access the lcd object from interrupt context.
If you need access from interrupt context, the easier way is to have
global storage.

-- http://www.piclist.com/techref/piclist PIC/SX FAQ & list archive
View/change your membership options at
mailman.mit.edu/mailman/listinfo/piclist
.

2020\07\30@130956 by Neil

flavicon
face
Bob,

Strange way to do this IMO... I would keep ports & pins together in a struct for each GPIO, then group those together for your call to lcd_create().
Either way, I'm not sure why gcc thinks your port and pin definitions aren't constants.  Aren't you using CMSIS for those definitions?
In HAL (which ultimately uses CMSIS), ports and pins are defined in the mail header file like this...
#define LED_Pin GPIO_PIN_13
#define LED_GPIO_Port GPIOC

Then these are further defined in a deeper file as:
#define GPIO_PIN_13                ((uint16_t)0x2000)  /* Pin 13 selected   */
and...
#define GPIOC               ((GPIO_TypeDef *)GPIOC_BASE)
and then...
#define GPIOC_BASE            (APB2PERIPH_BASE + 0x00001000UL)
and also...
/*!< Peripheral memory map */
#define APB1PERIPH_BASE       PERIPH_BASE
#define APB2PERIPH_BASE       (PERIPH_BASE + 0x00010000UL)
#define AHBPERIPH_BASE        (PERIPH_BASE + 0x00020000UL)

....which are constants.

How/where do you have your ports and pins defined?

Cheers,
-Neil.




On 7/30/2020 12:50 PM, Bob Blick wrote:
{Quote hidden}

-- http://www.piclist.com/techref/piclist PIC/SX FAQ & list archive
View/change your membership options at
mailman.mit.edu/mailman/listinfo/piclist
.

2020\07\30@132317 by Manu Abraham

picon face
Hi Bob,


Hi Bob,

On Thu, Jul 30, 2020 at 10:21 PM Bob Blick <@spam@bobblickKILLspamspamoutlook.com> wrote:
>
> Hi Sean,
>
> Here it is:
>
> https://github.com/4ilo/HD44780-Stm32HAL


Lcd_HandleTypeDef Lcd_create(
Lcd_PortType port[], Lcd_PinType pin[],
Lcd_PortType rs_port, Lcd_PinType rs_pin,
Lcd_PortType en_port, Lcd_PinType en_pin, Lcd_ModeTypeDef mode)
{
Lcd_HandleTypeDef lcd;

lcd.mode = mode;

lcd.en_pin = en_pin;
lcd.en_port = en_port;

lcd.rs_pin = rs_pin;
lcd.rs_port = rs_port;

lcd.data_pin = pin;
lcd.data_port = port;

Lcd_init(&lcd);

return lcd;
}


The easiest way ?
just ..

#define EN_PIN               PIN_NO
#define LCD_MODE        MODExx

and eventually do replace:

lcd.mode = mode
with lcd.mode = MODExx

for the rest of the lcd objects in Lcd_create()
I would say a bit too much of over engineering in there for a simple LCD config.

Regards,
Manu
-- http://www.piclist.com/techref/piclist PIC/SX FAQ & list archive
View/change your membership options at
mailman.mit.edu/mailman/listinfo/piclist
.

2020\07\30@132409 by Bob Blick

face
flavicon
face
Hi Neil,

The ports and pins are in main.h

Individually they are constants, but GCC doesn't like using arrays of them handed to Lcd_create, it no longer considers them constants. I think.

Thanks, Bob

________________________________________
From: KILLspampiclist-bouncesKILLspamspammit.edu <RemoveMEpiclist-bouncesTakeThisOuTspammit.edu> on behalf of Neil Sent: Thursday, July 30, 2020 10:10 AM
To: Microcontroller discussion list - Public.
Subject: Re: [PIC] Simple GCC question

Bob,

Strange way to do this IMO... I would keep ports & pins together in a
struct for each GPIO, then group those together for your call to
lcd_create().
Either way, I'm not sure why gcc thinks your port and pin definitions
aren't constants.  Aren't you using CMSIS for those definitions?
In HAL (which ultimately uses CMSIS), ports and pins are defined in the
mail header file like this...
#define LED_Pin GPIO_PIN_13
#define LED_GPIO_Port GPIOC

Then these are further defined in a deeper file as:
#define GPIO_PIN_13                ((uint16_t)0x2000)  /* Pin 13
selected   */
and...
#define GPIOC               ((GPIO_TypeDef *)GPIOC_BASE)
and then...
#define GPIOC_BASE            (APB2PERIPH_BASE + 0x00001000UL)
and also...
/*!< Peripheral memory map */
#define APB1PERIPH_BASE       PERIPH_BASE
#define APB2PERIPH_BASE       (PERIPH_BASE + 0x00010000UL)
#define AHBPERIPH_BASE        (PERIPH_BASE + 0x00020000UL)

....which are constants.

How/where do you have your ports and pins defined?

Cheers,
-Neil.



-- http://www.piclist.com/techref/piclist PIC/SX FAQ & list archive
View/change your membership options at
mailman.mit.edu/mailman/listinfo/piclist
.

2020\07\30@133715 by Bob Blick

face
flavicon
face
Hi Manu,

Although the LCD will not normally be driven from interrupts, if there is a lockup I would like it to leave an error code on the display, which would need to be done from an interrupt.

I guess to make it global without errors I will need to hardcode the ports and pins so I get rid of the arrays that are handed to Lcd_create()

Thanks, Bob  
________________________________________
From: spamBeGonepiclist-bouncesspamBeGonespammit.edu <TakeThisOuTpiclist-bouncesEraseMEspamspam_OUTmit.edu> on behalf of Manu Abraham Sent: Thursday, July 30, 2020 9:55 AM
To: Microcontroller discussion list - Public.
Subject: Re: [PIC] Simple GCC question

On Thu, Jul 30, 2020 at 9:57 PM Bob Blick wrote:
>
> I'm using a nice little HD44780 LCD library in an STM32 project, it works great but I'm stuck with a variable scope problem that I can only seem to make worse because GCC is a bit stricter than I want it to be at this point.. Sorry if the code example doesn't wordwrap properly.
>
> If, somewhere in main() I create an instance of the LCD like this:
>   Lcd_PortType ports[] = {LCD_D4_GPIO_Port, LCD_D5_GPIO_Port, LCD_D6_GPIO_Port, LCD_D7_GPIO_Port};
>   Lcd_PinType pins[] = {LCD_D4_Pin, LCD_D5_Pin, LCD_D6_Pin, LCD_D7_Pin};
>   Lcd_HandleTypeDef lcd = Lcd_create(ports, pins, LCD_RS_GPIO_Port, LCD_RS_Pin, LCD_EN_GPIO_Port, LCD_EN_Pin, LCD_4_BIT_MODE);
>

Why I dislike opaque datatypes...

Instead of copying objects to Lcd_create(), a pointer could have been used,
storage could have been global, made static ..
the variables appear to be for configuration purposes ?

One way to do is make the lcd object global.

Another way is to pass a lcd object pointer to whatever function you
are calling,
unless you need to access the lcd object from interrupt context.
If you need access from interrupt context, the easier way is to have
global storage.

--

-- http://www.piclist.com/techref/piclist PIC/SX FAQ & list archive
View/change your membership options at
mailman.mit.edu/mailman/listinfo/piclist
.

2020\07\30@134818 by Manu Abraham

picon face
Hi Bob,

Another look at the github code, I would do this:

#define MODE ..

#define EN_PIN    LL_GPIO_PIN_0
#define EN_PORT GPIOB
...
and more

Lcd_HandleTypeDef lcd;  <-- make this global,

int main(void)
{

... do whatever ..

lcd.mode = MODE;

lcd.en_pin = EN_PIN;
lcd.en_port = EN_PORT;

lcd.rs_pin = rs_pin;
lcd.rs_port = rs_port;

lcd.data_pin = pin;
lcd.data_port = port;

Lcd_init(&lcd);

... do whatever ..

}

Lcd_init() happens to be public via lcd.h.
So, you do not need to clobber the library,
but just that you do not need to call Lcd_create() fn.

Advantage, lower memory usage, lesser complexity. :-)

Regards,
Manu

On Thu, Jul 30, 2020 at 11:08 PM Bob Blick <RemoveMEbobblickspamTakeThisOuToutlook.com> wrote:
{Quote hidden}

-- http://www.piclist.com/techref/piclist PIC/SX FAQ & list archive
View/change your membership options at
mailman.mit.edu/mailman/listinfo/piclist
.

2020\07\30@140241 by Bob Blick

face
flavicon
face
Hi Manu,
I will give it a try. I guess I wasn't just too stupid not to be able to make it work without changes. Thank you very much.
Bob

________________________________________
From: RemoveMEpiclist-bouncesEraseMEspamEraseMEmit.edu <RemoveMEpiclist-bouncesspam_OUTspamKILLspammit.edu> on behalf of Manu Abraham Sent: Thursday, July 30, 2020 10:49 AM
To: Microcontroller discussion list - Public.
Subject: Re: [PIC] Simple GCC question

Hi Bob,

Another look at the github code, I would do this:

#define MODE ..

#define EN_PIN    LL_GPIO_PIN_0
#define EN_PORT GPIOB
...
and more

Lcd_HandleTypeDef lcd;  <-- make this global,

int main(void)
{

... do whatever ..

lcd.mode = MODE;

lcd.en_pin = EN_PIN;
lcd.en_port = EN_PORT;

lcd.rs_pin = rs_pin;
lcd.rs_port = rs_port;

lcd.data_pin = pin;
lcd.data_port = port;

Lcd_init(&lcd);

... do whatever ..

}

Lcd_init() happens to be public via lcd.h.
So, you do not need to clobber the library,
but just that you do not need to call Lcd_create() fn.

Advantage, lower memory usage, lesser complexity. :-)

Regards,
Manu

On Thu, Jul 30, 2020 at 11:08 PM Bob Blick  wrote:
{Quote hidden}

-- http://www.piclist.com/techref/piclist PIC/SX FAQ & list archive
View/change your membership options at
mailman.mit.edu/mailman/listinfo/piclist
.

2020\07\30@142804 by Manu Abraham

picon face
Hi Bob,

Sure, you are welcome, not a hassle.
Most code is similar a stone rolling through time,
Over time the sharp edges get nicely shaped.
Lot of public code is like that, it takes time and effort to evolve.

Best Regards,
Manu

On Thu, Jul 30, 2020 at 11:44 PM Bob Blick <RemoveMEbobblickKILLspamspamoutlook.com> wrote:
{Quote hidden}

-- http://www.piclist.com/techref/piclist PIC/SX FAQ & list archive
View/change your membership options at
mailman.mit.edu/mailman/listinfo/piclist
.

2020\07\30@173511 by Mike

picon face

I strongly suspect the non-constant value GCC is complaining about is
the return value from Lcd_create().  Since the lcd structure is being
initialased via a function call you cannot do this outside of a
function.  You can however define an instance of the lcd struct outside
of a function, and then initialise it later.

Something like this should work:

main.c

#include <lcd.h>

/* These can be initialised outside a function since the initialisers are constant */
Lcd_PortType ports[] = {LCD_D4_GPIO_Port, LCD_D5_GPIO_Port, LCD_D6_GPIO_Port, LCD_D7_GPIO_Port};
Lcd_PinType pins[] = {LCD_D4_Pin, LCD_D5_Pin, LCD_D6_Pin, LCD_D7_Pin};
/* You can only define an instance of Lcd_HandleTypeDef here, since the initialiser is a function */
Lcd_HandleTypeDef lcd;

void main(void)
{
  /* Initialise the lcd here */
  lcd = Lcd_create(ports, pins, LCD_RS_GPIO_Port, LCD_RS_Pin, LCD_EN_GPIO_Port, LCD_EN_Pin, LCD_4_BIT_MODE);
 
  /* Other stuff */
  for(;;);
}


mymodule.c

#include <lcd.h>

/* Declare the lcd struct defined in main  */
extern Lcd_HandleTypeDef lcd;

void myfunction(void)
{
  Lcd_clear(&lcd);
}


This is the simplest way I could show this, but using global variables
isn't typically the nicest way to do things.

Regards

Mike


On 30/07/2020 17:26, Bob Blick wrote:
{Quote hidden}

--
http://www.piclist.com/techref/piclist PIC/SX FAQ & list archive
View/change your membership options at
mailman.mit.edu/mailman/listinfo/piclist

2020\07\30@185118 by Bob Blick

face
flavicon
face
Here's a copy of an email Wouter sent directly. I will need to read up on a singleton. So much to learn!

> I'm using a nice little HD44780 LCD library in an STM32 project, it works great but I'm stuck with a variable scope problem that I can only seem to make worse because GCC is a bit stricter than I want it to be at this point.. Sorry if the code example doesn't wordwrap properly.

You could switch to C++, which allows globals to be initialized at
run-time ..... (but that comes with its own can of worms, so I never use
that feature).

I think your problem is solved by having a global pointer to your LCD
object, which you assign its value after you created the LCD thingy in
main. Just take care that nobody uses it before that time.

If you want to look clever, you could make a singleton: a function that
returns the lcd pointer. The first time it is executed, it creates the
LCD thingy. Maybe the ports and pins must be static too, depending on
whether the lcd thingy uses them after its creation.

  Lcd_HandleTypeDef * Lcd(){
     static Lcd_HandleTypeDef * p = NULL;
     static lcd_thingy;
     if( p == NULL ){
        Lcd_PortType ports[] = {LCD_D4_GPIO_Port, LCD_D5_GPIO_Port, LCD_D6_GPIO_Port, LCD_D7_GPIO_Port};
        Lcd_PinType pins[] = {LCD_D4_Pin, LCD_D5_Pin, LCD_D6_Pin, LCD_D7_Pin};
        lcd_thingy = Lcd_create(ports, pins, LCD_RS_GPIO_Port, LCD_RS_Pin, LCD_EN_GPIO_Port, LCD_EN_Pin, LCD_4_BIT_MODE);
        p = & lcd_thingy;
      }
      return p;
   }

-- Wouter "Objects? No Thanks!" van Ooijen
-- http://www.piclist.com/techref/piclist PIC/SX FAQ & list archive
View/change your membership options at
mailman.mit.edu/mailman/listinfo/piclist
.

2020\07\30@204126 by Bob Blick

face
flavicon
face
Hi Mike,

That was simple, and not only did it compile, but my LCD display still works.

Thank you so much and thanks for all the other answers!

Bob

________________________________________
From: @spam@piclist-bounces@spam@spamspam_OUTmit.edu <spamBeGonepiclist-bouncesspamKILLspammit.edu> on behalf of Mike Sent: Thursday, July 30, 2020 2:36 PM
To: .....piclistspam_OUTspammit.edu
Subject: Re: [PIC] Simple GCC question

I strongly suspect the non-constant value GCC is complaining about is
the return value from Lcd_create().  Since the lcd structure is being
initialased via a function call you cannot do this outside of a
function.  You can however define an instance of the lcd struct outside
of a function, and then initialise it later.

Something like this should work:

main.c

#include <lcd.h>

/* These can be initialised outside a function since the initialisers are constant */
Lcd_PortType ports[] = {LCD_D4_GPIO_Port, LCD_D5_GPIO_Port, LCD_D6_GPIO_Port, LCD_D7_GPIO_Port};
Lcd_PinType pins[] = {LCD_D4_Pin, LCD_D5_Pin, LCD_D6_Pin, LCD_D7_Pin};
/* You can only define an instance of Lcd_HandleTypeDef here, since the initialiser is a function */
Lcd_HandleTypeDef lcd;

void main(void)
{
  /* Initialise the lcd here */
  lcd = Lcd_create(ports, pins, LCD_RS_GPIO_Port, LCD_RS_Pin, LCD_EN_GPIO_Port, LCD_EN_Pin, LCD_4_BIT_MODE);

  /* Other stuff */
  for(;;);
}


mymodule.c

#include <lcd.h>

/* Declare the lcd struct defined in main  */
extern Lcd_HandleTypeDef lcd;

void myfunction(void)
{
  Lcd_clear(&lcd);
}


This is the simplest way I could show this, but using global variables
isn't typically the nicest way to do things.

Regards

Mike


On 30/07/2020 17:26, Bob Blick wrote:
{Quote hidden}

--

-- http://www.piclist.com/techref/piclist PIC/SX FAQ & list archive
View/change your membership options at
mailman.mit.edu/mailman/listinfo/piclist
.


'[PIC] Simple GCC question'
2020\08\01@212520 by Bob Blick
face
flavicon
face
Hi Sergio,

Thank you very much for this. I appreciate the detail, since I am not a natural programmer, it helps me a lot to see the right way to move when I improve this library. Which I will when I do the next project that needs it. Right now I must press on to finish this one.
Many thanks.

Bob

________________________________________
From: TakeThisOuTpiclist-bounces.....spamTakeThisOuTmit.edu <TakeThisOuTpiclist-bouncesKILLspamspamspammit.edu> on behalf of sergio Sent: Friday, July 31, 2020 2:00 AM
To: Microcontroller discussion list - Public.
Subject: Re: [PIC] Simple GCC question

Sorry I'm late to the party but for future reference this might help:

Try to convert the offending function to a macro and use that to
initialise your structure.

Looking at the "Lcd_create" function I can see that all it is doing is
initialising a copy of the structure, calling "Lcd_init" with a
pointer to that copy and then returning the copy to be copied into the
actual "lcd" structure (all (mostly) incredibly redundent and inefficent).

What I would do in this instance is remove the call to "Lcd_init" from
"Lcd_create" and then convert "Lcd_create" to a macro thus:

#define LCD_CREATE(                     \
               port, pin,              \
               rs_port, rs_pin,        \
               en_port, en_pin, mode)  \
       {                               \
               .mode = mode,           \
                                       \
               .en_pin = en_pin,       \
               .en_port = en_port,     \
                                       \
               .rs_pin = rs_pin,       \
               .rs_port = rs_port,     \
                                       \
               .data_pin = pin,        \
               .data_port = port       \
       }

BEWARE: the "\" at the end of the line should be the last character
before the newline. An unseen space such as "\ " will cause problems

Your code should then change to this:

Lcd_PortType
       ports[] =
       {       LCD_D4_GPIO_Port,
               LCD_D5_GPIO_Port,
               LCD_D6_GPIO_Port,
               LCD_D7_GPIO_Port
       };

Lcd_PinType
       pins[] =
       {       LCD_D4_Pin,
               LCD_D5_Pin,
               LCD_D6_Pin,
               LCD_D7_Pin
       };

/* can't remember if you need the "\" below */
Lcd_HandleTypeDef
       lcd = LCD_CREATE(ports,                 \
                        pins,                  \
                        LCD_RS_GPIO_Port,      \
                        LCD_RS_Pin,            \
                        LCD_EN_GPIO_Port,      \
                        LCD_EN_Pin,            \
                        LCD_4_BIT_MODE);

void main(void)
{
       /* Initialise the lcd here */
       Lcd_init(&lcd);

       /* Other stuff */
       for(;;);
}

I haven't actually tried compiling this (too much effort) but it should
work :-)

Regards
Sergio Masci


On Fri, 31 Jul 2020, Bob Blick wrote:

{Quote hidden}

-- http://www.piclist.com/techref/piclist PIC/SX FAQ & list archive
View/change your membership options at
mailman.mit.edu/mailman/listinfo/piclist
.

More... (looser matching)
- Last day of these posts
- In 2020 , 2021 only
- Today
- New search...