Author Topic: C Code Problem  (Read 12998 times)

0 Members and 1 Guest are viewing this topic.

Offline DigibinTopic starter

  • Regular Contributor
  • *
  • Posts: 90
C Code Problem
« on: January 12, 2015, 09:56:05 pm »
Hey

I've got an issue with some C code I'm working on. It's for a bat detector project. It measures the frequency of a LO and displays this on a LCD screen. It also polls a rotary encoder and displays a counter that goes up and down with movement of the encoder. It also talks to two 5k digital potentiometers that are hooked up in series. Changing the pots causes the LO to vary from approx 15kHz to 100kHz. The idea is to have everything integrated so that the encoder controls the LO frequency and this is measured and displayed to the user.

Before integrating it together I wanted to make the frequency control more linear. The LO is way more sensitive to each step change in potentiometer at the higher frequencies. So I took some measurements and created an array in my code that stores the values that I want the potentiometer to cycle through in order to get linear frequency control. This is when my code broke...

I've attached the code, main() is in Bat Detector v2. The problem area is right at the bottom of main(). The code, as attached, compiles with no warnings but instead of seeing the measured frequency on the display it gives a series of random characters/symbols. The problem to the best of my knowledge is related to the line:

unsigned int digipot_value = digipot_linear_values[enc_count];

So this is meant to the element in the array that corresponds to 'enc_count' and copy it to 'digipot_value'. It works just fine with this line, but if 'digipot_value' is then used in either of the following functions sprintf() or update_digipot(), the LCD displays rubbish. If I comment out those two functions it works fine. With either of them in, it breaks. Both functions work fine if another variable is passed or if a straight integer is passed. It's only the digipot_value variable that's causing trouble. So I suspect something is going on with respect to the array I'm using.

Somehow the strings that I pass to the lcd_puts() function are being messed with, resulting in jibberish on the screen. But even the string 'char_freq_Hz_avg' is messed up even though  it seemingly has no connection to the code in the problem area. This leads me to think that somehow the memory is being messed up.

I'm not really familiar with pointers (haven't worked with them much) but this may be related to how C deals with arrays as pointers to the first memory address, perhaps?

Been struggling with this one for a while. Any ideas?

Cheers.
 

Online IanB

  • Super Contributor
  • ***
  • Posts: 11771
  • Country: us
Re: C Code Problem
« Reply #1 on: January 12, 2015, 10:11:22 pm »
It seems you keep adding "result" onto "enc_count" without ever resetting enc_count. Is this really what you intend?
 

Offline jav

  • Contributor
  • Posts: 37
Re: C Code Problem
« Reply #2 on: January 12, 2015, 10:49:28 pm »
Also, you should make sure that len > 2 (from len = strlen(char_freq_Hz_avg);) before continuing or really bad things may happen when you try to modify char_freq_Hz_avg[len-3].
 

Offline dgtl

  • Regular Contributor
  • *
  • Posts: 183
  • Country: ee
Re: C Code Problem
« Reply #3 on: January 12, 2015, 11:46:06 pm »
Stack overflow? You are using quite a lot of large variables; AVRs do not have too much memory.

Just some ideas to optimize the code:
Declare constant (non-changeable) arrays as const. The digipot_linear_values array will otherwise be copied to memory at start; instead it should be accessed from flash as needed. Flash is not modifiable (at least for C code), thus everiything non-const goes to ram.
The digipot_linear_values array could actually be specified as sparse array. For example, you store only every 2nd or fourth value; then perform linear approximation for values inbetween. Using power of two (2,4,8,...) as the step of the array makes calculations much faster. For longer arrays, the space savings may be quite large and the added code space taken by the calculation could be less.
Also, you should make static the functions and global variables that are not used outside that compilation object (outside that c file). This way the compiler may find better ways to optimize the code if it does not have to keep the standard interface (to be compatible in case maybe somebody calls from outside). Then you can make more smaller functions; for example move the lcd output of frequency and lcd ouput of digipot value to separate functions, that just take value as argument. it is much more  readable to write update_digipot(value); update_lcd_digipot(value);. I like the code style that every function must fit fully on screen; if you need to scroll, it is too long.

