Forum Menu




 


Log in Problems?
New User? Sign Up!
AVR Freaks Forum Index

Post new topic   Reply to topic
View previous topic Printable version Log in to check your private messages View next topic
Author Message
stewpye
PostPosted: Apr 13, 2012 - 06:32 AM
Rookie


Joined: Jul 04, 2009
Posts: 36
Location: Brisbane, Australia

I have a problem in that when I declare a variable as volatile I can't use it as the index for an array anymore. I need to declare it as volatile so I can access it from an ISR.

I have stripped down the code to this:

Code:
unsigned char pat_count;

unsigned char pat[8] PROGMEM =
   {
   0b00000001, 0b00000010, 0b00000100, 0b00001000, 0b00010000, 0b00100000, 0b01000000, 0b10000000,
   };

   

int main (void)
{
board_init();

LEDPORT.DIR = 0xff;    // Set all pins of port D to output.
pat_count = 1;
LEDPORT.OUT = ~pat[pat_count];
     
   
   while (1)
   {
             
   }
   
}


As soon as I change "unsigned char pat_count" to "volatile unsigned char pat_count" the code doesn't work. Is there something obvious I'm missing here. I have searched here and google but only found why you should use volatile...

AVR Studio 5.1.208

Regards,
Stewart.
 
 View user's profile Send private message  
Reply with quote Back to top
ChaunceyGardiner
PostPosted: Apr 13, 2012 - 06:41 AM
Posting Freak


Joined: Mar 09, 2012
Posts: 1452
Location: North Carolina, USA

Does the problem disappear if you remove PROGMEM from the array declaration ?
 
 View user's profile Send private message  
Reply with quote Back to top
stewpye
PostPosted: Apr 13, 2012 - 06:52 AM
Rookie


Joined: Jul 04, 2009
Posts: 36
Location: Brisbane, Australia

ChaunceyGardiner wrote:
Does the problem disappear if you remove PROGMEM from the array declaration ?

Yes it does. Why is it so?

I'm pretty new to C programming as you can probably tell...

Regards,
Stewart.
 
 View user's profile Send private message  
Reply with quote Back to top
JohanEkdahl
PostPosted: Apr 13, 2012 - 08:37 AM
10k+ Postman


Joined: Mar 27, 2002
Posts: 18515
Location: Lund, Sweden

The counter-question is: Why do you want to make it volatile? It's an array pointing to eight strings in PROGMEM. As such it will be known at compile time. Your example shows no chage of it at runtime, so the problem that volatile does not come into play - it evolves entirely around data that changes during runtime and your (PROGMEM) data is constant.

OTOH, if you are really using avr-gcc then you can not simply
Code:
LEDPORT.OUT = ~pat[pat_count];

You will have to actively, by a function call, read out the element of the PROGMEM based array element. As it stands now you are using a pointer into flash memory as a pointer into RAM. AVR-GCC does not support readout of flash based data as elegantly as some other compilers.

You need to go over to the Tutorials forum and read the PROGMEM tutorial.
 
 View user's profile Send private message Visit poster's website 
Reply with quote Back to top
ChaunceyGardiner
PostPosted: Apr 13, 2012 - 08:41 AM
Posting Freak


Joined: Mar 09, 2012
Posts: 1452
Location: North Carolina, USA

It's more of a hardware issue than a C issue.

You have to use pgm_read_byte(pat+pat_count) or something to that effect to get the right value.

You previously got away with it because the compiler optimized your code, as your array contents were known and the indexing expression was a constant.

Check out this tutorial. (It contains a link to a newer version, too.)
 
 View user's profile Send private message  
Reply with quote Back to top
JohanEkdahl
PostPosted: Apr 13, 2012 - 08:56 AM
10k+ Postman


Joined: Mar 27, 2002
Posts: 18515
Location: Lund, Sweden

Quote:

It's more of a hardware issue than a C issue.

As I hinted at above, I'd say that it is mostly a compiler implementation issue. Other tool chains are implemented so that they recognize that the variable is in flash, and generate the corect LPM instructions.
 
 View user's profile Send private message Visit poster's website 
Reply with quote Back to top
ChaunceyGardiner
PostPosted: Apr 13, 2012 - 09:21 AM
Posting Freak


