[PIC] Simple GCC question

classic Classic list List threaded Threaded
16 messages Options
Reply | Threaded
Open this post in threaded view
|

[PIC] Simple GCC question

Bob Blick-5
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
http://mailman.mit.edu/mailman/listinfo/piclist
Reply | Threaded
Open this post in threaded view
|

Re: [PIC] Simple GCC question

Sean Breheny
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 <[hidden email]> 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);
>
> 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
> http://mailman.mit.edu/mailman/listinfo/piclist
>
--
http://www.piclist.com/techref/piclist PIC/SX FAQ & list archive
View/change your membership options at
http://mailman.mit.edu/mailman/listinfo/piclist
Reply | Threaded
Open this post in threaded view
|

Re: [PIC] Simple GCC question

Bob Blick-5
Hi Sean,

Here it is:

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

Thanks, Bob

________________________________________
From: [hidden email] <[hidden email]> 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:

> 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
http://mailman.mit.edu/mailman/listinfo/piclist
Reply | Threaded
Open this post in threaded view
|

Re: [PIC] Simple GCC question

Manu Abraham-2
In reply to this post by Bob Blick-5
On Thu, Jul 30, 2020 at 9:57 PM Bob Blick <[hidden email]> 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
http://mailman.mit.edu/mailman/listinfo/piclist
Reply | Threaded
Open this post in threaded view
|

Re: [PIC] Simple GCC question

Neil
In reply to this post by Bob Blick-5
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:

> Hi Sean,
>
> Here it is:
>
> https://github.com/4ilo/HD44780-Stm32HAL
>
> Thanks, Bob
>
> ________________________________________
> From: [hidden email] <[hidden email]> 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:
>
>> 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
http://mailman.mit.edu/mailman/listinfo/piclist
Reply | Threaded
Open this post in threaded view
|

Re: [PIC] Simple GCC question

Manu Abraham-2
In reply to this post by Bob Blick-5
Hi Bob,


Hi Bob,

On Thu, Jul 30, 2020 at 10:21 PM Bob Blick <[hidden email]> 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
http://mailman.mit.edu/mailman/listinfo/piclist
Reply | Threaded
Open this post in threaded view
|

Re: [PIC] Simple GCC question

Bob Blick-5
In reply to this post by Neil
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: [hidden email] <[hidden email]> 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
http://mailman.mit.edu/mailman/listinfo/piclist
Reply | Threaded
Open this post in threaded view
|

Re: [PIC] Simple GCC question

Bob Blick-5
In reply to this post by Manu Abraham-2
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: [hidden email] <[hidden email]> 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
http://mailman.mit.edu/mailman/listinfo/piclist
Reply | Threaded
Open this post in threaded view
|

Re: [PIC] Simple GCC question

Manu Abraham-2
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 <[hidden email]> wrote:

>
> 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: [hidden email] <[hidden email]> 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
> http://mailman.mit.edu/mailman/listinfo/piclist

--
http://www.piclist.com/techref/piclist PIC/SX FAQ & list archive
View/change your membership options at
http://mailman.mit.edu/mailman/listinfo/piclist
Reply | Threaded
Open this post in threaded view
|

Re: [PIC] Simple GCC question

Bob Blick-5
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: [hidden email] <[hidden email]> 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:

>
> 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: [hidden email] <[hidden email]> 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
http://mailman.mit.edu/mailman/listinfo/piclist
Reply | Threaded
Open this post in threaded view
|

Re: [PIC] Simple GCC question

Manu Abraham-2
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 <[hidden email]> wrote:

>
> 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: [hidden email] <[hidden email]> 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:
> >
> > 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: [hidden email] <[hidden email]> 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
> http://mailman.mit.edu/mailman/listinfo/piclist

--
http://www.piclist.com/techref/piclist PIC/SX FAQ & list archive
View/change your membership options at
http://mailman.mit.edu/mailman/listinfo/piclist
Reply | Threaded
Open this post in threaded view
|

Re: [PIC] Simple GCC question

Mike Rigby-Jones
In reply to this post by Bob Blick-5
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:

> 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
http://mailman.mit.edu/mailman/listinfo/piclist
Reply | Threaded
Open this post in threaded view
|

Re: [PIC] Simple GCC question

Bob Blick-5
In reply to this post by Bob Blick-5
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
http://mailman.mit.edu/mailman/listinfo/piclist
Reply | Threaded
Open this post in threaded view
|

Re: [PIC] Simple GCC question

Bob Blick-5
In reply to this post by Mike Rigby-Jones
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: [hidden email] <[hidden email]> on behalf of Mike
Sent: Thursday, July 30, 2020 2:36 PM
To: [hidden email]
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:

> 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
http://mailman.mit.edu/mailman/listinfo/piclist
Reply | Threaded
Open this post in threaded view
|

Re: [PIC] Simple GCC question

sergio
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:

> 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: [hidden email] <[hidden email]> on behalf of Mike
> Sent: Thursday, July 30, 2020 2:36 PM
> To: [hidden email]
> 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:
>> 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
> http://mailman.mit.edu/mailman/listinfo/piclist
>
--
http://www.piclist.com/techref/piclist PIC/SX FAQ & list archive
View/change your membership options at
http://mailman.mit.edu/mailman/listinfo/piclist
Reply | Threaded
Open this post in threaded view
|

Re: [PIC] Simple GCC question

Bob Blick-5
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: [hidden email] <[hidden email]> 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:

> 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: [hidden email] <[hidden email]> on behalf of Mike
> Sent: Thursday, July 30, 2020 2:36 PM
> To: [hidden email]
> 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:
>> 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
http://mailman.mit.edu/mailman/listinfo/piclist