freq_Hz_avg divide by three rounds always down. Maybe use 4 samples and add value of 2 before dividing by 4?

The main and TIMER0_OVF_vect are not synchronized properly? freq_Hz can be modified from ISR at the same time it is read from main (reading multi-byte data from ram is not atomic, some bytes may change during the read)? Also, you are not waiting until measurement is complete, so you may add the same value twice etc. The IRQ handler might fire for example after evaluating switch condition, so you'll use new result with old switch condition. One way would be to move the closing brace from line 170 to line 208 and the init_freq_measurement also to line 208. So you handle the result only if it is ready and then start another.
You do not have to stop the timers when taking result reading. Just read the TCNT value as the first thing in the IRQ handler and substract the last value (that you got the last IRQ handler run). Then store the TCNT value you read out to a variable for using it next time. Just disable the irqs while reading the volatile global variables to local variables to get consistent state, then re-enable.
Instead of 3-measurement average, you could also use a simple low-pass filter, like exponential moving average. For example, after every measurement, output=(1/4)*input+(3/4)*last_output. This can be optimized: sum = sum - output + input; output = sum/4; (store both sum and output vars). This could be also calculated in IRQ handler as it is fast enough. Then you have only one global variable shared between irq and main, the filtered output frequency itself. As it is multi-byte, still disable irqs while reading it out to a local var in main.
How precise the freq measurement has to be? If some kind of interrupt fires and wastes time at init_freq_measurement between lines 59/60, your timings and result would be off. If it is important, disable IRQs during those 2 lines. The same happens when something delays timer0 ovf irq. If you have spare pins, you could connect timer0 output compare output pin to timer1 input capture pin and handle the timer1 input capture IRQ. Then capturing is done on every cycle of timer0 by hardware and you just post-process the result. You get single-clock-cycle precision.

sprintf is large and slow. If it is the only use of it you may consider replacing it. Even divide-output-divide-output unrolled cycle would be smaller for uint16.

enc_count may go out of bounds (line216). C does not check array bounds. Check it yourself, do not allow the value to go outside of bounds. Also, enc_count may be uint8_t?

Hope this helps. Maybe the real problem is somewhere else, but these are the things that I happened to notice.
 

Offline dannyf

  • Super Contributor
  • ***
  • Posts: 8221
  • Country: 00
Re: C Code Problem
« Reply #4 on: January 13, 2015, 12:24:06 am »
I just went through your code quickly.

You did a good job modularizing and commenting the code. NO obvious problems jumping out at me.

I would suggest that you try to isolate the bug by passing given values to the various code pieces and see if they produce the desired behaviors. For example, if you suspect that the lcd_puts() isn't doing its job right, initialize a string and pass it to lcd_puts() and see what happens.
================================
https://dannyelectronics.wordpress.com/
 

Offline dannyf

  • Super Contributor
  • ***
  • Posts: 8221
  • Country: 00
Re: C Code Problem
« Reply #5 on: January 13, 2015, 12:43:21 am »
lcd_puts() is fine.

The issue is with the conversion of freq_Hz_avg to a string, when freq_Hz_avg is less than 1000.

You used a lot of string functions, particularly sprintf(). They are quite expensive in terms of space + time. Try to think something else.

In the mean time, you can replace the whole section with a one-liner:

Code: [Select]
sprintf(char_freq_Hz_avg, "%lu.%lu kHz   ", freq_Hz_avg/1000, (freq_Hz_avg % 1000) / 100);
================================
https://dannyelectronics.wordpress.com/
 

Offline Howardlong

  • Super Contributor
  • ***
  • Posts: 5313
  • Country: gb
Re: C Code Problem
« Reply #6 on: January 13, 2015, 10:46:04 am »
I would agree that the sprintf probably needs looking at. It is both flash and RAM hungry.