Joined: Mar 09, 2012
Posts: 1452
Location: North Carolina, USA

JohanEkdahl wrote:
Quote:

It's more of a hardware issue than a C issue.

As I hinted at above, I'd say that it is mostly a compiler implementation issue. Other tool chains are implemented so that they recognize that the variable is in flash, and generate the corect LPM instructions.


My point was that you can be completely fluent in C and never come across things like PROGMEM. That seemed relevant to what the OP said about his C skills in this context.

PROGMEM is not a part of the C standard, so to claim that it's a compiler implementation issue is stretching it a bit.
 
 View user's profile Send private message  
Reply with quote Back to top
clawson
PostPosted: Apr 13, 2012 - 09:36 AM
10k+ Postman


Joined: Jul 18, 2005
Posts: 62209
Location: (using avr-gcc in) Finchingfield, Essex, England

Quote:

PROGMEM is not a part of the C standard, so to claim that it's a compiler implementation issue is stretching it a bit.

Of course it's an implementation issue. It is GCC's (current!) mechanism for handling Harvard architecture. IAR, Codevision and the latest Imagecraft (I believe) use _flash for the same thing - and their implementations have intrinsic rather than extrinisc read support. avr-gcc gets this when 4.7 becomes mainstream.

To the OP, in case it's not clear, I'd suggest your code needs to be something like:
Code:
#undef F_CPU
// set number in following line to CPU speed in Hz
#define F_CPU 1000000UL
#include <avr/io.h>
#include <util/delay.h>
#include <avr/pgmspace.h>

unsigned char pat_count;

unsigned char pat[8] PROGMEM =
{
   0b00000001, 0b00000010, 0b00000100, 0b00001000, 0b00010000, 0b00100000, 0b01000000, 0b10000000,
};

int main (void)
{
  board_init();

  LEDPORT.DIR = 0xff;    // Set all pins of port D to output.
  pat_count = 0;
  while (1)
  {
    LEDPORT.OUT = ~pgm_read_byte(&pat[pat_count++]);
    _delay_ms(100);
    if (pat_count >= 8) {
      pat_count = 0;
    }
  }
}

I'm not sure why you'd want to make pat_count 'volatile' in this but maybe it's to be sure of watching it in the simulator/debugger?

_________________


Last edited by clawson on Apr 13, 2012 - 10:47 AM; edited 2 times in total
 
 View user's profile Send private message  
Reply with quote Back to top
ChaunceyGardiner
PostPosted: Apr 13, 2012 - 09:46 AM
Posting Freak


Joined: Mar 09, 2012
Posts: 1452
Location: North Carolina, USA

clawson wrote:
Quote:

PROGMEM is not a part of the C standard, so to claim that it's a compiler implementation issue is stretching it a bit.

Of course it's an implementation issue. It is GCC's (current!) mechanism for handling Harvard architecture. IAR, Codevision and the latest Imagecraft (I believe) use _flash for the same thing - and their implementations have intrinsic rather than extrinisc read support. avr-gcc gets this when 4.7 becomes mainstream.
It may be perceived as an issue about the implementation of an extension to the standard in order to accomodate specific hardware features. As is usual, this non-standard feature gets an underscore in front of the symbol to distinguish it from things that are standard.

It is not a C issue, and the OP seemed to think it was.
 
 View user's profile Send private message  
Reply with quote Back to top
ChaunceyGardiner
PostPosted: Apr 13, 2012 - 10:00 AM
Posting Freak


Joined: Mar 09, 2012
Posts: 1452
Location: North Carolina, USA

clawson wrote:
To the OP, in case it's not clear, I'd suggest your code needs to be something like:
Code:
unsigned char pat_count;

unsigned char pat[8] PROGMEM =
{
   0b00000001, 0b00000010, 0b00000100, 0b00001000, 0b00010000, 0b00100000, 0b01000000, 0b10000000,
};

int main (void)
{
  board_init();

  LEDPORT.DIR = 0xff;    // Set all pins of port D to output.
  pat_count = 0;
  while (1)
  {
    LEDPORT.OUT = ~pgm_read_byte(&pat[pat_count++]);
    _delay_ms(100);
    if (pat_count >= 8) {
      pat_count = 0;
    }
  }
}

