I'm getting odd results passing shorter integers to a fn with a uint23_t argument.
The fn is part of GRBL ( i've just changed the second char output for n==0 to be sure this is the path taken)
void print_uint8_base16(uint8_t n)
{
if (n == 0) {
serial_write('0');
serial_write('x');
return;
}
unsigned char buf[2];
uint8_t i = 0;
while (n > 0) {
buf[i] = n % 16 ;
if (buf[i]> (char) 9 ) {buf[i]+='@' -(char) 9;} else {buf[i]+='0' ;};
i++;
n /= 16;
}
for (; i > 0; i--)
serial_write(buf[i - 1]);
}
To debug some code I'm adding it needed to print out some values, results were not as expected, so I printed out the same thing using functions with different length arguments. uint16_base16() is as the above but extended to print four chars, it does work as expected.
This code produces the following on the Arduino monitor.
[sh#1 status = FFFF
255
FF
0x ]
[sh#2 status = FFFF ]
For conversion to a type of width N, the value is reduced modulo 2^N to be within range of the type; no signal is raised.
What I can't see is why the 8bit fn above is getting sent a byte that it tests as being equal to zero, when all the other outputs show the 32b value is 0xFFFFFFFF.
This does not seem to be in agreement with the modulo given in the gcc spec.
That is weird. Tried your code using VC++, and GNU GCC on x86. Both came back with FF when resp was set to 0xff and and'ed with 0xff. Didn't seem to matter whether resp was signed or not.
Guess this is the offending part for Arduino compiler:
Sorry Nick, I missed that one of the code block did not get the tag. Since I did use it twice you don't need to tell where it is ( though I agree the icon is so obscure it does need explaining. )
I've fixed the code block, thanks.
That's not my code, as I said it's part of GRBL. The competition is get something that is fast enough to run stepper motors at upto 30kHz. That means not wasting time doing everything tens time over with things like digitalWrite and writing efficient code. Sometimes that becomes a little less obvious read but it's not too hard to follow.
Since the problem has already occured by the first line
if (n == 0) {
You don' t need to go in to that anyway though I'm surprised to see an old hand like yourself confused by something so simple.
tammytam:
That is weird. Tried your code using VC++, and GNU GCC on x86. Both came back with FF when resp was set to 0xff and and'ed with 0xff. Didn't seem to matter whether resp was signed or not.
Guess this is the offending part for Arduino compiler:
resp & 0xFF;
Thanks for confirming I'm not hallucinating. You are correct, without the mask it produces the expected result.
Thanks, I'd also tried extending your minimal sketch with masked line and it produces expected output.
print_uint8_base16 (resp & 0xFF);
I'll have to try building up the call sequence to see where the problem starts.
It maybe a size problem the modified GRBL is getting a bit close to the limits:
31,130 bytes
There's utility fn in GRBL and it is showing 500 bytes free just before the (n==0) test.
// Debug tool to print free memory in bytes at the called point. Not used otherwise.
void printFreeMemory()
{
extern int __heap_start, *__brkval;
uint16_t free; // Up to 64k values.
free = (int) &free - (__brkval == 0 ? (int) &__heap_start : (int) __brkval);
printInteger((int32_t)free);
printString(" ");
}
I recall that you did some detailed work on leaks in the string libraries and produced some memory usage fns. Does this calculation look legit?
Thanks Nick, that looks useful , the Andy Brown free mem figure is about 80b less than grbl fn , showing 361b , so it does not look like that is the problem.
I think the main problem here is implicit type conversions are not doing what I expect and the wrong values are getting passed to functions.
Is there a way to turn on -Wall when compiling in the "IDE"?
BTW where is size-t defined, I can't find stdlib.h in /usr/share/arduino
Thanks, but where is it? I can't see this in /usr/share either.
One of the problems with the Arduino "IDE" is that there are so many different bits spread across the system, I'm never sure where to look or what is actually getting compiled.
exiting CmdGet3StatusParam: 0000FE13
00FE13
[sh#1 status = 0000
0000:0000
So the uint32_t that gets passed as the return value and the individual variables have the expected values leaving CmdGet3StatusParam() but by the time control gets back to the caller the value seems to be zero.
Here is the 32b printing fn ( simple extension of the 8 and 16b versions. )
Again please don't get into trouble trying to understand what the code is doing beyond first two lines. The arg tests as being zero and the string "0000:0000\r\n" gets output.
void print_uint32_base16(uint32_t n)
{
if (n == 0) {
printPgmString(PSTR("0000:0000\r\n"));
return;
}
unsigned char buf[8];
uint8_t i=0 ,j = 0;
while (n > 0) {
buf[i] = n % 16 ;
if (buf[i]> (char) 9 ) {buf[i]+='@' -(char) 9;} else {buf[i]+='0' ;};
i++;
n/=16;
}
for (j=8;j>i;j--){serial_write('0');}
for (; i > 0; i--)
serial_write(buf[i - 1]);
printPgmString(PSTR("\r\n"));
}
I'm starting to get to the cause of the FF==0 business.
I've changed the return value in CmdGet3StatusParam() to uint16_t as well as the local variable used to calculate the return value. That is all 16b now.
The output lines in the caller are still as before, one 16b and one 32b , so there is an implicit cast to 32b in the latter case:
printPgmString(PSTR("\r\n[sh#1 status = "));
print_uint16_base16(res);
printPgmString(PSTR("\r\n"));
print_uint32_base16(res);
printPgmString(PSTR("\r\n"));
and the output produces is:
exiting CmdGet3StatusParam: FE130541
00FE13
[sh#1 status = FE13
FFFFFE13
The implicit cast in calling the 32b display routine before returning over-runs in to arbitrary data, FE13 being the correct value.
Now having returned a 16b value and assigned it to a uint32_t we see that it gets stretched left but using sign extension as though it was in int , not a uint.
The return value is a uint , the local variable to which it is assigned is a uint and the the header of the output fn shows it taking a uint.
void print_uint32_base16(uint32_t n)
So why is the value getting typecast as though it was in an signed integer?!
'@' is the character in front of ascii 'A'. I prefer 'A'-10 to '@'-9 in this situation: it compiles down to the same thing, but is clearer.
The equivalent is
buf[i] += buf[i] < 10 ? '0' : ('A'-10);
What annoys me is the trouble that he is going to to do zero padding - putting the digits into a buffer, counting the digits, then printing out leading zeros to fill and then finally what was put in the buffer. If you know you want 8 digits, print 8 digits:
for(int i = 32-4; i>=0; i -= 4) {
char hexdigit = (n>>i) & 0xF;
serial_write(hexdigit + (hexdigit<10?'0':'A'-10));
}
But that's not why his code is giving odd results.
His code is giving odd results because he is putting potentially 8 digits of hex into a buffer that's two digits long, and clobbering other things on the stack.
Thanks Paul. Please don't get annoyed. The leading zeros thing was just a quick hack to get consistent length output to visualise this for debugging. It's not part of the original code which, once again is not mine. I did not want to recode the whole only to revert it later.
Your comment about clobbering the stack is the sort of thing I'm look for and may be what I'm getting wrong.
I was expecting that if I call a fn with a 16b arg , using a 32b variable there was an implicit typecast. The GCC doc that I linked to above about this said that the longer value would be truncated to modulo N^2 to ensure that it fits in the range of the smaller variable.
What you are saying is that this is not what happens.
Maybe if I could turn on warnings with -Wall in the IDE I would have got a warning about this and saved myself three days chasing phantoms.
Do you know if that is possible?
Perhaps you could also comment on why the return value gets lost as described in #16.
Despite all being done with int32_t it seems that only the MS-word is getting passed back then being extended to be 32b zero. :?