I don't know the sprintf implementation on the device you're using, but if I am reading the device's spec sheet right you only have 256 or 512 bytes of RAM available on an attiny48. Quite frequently sprintf implementations use heap, and some implementations automatically require floating point primitives, but either way it will be using a lot of RAM. With only 4/8k flash and  256/512 bytes RAM I'd look at less generic methods of generating ASCII.

If that sprintf fails, then you do that strlen on the resulting string, the results are unlikely to be very nice.
 

Offline DigibinTopic starter

  • Regular Contributor
  • *
  • Posts: 90
Re: C Code Problem
« Reply #7 on: January 13, 2015, 09:32:05 pm »
It seems you keep adding "result" onto "enc_count" without ever resetting enc_count. Is this really what you intend?

Yes - 'result' can only be -1, 0, or +1. So 'enc_count' just follows CW/CCW turns of the encoder. However I agree that it should be bounded manually as pointed out later.

Also, you should make sure that len > 2 (from len = strlen(char_freq_Hz_avg);) before continuing or really bad things may happen when you try to modify char_freq_Hz_avg[len-3].

Good point. In this case would it go the memory address one prior to the start of the array (whatever may be there) and operate on that location?

Just some ideas to optimize the code:

Really useful post, thanks very much. Your tips make a lot of sense and I will implement them before further developing the code.

lcd_puts() is fine.

The issue is with the conversion of freq_Hz_avg to a string, when freq_Hz_avg is less than 1000.

You used a lot of string functions, particularly sprintf(). They are quite expensive in terms of space + time. Try to think something else.

In the mean time, you can replace the whole section with a one-liner:

Code: [Select]
sprintf(char_freq_Hz_avg, "%lu.%lu kHz   ", freq_Hz_avg/1000, (freq_Hz_avg % 1000) / 100);


Yep that's definitely a more elegant way of doing it. The only reason I'm using sprintf()is because I need to convert integers to strings for the lcd_puts() function. Previously I was using itoa() but moved to sprintf() because it seems more formal; it's an implicit function of C (to my understanding anyway). What are the alternatives to sprintf() in my case?

What do you mean specifically by 'when freq_Hz_avg is less than 1000'?

I would agree that the sprintf probably needs looking at. It is both flash and RAM hungry.

When I build the code in AVR Studio it gives me:

Program Memory Usage     : 3286 bytes   80.2% Full
Data Memory Usage           : 195 bytes     76.2% Full

So presumably it's not a lack of memory. Having said that, how would you implement integer to character conversion more efficiently?
 

Offline senso

  • Frequent Contributor
  • **
  • Posts: 951
  • Country: pt
    • My AVR tutorials
Re: C Code Problem
« Reply #8 on: January 13, 2015, 10:47:11 pm »
If you are using all that RAM at compile time, its sure that you are blowing up the stack when that sprintf function gets called.
Use a micro with more ram or change the code.
 

Offline jav

  • Contributor
  • Posts: 37
Re: C Code Problem
« Reply #9 on: January 13, 2015, 10:47:39 pm »
Also, you should make sure that len > 2 (from len = strlen(char_freq_Hz_avg);) before continuing or really bad things may happen when you try to modify char_freq_Hz_avg[len-3].

Good point. In this case would it go the memory address one prior to the start of the array (whatever may be there) and operate on that location?

What are the alternatives to sprintf() in my case?

What do you mean specifically by 'when freq_Hz_avg is less than 1000'?

It'll depend on the alignment of the array. If it isn't at the beginning of a block, it'll probably overwrite the previous addresses. It may also "wrap around" to higher addresses and overwrite those.

When  freq_Hz_avg < 100, len will be < 3 and you'll run into that problem.

If you only need to convert integers it's more efficient in terms of code size to write your own itoa function. You can write it specifically for the format you need.