I'm not sure why you'd want to make pat_count 'volatile' in this but maybe it's to be sure of watching it in the simulator/debugger?
He said in the first post that he wants to "access" pat_count in an ISR. That seems like a good reason to make it volatile to me.

How did you deduce that he wants to set LEDPORT.OUT more than once ? As far as I can tell from what he's said here, he has enough trouble when trying to set it once. I'm pretty sure he will survive that based on suggestions made earlier in this thread.
 
 View user's profile Send private message  
Reply with quote Back to top
clawson
PostPosted: Apr 13, 2012 - 10:15 AM
10k+ Postman


Joined: Jul 18, 2005
Posts: 62209
Location: (using avr-gcc in) Finchingfield, Essex, England

Quote:

How did you deduce that he wants to set LEDPORT.OUT more than once ?

Because if he does it once without a delay he's going to need VERY keen eyesight to see anything happen - doh!

_________________
 
 View user's profile Send private message  
Reply with quote Back to top
ChaunceyGardiner
PostPosted: Apr 13, 2012 - 10:32 AM
Posting Freak


Joined: Mar 09, 2012
Posts: 1452
Location: North Carolina, USA

clawson wrote:
Quote:

How did you deduce that he wants to set LEDPORT.OUT more than once ?

Because if he does it once without a delay he's going to need VERY keen eyesight to see anything happen - doh!


He had a delay. A very long one. You removed it.
 
 View user's profile Send private message  
Reply with quote Back to top
SprinterSB
PostPosted: Apr 13, 2012 - 10:41 AM
Posting Freak


Joined: Dec 21, 2006
Posts: 1483
Location: Saar-Lor-Lux

JohanEkdahl wrote:
Quote:
It's more of a hardware issue than a C issue.
As I hinted at above, I'd say that it is mostly a compiler implementation issue. Other tool chains are implemented so that they recognize that the variable is in flash, and generate the corect LPM instructions.
Just out of interest: Suppose there is a module A that reads a char like so
Code:
char read (const char *p)
{
    return *p;
}
What is the result of compiling that module?
 
 View user's profile Send private message Visit poster's website 
Reply with quote Back to top
david.prentice
PostPosted: Apr 13, 2012 - 10:44 AM
10k+ Postman


Joined: Feb 12, 2005
Posts: 16256
Location: Wormshill, England

Let me do some crystal ball work.
I guess that this will end up as a multiplexed display that is eventually run by an ISR(). It is being tested in foreground.

You can read the PROGMEM array by using the <avr/pgmspace.h> functions:

Code:

unsigned char pat_count;

unsigned char pat[8] PROGMEM =
{
   0b00000001, 0b00000010, 0b00000100, 0b00001000, 0b00010000, 0b00100000, 0b01000000, 0b10000000,
};

int main (void)
{
  board_init();

  LEDPORT.DIR = 0xff;    // Set all pins of port D to output.
  pat_count = 0;
  while (1)
  {
    LEDPORT.OUT = ~pgm_read_byte(&pat[pat_count]);
    _delay_ms(100);
    if (++pat_count >= 8) {
      pat_count = 0;
    }
  }
}

I have moved the ++pat_count for clarity. It should work just as well in the original place.
A lookup[] array is handy when your bits are not in order. In this case you could just rotate the 'COLUMN_ENABLE' bit.

Code:

ISR()
{
    static char col;
    ROWPORT.OUT = display_buffer[col];
    COLPORT.OUT = ~pgm_read_byte(&pat[col]); //lookup
//    COLPORT.OUT = ~(1 << col);     // calculate
    if (++col >= 8) col = 0;
}

Your foreground code simply 'prints' to the display_buffer[] array. The ISR() needs to fire every 2ms or so.

There is little saving for using an 8-byte PROGMEM array. Both calculate or lookup method will be pretty efficient.

The efficiency is in using a display_buffer[]. e.g you can 'print' the 7-seg value in the buffer when necessary. Otherwise the ISR() needs to find the 7-seg code every multiplex.

David.
 
 View user's profile Send private message Send e-mail  
