I have to following code:
// Write screen n = sprintf( screen_msg, " TV: %#4.1f FV: %#4.1f CV: %#4.1f SV: %#4.1f\r\n", ad_value[2], ad_value[1], ad_value[0], ad_value[3] ); n += sprintf( screen_msg + n, " TI: %#4.1f AT: %#4.1f CT: %#4.1f ST: %#4.1f\r\n", ad_value[4], ad_value[5], ad_value[7], ad_value[6] ); n += sprintf( screen_msg + n, "\r\n" ); n += sprintf( screen_msg + n, "ENGINE CHARGE CLK EN DIS" ); write_0( screen_msg );
When the string is printed to my LCD CV, SV, and ST are always zero, regardless of the value in the ad_value array. If I print just those values in the same routine they appear as they should. When I debug the program the entire string appears as it should. My screen_msg buffer is 256 characters and the entire string in only 170.
I have looked over the code a dozen times and can't figure out what's going on. Can anyone see something that I may be missing?
Thanks, Paul
Hello,
Here is the root fault: sprintf() returns NUMBER OF CHARACTERS written (or a negative value if an output error occurred), so trying to use the n to shift pointer is totally wrong. Example: your first line would return 4 so at your 2nd line you refresh buffer starting at screen_msg[4] having had screen_msg filled-up well after that. And... from line to line you have even more interesting behaviour with shift value not really being grown as you probably expected - in reality, you are hitting near the same index.
So, while troubleshooting, it is worth to check values of every variable in the scope + get familiar with ANSI C functions description...
-- Nikolay.
Nikolay,
That is the way I have seen it written in pretty much every example that I have read. Should I check n after every sprintf() to make sure my index increments as expected?
Also, if I just write the first line I have the same results. No index is involved for that one.
Paul
I've never seen string handling done like that either (I like to keep things simple so would write sprintf to a temporary string and strcat() it to the message buffer after each step) but after a quick glance at your code I can't see why it shouldn't work.
How are you declaring screen_msg?
unsigned char xdata screen_msg[256];
Just remembered this: http://www.keil.com/support/docs/867.htm
Hmm, lost the end of the previous post..
Does the limitation apply to your situation?
Paul,
Sorry, I confused you with the notation of return value of sprintf(). The function does return number of characters written, so in my example I should be showing an estimate value around 40 (for floats like 1.1, 2.2, 3.3, ...), and not 4 as I wrote (0 omitted). In other words, what you have, YES, IT IS VALID WAY to go, but nevertheless care should be taken to check how the n is really changing - did you check it is changing? - Don't you hit the same index?
However typically in cases like this one I would use sprint() to temporary long enough buffer, then concatenate it to the main string using strcat() + do the verification no array boundaries are crossed.
Try to check whether XDATA places the buffer in a right location. You may just write to the buffer IMPLICITLY, then read it back. You may have problem accessing physical memory. Try place the string to the near memory/reduce size if needed, and play with the single line - does it help?
Regards, Nikolay.
Also, you may want to check whether ad_value[] buffer gets both its right values and location in the address space. Try to substitute ad_value[] with some constants when doing sprintf(). Try to write and then read back the ad_value[] to verify access is OK. Hope, some of it will help.
Nicolay: I just said that (the first bit anyway) :-)
<a bit off topic> Do you think we can convince Paul to ditch the floating point and the printf library, switch to fixed point numbers and roll his own code to concatenate the numbers to his string?
Does this print valid data?
strcpy(screen_msg,"hello world"); write_0(screen_msg);
Your use of sprintf() should work unless the C51 architecture have specific problems.
But, why bother with sprintf and many, many values when all floats have a common formatting and are stored in an array?
p = screen_msg; for (i = 0; i < 8; i++) { n = sprintf(p,"%s %4.1f ",prompt[i],ad_val[i]); if (n < 0) return FAIL; p += n; } strcpy(p,"\r\nMy final message here"); wrrite_0(screen_buffer);
And, as already mentioned, do consider fixed-point arithemetic for your poor chip.
Mike,
Yes, you are the first ;-) - I was posting this particular message w/o resfreshing the list. I would say, many stuff in this world depends on habits and style so I should not be so categorical saying "it is totally wrong", what I meant ;-) is actually, "I would not recommend this approach" - because in my opinion constructions like n+= sprintf(buffer + n, ...) are prone to be messed-up with, easily. That is the reason when you look at glance (typically, at glance ;-) ) at somebody's code you might not really dig deep into it but the very first suspicion might be brought up due to the syntax only. And only after that if you have deeper look at the code (as it happened here with me), yes, you can say: "well, it works..." but still simpler syntax arouses more confidence. Anyway, this is just my vision..
The link you posted did help a lot. I was sending too many bytes to sprintf.
As for ditching the floating point and printf libraries, I would love to come over to the dark side, just let me know of any good resources. I'm always happy to learn new, and aboviously better, ways of doing things. I have been programing in Windows for so long I need to rethink the basics!
Thanks for all your help.
After reading your first post again I understood what you were trying to say. I agree with you in that the structure as written is prone error. With your help and Mike's help I fixed the problem.
When I started out using XDATA meant an external chip and all the adress decoding hardware that went with it. And using printf once in a project would eat up half the available internal ROM so it was always avoided if possible. These days it isn't as important.
Fixed point means that for example if all your numbers are positive, less than 6553 and you only nead one decimal place then instead of using floats, use 16 bit integers and store them as 10 times the actual value. Then when you come to print the value just manually insert the decimal place before the last figure.
A simplified routine to convert such a number to a string is as follows. It would need a little bit of work to remove any leading zero's and it could be tidied up a lot but it should give you an idea.
char *shorttostr (unsigned short value) { static unsigned char valuestr[5]; valuestr[5] = '\0'; valuestr[0] = (value/1000); valuestr[1] = ((value-(1000*valuestr[0]))/100); valuestr[2] = ((value-(1000*valuestr[0])-(100*valuestr[1]))/10); valuestr[4] = (value-(1000*valuestr[0])-(100*valuestr[1])-(valuestr[2]*10)); valuestr[0] +='0'; valuestr[1] +='0'; valuestr[2] +='0'; valuestr[3] = '.'; valuestr[4] +='0'; return (valuestr); }
MOst often, integers are emitted into a temporary buffer from least significant digit. When the conversion is done, and you know how many digits the result become, you then check if the data should be padded with one or more spaces or zeroes and if a sign needs to be added.
Conversion right-to-left requires one division, one modulo and no multiply for each emitted digit and is well-suited for a loop.
But if a project already needs (s)printf() for other reasons, there is no reason to not use it. It isn't really so big. The bad part is for the processor to have to emulate floating point. And if the compiler treat double as an alias for float, there can be problems with the precision, since floating point can not represent most numbers exactly.