For instance:
Code: [Select]
void itokhz(unsigned int freq, unsigned char *buffer) {
  int started = 0;

  if (freq >= 10000) {
    *buffer++ = '0' | (freq / 10000);
    freq %= 10000;
    started = 1;
  }
  if (started || freq >= 1000) {
    *buffer++ = '0' | (freq / 1000);
    freq %= 1000;
  } else
    *buffer++ = '0';

  *buffer++ = '.';
  if (freq >= 100) {
    *buffer++ = '0' | (freq / 100);
    freq %= 100;
  }
  if (freq >= 10) {
    *buffer++ = '0' | (freq / 10);
    freq %= 10;
  }
  *buffer++ = '0' | freq;

  strcpy(buffer, " kHz");
}

You can even optimize it further depending of the cost of divisions in your architecture or statically allocating the output buffer within the function if you don't require reentrancy.
 

Offline dannyf

  • Super Contributor
  • ***
  • Posts: 8221
  • Country: 00
Re: C Code Problem
« Reply #10 on: January 13, 2015, 10:50:28 pm »
Quote
What are the alternatives to sprintf() in my case?

itoa(), either the stock version or your own roll -> which can be quickly implemented using a look-up table.
================================
https://dannyelectronics.wordpress.com/
 

Offline Sal Ammoniac

  • Super Contributor
  • ***
  • Posts: 1660
  • Country: us
Re: C Code Problem
« Reply #11 on: January 14, 2015, 07:25:01 am »
Is this a commercial project (and, hence, cost sensitive), or a one-off?

If it's a one-off, why bother using a memory constricted MCU like an ATTiny when for just a little bit more you can use an ARM Cortex-M0 or M3 that won't have the tight memory issues?
Complexity is the number-one enemy of high-quality code.
 

Offline Jeroen3

  • Super Contributor
  • ***
  • Posts: 4064
  • Country: nl
  • Embedded Engineer
    • jeroen3.nl
Re: C Code Problem
« Reply #12 on: January 14, 2015, 08:04:30 am »
If you are using all that RAM at compile time, its sure that you are blowing up the stack when that sprintf function gets called.
Use a micro with more ram or change the code.
Increase the stack. Generate a callgraph to know the size.
Every function call uses stack. This stack consists of the cpu registers + local variables. And you must always have enough free stack to fit your biggest interrupt routine. Again, check the callgraph to know the stack of each routine.

I once tried C on an Attiny10. Not a single function call is possible in that tiny memory. Its called tiny for a reason.
And use snprintf if you can.
 

Offline Howardlong

  • Super Contributor
  • ***
  • Posts: 5313
  • Country: gb
Re: C Code Problem
« Reply #13 on: January 14, 2015, 08:15:37 am »
Is this a commercial project (and, hence, cost sensitive), or a one-off?

If it's a one-off, why bother using a memory constricted MCU like an ATTiny when for just a little bit more you can use an ARM Cortex-M0 or M3 that won't have the tight memory issues?

... and the small matter of learning a whole new development paradigm, getting the toolchains to work, etc... and six weeks later, maybe have enough to be productive.  Just sayin' :)
 

Offline dannyf

  • Super Contributor
  • ***
  • Posts: 8221
  • Country: 00
Re: C Code Problem
« Reply #14 on: January 14, 2015, 11:58:16 am »
Quote
ARM Cortex-M0 or M3

The existing code fits an Attiny. I would use a mega if I already have the chip. It is crazy to go to a CM0/3 chip for that.

printf(), depending on implementation, takes about 2kb of space. So if you are tight on space, thinking about taking it out.

One way to go is something like this:

Code: [Select]
  strcpy(char_freq_Hz_avg, str1); //copy string into lcd buffer
  freq_Hz_avg /= 100; //get to the hundreds
  char_freq_Hz_avg[6]=(freq_Hz_avg % 10) + '0'; freq_Hz_avg /= 10; //convert the hundreds
  char_freq_Hz_avg[5]='.';
  char_freq_Hz_avg[4]=(freq_Hz_avg % 10) + '0'; freq_Hz_avg /= 10; //convert the thousands
  char_freq_Hz_avg[3]=(freq_Hz_avg % 10) + '0'; freq_Hz_avg /= 10; //convert the ten thousands
  ...
  //if you don't want leading zero, use this instead
  if (freq_Hz_avg)   char_freq_Hz_avg[3]=(freq_Hz_avg % 10) + '0'; freq_Hz_avg /= 10; //convert the ten thousands