Reply with quote Back to top
clawson
PostPosted: Apr 13, 2012 - 10:46 AM
10k+ Postman


Joined: Jul 18, 2005
Posts: 62209
Location: (using avr-gcc in) Finchingfield, Essex, England

Quote:

He had a delay. A very long one. You removed it.

Yes but he was only (attempting) to show one array element. I assume the array exists because the intention is to use all the elements it contains? If so then you need to read them one at a time and give the viewer a chance to see each in turn. In effect my whole code was a test bed to show the use of:
Code:
LEDPORT.OUT = ~pgm_read_byte(&pat[pat_count++]);

as it was pgm_read_byte() that he seemed to not know about the requirement to use.

_________________
 
 View user's profile Send private message  
Reply with quote Back to top
ChaunceyGardiner
PostPosted: Apr 13, 2012 - 11:30 AM
Posting Freak


Joined: Mar 09, 2012
Posts: 1452
Location: North Carolina, USA

He did the right thing. He reduced the problem down to the smallest source code that would reproduce the problem as he saw it.

The best way to go about it is telling him what he needs to do to solve the problem he has posted, when that is possible. In this case, that was that he needs to use pgm_read_byte() to access the elements in his array, and he should be able to take it from there.

I told him to use pgm_read_byte() way up there. There was also another useful suggestion from JohanEkdahl to find the relevant tutorial.

What you guys are doing is guessing what his original code looked like, and start advising him based on that guess. That is a rather strange way to approach the problem at hand, unless you want people to stop reducing their problems to the smallest possible representation.

Sometimes you have to guess, or ask for more info, before answering because it is not obvious from the initial post what the problem is. In this case it was quite clear from the start what the problem was. There is no call for throwing in detailed guesswork and irrelevent compiler-specific talk which serves no other purpose than keeping your ego inflated and confuse the OP.
 
 View user's profile Send private message  
Reply with quote Back to top
theusch
PostPosted: Apr 13, 2012 - 02:08 PM
10k+ Postman


Joined: Feb 19, 2001
Posts: 25881
Location: Wisconsin USA

Quote:

He did the right thing. He reduced the problem down to the smallest source code that would reproduce the problem as he saw it.

Indeed, and OP is to be applauded for that. (So often we have to wade through a bunch of code from an app, and/or are only given fragments that do not completely describe the situation.)

Lee
 
 View user's profile Send private message  
Reply with quote Back to top
SprinterSB
PostPosted: Apr 13, 2012 - 08:07 PM
Posting Freak


Joined: Dec 21, 2006
Posts: 1483
Location: Saar-Lor-Lux

@Johan: Still curious about the following:

SprinterSB wrote:
JohanEkdahl wrote:
Other tool chains are implemented so that they recognize that the variable is in flash, and generate the corect LPM instructions.
Just out of interest: Suppose there is a module A that reads a char like so
Code:
char read (const char *p)
{
    return *p;
}
What is the result of compiling that module?
 
 View user's profile Send private message Visit poster's website 
Reply with quote Back to top
theusch
PostPosted: Apr 13, 2012 - 09:53 PM
10k+ Postman


Joined: Feb 19, 2001
Posts: 25881
Location: Wisconsin USA

SprinterSB wrote:

...
What is the result of compiling that module?


I'll speak as a CodeVision user. Indeed, it has had "transparent flash access" since I started working with it, but it has evolved a bit over the years.

I think you are asking how that pointer gets "overloaded" in e.g. CV, and the short answer is that it doesn't. There are no generic pointers in CV. (There have been discussions on that before; I tend to write >>micro<<controller Twisted Evil smaller AVR model apps and I wouldn't want the baggage. But that is IMO.)

CV originally treated "const" and "flash" as mostly equivalent. At some point (2.x? Don't remember) that was deprecated so that "const" has its real C meaning. At another point (don't remember when) "__flash" was introduced to get into the proper C namespace (is that the right term?)

While it is nicely transparent, one must still be aware of what is going on. So e.g. you can't just use memcpy() everywhere; you have to use memcpyf() to copy from flash to SRAM.

Now, I'm used to it and can live with the restrictions. In some cases, perhaps even memcpy(), I suppose the compiler could sort it out. In other cases such as a passed pointer...that would be quite difficult.
 
 View user's profile Send private message  