Do the same with other sprintf() invocations and you should save quite a lot of space.
================================
https://dannyelectronics.wordpress.com/
 

Offline Sal Ammoniac

  • Super Contributor
  • ***
  • Posts: 1660
  • Country: us
Re: C Code Problem
« Reply #15 on: January 14, 2015, 07:15:31 pm »
... and the small matter of learning a whole new development paradigm, getting the toolchains to work, etc... and six weeks later, maybe have enough to be productive.  Just sayin' :)

It typically takes me about three days to get up to speed on a new processor/toolchain to the point where I'm productive writing C code for that platform. I guess I assumed everyone can do that. I guess I thought wrong.  ::)
Complexity is the number-one enemy of high-quality code.
 

Offline Howardlong

  • Super Contributor
  • ***
  • Posts: 5313
  • Country: gb
Re: C Code Problem
« Reply #16 on: January 14, 2015, 09:44:17 pm »
In which case you are super clever!

Does that include the time spent figuring out the difference between the gazillions of offerings out there, and evaluating which one would work for your requirement?

Either way, to be productive, ie realistically writing production code, for a new platform in three days froms scratch, is a very big ask indeed.

Fannying about trying to understand the USB stack or something, yes, three days, but you wouldn't be writing production code at that point but maybe I'm just a thicko!
« Last Edit: January 14, 2015, 09:51:19 pm by Howardlong »
 

Offline Sal Ammoniac

  • Super Contributor
  • ***
  • Posts: 1660
  • Country: us
Re: C Code Problem
« Reply #17 on: January 15, 2015, 01:24:35 am »
Either way, to be productive, ie realistically writing production code, for a new platform in three days froms scratch, is a very big ask indeed.

It only took me a week to port an RTOS from ARM Cortex-M3 to PIC32. I had never used any of the PIC tools before, nor did I have any experience with MIPS before I started. Where there's a will, there's a way.

I'm not saying any of this to brag--just that learning a new processor and/or environment isn't that hard and needn't take very long for an experienced person. I have 30+ years experience--perhaps that's why it doesn't take me long to get up to speed on something new.
Complexity is the number-one enemy of high-quality code.
 

Offline DigibinTopic starter

  • Regular Contributor
  • *
  • Posts: 90
Re: C Code Problem
« Reply #18 on: January 15, 2015, 09:57:10 pm »
Okay I've gone through the code and cleaned it up based on the various comments. Thanks for all the advice. However the problem still persists.

I've not moved to an alternative to sprintf() yet but I will do soon (limited time in the evenings to work on this). It sounds like sprintf() may be an issue but I'm not seeing exactly why simply using this function causes the strings in memory to be corrupted? I have enough memory, according to the Program Memory Usage and Data Memory Usage reported by the compiler, yes?

Either way I will try an alternative.

For instance:
Code: [Select]
void itokhz(unsigned int freq, unsigned char *buffer) {
  int started = 0;

  if (freq >= 10000) {
    *buffer++ = '0' | (freq / 10000);
    freq %= 10000;
    started = 1;
  }
  if (started || freq >= 1000) {
    *buffer++ = '0' | (freq / 1000);
    freq %= 1000;
  } else
    *buffer++ = '0';

  *buffer++ = '.';
  if (freq >= 100) {
    *buffer++ = '0' | (freq / 100);
    freq %= 100;
  }
  if (freq >= 10) {
    *buffer++ = '0' | (freq / 10);
    freq %= 10;
  }
  *buffer++ = '0' | freq;

  strcpy(buffer, " kHz");
}

Code: [Select]
  strcpy(char_freq_Hz_avg, str1); //copy string into lcd buffer
  freq_Hz_avg /= 100; //get to the hundreds
  char_freq_Hz_avg[6]=(freq_Hz_avg % 10) + '0'; freq_Hz_avg /= 10; //convert the hundreds
  char_freq_Hz_avg[5]='.';
  char_freq_Hz_avg[4]=(freq_Hz_avg % 10) + '0'; freq_Hz_avg /= 10; //convert the thousands
  char_freq_Hz_avg[3]=(freq_Hz_avg % 10) + '0'; freq_Hz_avg /= 10; //convert the ten thousands
  ...
  //if you don't want leading zero, use this instead
  if (freq_Hz_avg)   char_freq_Hz_avg[3]=(freq_Hz_avg % 10) + '0'; freq_Hz_avg /= 10; //convert the ten thousands

Okay took me a while to realise that in both these cases you're actually manually setting the ASCII code of each char according to each digit in the integer. Didn't get this until I looked up the ASCII code of zero. In the second example, what is the purpose of the strcpy() line? Does 'str1' need to be initialised to something specific or...?
 

Offline AndreasF

  • Frequent Contributor
  • **
  • Posts: 251
  • Country: gb
    • mind-dump.net
Re: C Code Problem
« Reply #19 on: January 16, 2015, 09:36:16 am »
I agree with the others that it's worthwhile going away from sprintf. There is the potential memory use/corruption issue, and given that your requirements of converting your measurements from int to char[] are very predictable, you don't really need the flexibility of sprintf. I've used similar approaches to the ones already given in the past. I'v even simplified it further by not using divisions or modulus operations:
Code: [Select]
char temp = '0';
char started = 0;
while (freq>=10000) {
    temp++;
    freq-=10000;
    started = 1;
}
if (started) {
    LCD_putChar(temp);  //or whatever the actual function is to send a single char
} else {
    LCD_putChar(' ');
}
temp = '0';

while (freq>=1000) {
    temp++;
    freq-=1000;
}
LCD_putChar(temp);

LCD_putChar('.')

temp = '0';
while (freq>=100) {
    temp++;
    freq-=100;
}
LCD_putChar(temp);

temp = '0';
while (freq>=10) {
    temp++;
    freq-=10;
}
LCD_putChar(temp);


As you can see, this also gets rid of the buffer. Since each decimal place is determined individually, you can directly write it to the LCD instead of into a string buffer. That would also avoid having to write " kHz" every single time. It only needs to be written once (or after every LCD reset/clear) and should just stay on the display from then on.

To further avoid divisions, you may want to consider taking four samples instead of 3 (and why not summing them directly?), and then dividing by 4, which can be achieved with a simple bitshift by 2.

Lastly, how confident are you that the LCD functions don't also use your timer?

Quote
Okay I've gone through the code and cleaned it up based on the various comments. Thanks for all the advice. However the problem still persists.

Does your string buffer end with 0 as a proper 'end-of-string' marker?  Otherwise lcd_puts may simple keep sending random memory content to the LCD (until it hits a 0 byte).
my random ramblings mind-dump.net
 

Offline DigibinTopic starter

  • Regular Contributor
  • *
  • Posts: 90
Re: C Code Problem
« Reply #20 on: January 16, 2015, 12:51:59 pm »
Code: [Select]
char temp = '0';
char started = 0;
while (freq>=10000) {
    temp++;
    freq-=10000;
    started = 1;
}
if (started) {
    LCD_putChar(temp);  //or whatever the actual function is to send a single char
} else {
    LCD_putChar(' ');
}
temp = '0';

while (freq>=1000) {
    temp++;
    freq-=1000;
}
LCD_putChar(temp);

LCD_putChar('.')

temp = '0';
while (freq>=100) {
    temp++;
    freq-=100;
}
LCD_putChar(temp);

temp = '0';
while (freq>=10) {
    temp++;
    freq-=10;
}
LCD_putChar(temp);


As you can see, this also gets rid of the buffer.

Thanks for the code, I'll give this a go. I've been trying the other examples too but with no luck - don't understand why so I'll post more details on that later.

What buffer are you referring to? The one mentioned in the example below?