Reply with quote Back to top
stewpye
PostPosted: Apr 14, 2012 - 12:35 AM
Rookie


Joined: Jul 04, 2009
Posts: 36
Location: Brisbane, Australia

Thanks to all for the replies. I have read the turoials and all replies and it makes sense now. I was trying to do something like in Clawson's code, but using a timer interrupt instead of a delay, which is why I used volatile.

I didn't have a specific application for this, I was just getting used to some of the peripherals on the xmega, c programming, and ASF.

The project I have planned is an analogue sequencer for analogue music synthesizers (similar in operation to roland TR606 or TR808 drum machine), so I will be using an array to store the LED indicators but it will be in RAM. As I'm new to the Xmega, ASF and C programming I want to just get small individual modules working first before I attack the whole problem. At this point it would be easier for me to write it all in assembler but I can see that (hopefully) once I learn C more and understand the libraries and ASF it should be a lot easier.

Regards,
Stewart.
 
 View user's profile Send private message  
Reply with quote Back to top
stewpye
PostPosted: Apr 14, 2012 - 12:51 AM
Rookie


Joined: Jul 04, 2009
Posts: 36
Location: Brisbane, Australia

Sorry, I should have specified I am using GCC from within AVR Studio 5.1.208.

Regards,
Stewart.
 
 View user's profile Send private message  
Reply with quote Back to top
ChaunceyGardiner
PostPosted: Apr 14, 2012 - 01:26 AM
Posting Freak


Joined: Mar 09, 2012
Posts: 1452
Location: North Carolina, USA

stewpye wrote:
The project I have planned is an analogue sequencer for analogue music synthesizers (similar in operation to roland TR606 or TR808 drum machine)

That sounds very cool, I'd like to hear more about that project. (I have a TR808 and a TR909.)

I suppose that is off topic in here, but there is a subforum for off topic stuff if you want to share.
 
 View user's profile Send private message  
Reply with quote Back to top
JohanEkdahl
PostPosted: Apr 14, 2012 - 07:49 AM
10k+ Postman


Joined: Mar 27, 2002
Posts: 18515
Location: Lund, Sweden

SprinterSB wrote:
JohanEkdahl wrote:
Quote:
It's more of a hardware issue than a C issue.
As I hinted at above, I'd say that it is mostly a compiler implementation issue. Other tool chains are implemented so that they recognize that the variable is in flash, and generate the corect LPM instructions.
Just out of interest: Suppose there is a module A that reads a char like so
Code:
char read (const char *p)
{
    return *p;
}
What is the result of compiling that module?


I lost you, Sprinter. Care to be explicit about the point you're trying to make?
 
 View user's profile Send private message Visit poster's website 
Reply with quote Back to top
SprinterSB
PostPosted: Apr 14, 2012 - 09:31 AM
Posting Freak


Joined: Dec 21, 2006
Posts: 1483
Location: Saar-Lor-Lux

You said:
JohanEkdahl wrote:
Other tool chains are implemented so that they recognize that the variable is in flash, and generate the corect LPM instructions.

Thus, I wonder how other toolchains compile a little C module like
Code:
char read (const char *p)
{
    return *p;
}
Or is it not possible to compile modules, build libraries and link against them and instead, everything must be known at compile time and there is effectively only one big source file?

Basically, I am curious about the GNU equivalent of
Code:
avr-gcc foo.c -c -Os -mmcu=atmega8
avr-objdump -d foo.o
for the mentioned module which is
Code:
00000000 <read>:
   0:   fc 01          movw   r30, r24
   2:   80 81          ld   r24, Z
   4:   08 95          ret
and where the LPM you mentioned comes into play.

Notice that legal calls of read include
Code:
void read (const char*);

void read2 (char *p)
{
    read (p);
}
 
 View user's profile Send private message Visit poster's website 
Reply with quote Back to top
david.prentice
PostPosted: Apr 14, 2012 - 09:51 AM
10k+ Postman


Joined: Feb 12, 2005
Posts: 16256
Location: Wormshill, England

Harvard compilers have always coped with this in one of two ways.