Code: [Select]
void itokhz(unsigned int freq, unsigned char *buffer) {
  int started = 0;

  if (freq >= 10000) {
    *buffer++ = '0' | (freq / 10000);
    freq %= 10000;
    started = 1;
  }
  if (started || freq >= 1000) {
    *buffer++ = '0' | (freq / 1000);
    freq %= 1000;
  } else
    *buffer++ = '0';

  *buffer++ = '.';
  if (freq >= 100) {
    *buffer++ = '0' | (freq / 100);
    freq %= 100;
  }
  if (freq >= 10) {
    *buffer++ = '0' | (freq / 10);
    freq %= 10;
  }
  *buffer++ = '0' | freq;

  strcpy(buffer, " kHz");
}

If so I'm not sure I understand it's purpose to begin with! My understanding is that the function expects a dereferenced pointer to a char variable (i.e. the value stored in the location it's pointing to). So does this mean I have to actually pass it a pointer variable that's initialised to point to my string, or can I just pass it the string directly? i.e.

itokhz(freq_Hz_avg, *my_pointer);    (where 'my_pointer' points to &char_freq_Hz_avg)

or

itokhz(freq_Hz_avg, char_freq_Hz_avg);

To further avoid divisions, you may want to consider taking four samples instead of 3 (and why not summing them directly?), and then dividing by 4, which can be achieved with a simple bitshift by 2.

Why is avoiding divisions desirable? Purely to minimise computational effort, or for other reasons too?

Lastly, how confident are you that the LCD functions don't also use your timer?

Pretty sure - it uses software delays for its timing. I've not had a problem with the LCD with any of the code I originally posted except for when I tried to use the array. Without the array, sprintf() works just fine and the code behaves as I intended. So I'm still not seeing where the actual problem is, I'm sure it's related to the integer array.

Does your string buffer end with 0 as a proper 'end-of-string' marker?  Otherwise lcd_puts may simple keep sending random memory content to the LCD (until it hits a 0 byte).

Again not sure what you mean by buffer - my string is my string, it's not buffering anything that I'm aware of. I write to it, and push it to the LCD. But again this all works fine in the absence of the array-related code. So I can't help but suspect that these lines are somehow affecting the strings, but can't see how they might be doing so as they don't involve the string variables.
 

Offline sleemanj

  • Super Contributor
  • ***
  • Posts: 3020
  • Country: nz
  • Professional tightwad.
    • The electronics hobby components I sell.
Re: C Code Problem
« Reply #21 on: January 16, 2015, 01:24:40 pm »
I have enough memory, according to the Program Memory Usage and Data Memory Usage reported by the compiler, yes?

No.

You mistake that report for "this is all the memory you will ever use", which it is not.  It is how much memory is used to begin with.

Every time you call a function, a stack frame is allocated to cater for the local variables used within that function call, this does not form part of the linked size, it's a runtime allocation.  When the function exits, the stack frame is released.  Additionally functions may at run time dynamically allocate memory from the heap. 

Neither stack growth, nor heap growth can be known at compile time because the compiler, does not know in what exact manner your program will run, what functions it will call, how "deep" it will go, and how much heap will be consumed at any given time, because this is all determined by your program behaviour, inputs etc.  And therefore, it could not know that, without running the program, with infinite combinations of inputs, which is of course, impossible.



~~~
EEVBlog Members - get yourself 10% discount off all my electronic components for sale just use the Buy Direct links and use Coupon Code "eevblog" during checkout.  Shipping from New Zealand, international orders welcome :-)
 

Offline DigibinTopic starter

  • Regular Contributor
  • *
  • Posts: 90
Re: C Code Problem
« Reply #22 on: January 16, 2015, 01:35:02 pm »
I've tried creating a function based on danny's suggested code:

Code: [Select]
static void itokhz(unsigned int freq, char char_freq[])
{ char_freq[7]= '\0'; // ensure string is properly terminated
freq /= 100; // get to the hundreds
char_freq[6]=(freq % 10) + '0'; // convert the hundreds to ASCII char
freq /= 10; // get to the thousands
char_freq[5]='.'; // insert decimal point
char_freq[4]=(freq % 10) + '0'; // convert the thousands to ASCII char
freq /= 10; // get to the ten thousands
if (freq) // if the ten thousands is non-zero
{ char_freq[3]=(freq % 10) + '0'; // convert the ten thousands
freq /= 10; // get to the hundred thousands
}
if (freq) // if the hundred thousands is non-zero
{ char_freq[2]=(freq % 10) + '0'; // convert the hundred thousands to ASCII char
freq /= 10;
}
if (freq) // if the millions is non-zero
{ char_freq[1]=(freq % 10) + '0'; // convert the millions to ASCII char
freq /= 10;
}
if (freq) // if the tens of millions is non-zero
{ char_freq[0]=(freq % 10) + '0'; // convert the tens of millions to ASCII char
freq /= 10;
}
}

I've attached the full code (attempt1.c). Note that I've commented out the code that was causing my original issue so I can focus on replacing sprintf(). Lines 204 and 205 now contain the two alternative methods, sprintf() and itokhz(). Using sprintf() it's all fine. But when I use itokhz(), instead of seeing the frequency as I'd expect, the bottom line of the LCD for some reason displays the 'enc_count' variable (which is also displayed on the top line, as intended). And it updates slower than the top line (just like the frequency string should do because of the time it takes to do the averaging), so for some reason line 208 is putting 'enc_count' to the LCD instead of 'freq_char_Hz_avg'. I suspect this may be again something related to the other sprintf() on line 233, because if I comment out these four lines the display goes blank. But I would still expect the frequency to be displayed on the bottom line.

I had a look at the lcd.h file and the lcd_puts() function is declared as:

extern void lcd_puts(const char *s);

I've not had any issues with this previously so I'm sure it's fine, but out of curiosity, if the function expects a const char, could there be a problem in passing a variable char? Seems unlikely as it would be rather limited if the char had to be const.
 

Offline DigibinTopic starter

  • Regular Contributor
  • *
  • Posts: 90
Re: C Code Problem
« Reply #23 on: January 16, 2015, 01:39:53 pm »
I have enough memory, according to the Program Memory Usage and Data Memory Usage reported by the compiler, yes?

No.

You mistake that report for "this is all the memory you will ever use", which it is not.  It is how much memory is used to begin with.

Every time you call a function, a stack frame is allocated to cater for the local variables used within that function call, this does not form part of the linked size, it's a runtime allocation.  When the function exits, the stack frame is released.  Additionally functions may at run time dynamically allocate memory from the heap. 

Neither stack growth, nor heap growth can be known at compile time because the compiler, does not know in what exact manner your program will run, what functions it will call, how "deep" it will go, and how much heap will be consumed at any given time, because this is all determined by your program behaviour, inputs etc.  And therefore, it could not know that, without running the program, with infinite combinations of inputs, which is of course, impossible.

Oh right, that makes sense. So when people say that sprintf() needs say 2kb, that's not allocated to it at compile time? And therefore I need to ensure that there is sufficient memory left after compiling to execute it?
 

Offline Jeroen3

  • Super Contributor
  • ***
  • Posts: 4064
  • Country: nl
  • Embedded Engineer
    • jeroen3.nl
Re: C Code Problem
« Reply #24 on: January 16, 2015, 02:01:07 pm »
Standard library functions sometimes require C-runtime heap. Heap is the opposite of stack. You allocate something in heap using malloc. Heap is defined in the startup file. (hidden in the project settings for you avr users)
Sprintf does not return anything dynamically, so it probably only uses stack.

Converting an integer to a char stream is not that difficult. I used to have this routine to printout integers for debugging.
Code: [Select]
void sTX_putint_ascii(int data){
unsigned int i;
for(i=10000;i>=1;i=i/10){
    sTX_putchar ( ((data/i)%10)+48 );    //digit = (input / divisor) Mod 10

}
Just make sure you end your string with a NULL.
 


Share me

Digg  Facebook  SlashDot  Delicious  Technorati  Twitter  Google  Yahoo
Smf