1. Use a generic pointer.
2. Use a section specific pointer.

Traditionally, 8051 compilers do both.

AVR compilers use method (2). e.g. CodeVision strcpy() and strcpyf()

An 8051 has __data, __idata, __xdata, __code, ... and several variations of the __xdata addressing.

The overhead involved with (1) is horrendous. Since an AVR only really has SRAM or FLASH, it seems inefficient to cater for (1)

__eeprom memory if available generally requires sophisticated support. In fact CodeVision manages these accesses transparently. OTOH, it does not supply strcpy_E() variants. They are simple to write yourself.

David.
 
 View user's profile Send private message Send e-mail  
Reply with quote Back to top
JohanEkdahl
PostPosted: Apr 14, 2012 - 10:47 AM
10k+ Postman


Joined: Mar 27, 2002
Posts: 18515
Location: Lund, Sweden

I hereby confess that I am utterly stupid.

1) I simply tried to express that I believe some other compiles do not require you to explicitly call a function when you want to access a variable in flash.

2) I am still not understanding what you are asking. In your last post I lose you right about "Thus". What does that module have to do with a compiler recognizing that a variable is in flash and generating code to access it? In order to answer you I need you to be very explicit with what you are questioning in my statement.
 
 View user's profile Send private message Visit poster's website 
Reply with quote Back to top
SprinterSB
PostPosted: Apr 14, 2012 - 11:27 AM
Posting Freak


Joined: Dec 21, 2006
Posts: 1483
Location: Saar-Lor-Lux

Maybe I misunderstood what
Quote:
Other tool chains are implemented so that they recognize that the variable is in flash, and generate the corect LPM instructions.
means. If the compiler is a Standard-C compiler and recognize if a variable shall be located in flash, ten I just wonder how that works without beeing overly inefficient.

Suppose the compiler recognizes that the following varf can be located in flash:
Code:
static const char varF[] = "flash";
and you have a second variable:
Code:
extern char varR;
Now suppose you call the function read from above:
Code:
extern char read (const char*);

char read1 (void)
{
    return read (varF) + read (&varR);
}
For this small fcuntions I am curious about the "recognize that a variable can be in flash and generate LPM".

If there are __flash Qualifiers or things like that needed the answer is trivial, of course, but that's not "recognizing", it's "writing down explicitly what you want".
 
 View user's profile Send private message Visit poster's website 
Reply with quote Back to top
david.prentice
PostPosted: Apr 14, 2012 - 11:51 AM
10k+ Postman


Joined: Feb 12, 2005
Posts: 16256
Location: Wormshill, England

Just look at the way that CodeVision handles this.

An anonymous string is always in flash.
Your example is a named variable. Hence you need a qualifier if you want to override the default SRAM storage.

If you want a single function to access multiple storage areas, you either need a qualifier or a generic pointer.

You could allow a Compiler a certain amount of 'casting'. e.g. if the function prototype specifies a char*, you can 'cast' the char __flash * to a char* by copying to a common SRAM area. The function would receive the arguments that it wants.

Automatic casting of function arguments works fine for promotion e.g. extending a uint8_t to uint32_t.
However these arguments have a limited size.

Silently copying structure or array contents purely to satisfy pointer requirements is horrific. However it may often suit a User application. i.e. a User call rather than a Compiler built-in.

David.
 
 View user's profile Send private message Send e-mail  
Reply with quote Back to top
JohanEkdahl
PostPosted: Apr 14, 2012 - 12:12 PM
10k+ Postman


Joined: Mar 27, 2002
Posts: 18515
Location: Lund, Sweden

I had a long post here that I lost (pilot error). In short:


1) My statement on other compilers was just a statement of fact, not an endorsement.

2) Now I see what you're asking, Sprinter.

3) I do not know the answer. I am curious but not to the point of spending time finding out (I'm a avr-gcc user myself).
 
 View user's profile Send private message Visit poster's website 
Reply with quote Back to top
Display posts from previous:     
Jump to:  
All times are GMT + 1 Hour
Post new topic   Reply to topic
View previous topic Printable version Log in to check your private messages View next topic
Powered by PNphpBB2 © 2003-2006 The PNphpBB Group
Credits