Post by thread:[PIC]10-bit ADC integer formatting question

I am using the PIC18F4320. I am writing in C-language and using MPLAB C18 tool suite(compiler V2.2). I have a question about the ADC result registers (ADRESL and ADRESH). I must use all 10 bits; therefore, I have to do the simple task of taking the two 8-bit result registers and combining them into a 16-bit integer. The following code is how I did it: (obviously before this I set the ADC result to be right justified and VBattNew is defined as an unsigned int) (I also send the raw values out the USART before the statements below - getting exactly what one would expect in each of the bytes) //Sent data out USART here VBattNew = ADRESL; // Get low byte //Sent data out USART here VBattNew |= (ADRESH << 8); // Combine with the high byte into 16 int //Sent data out USART here The problem is the high byte continually gets cleared to 0x00 - I cannot get past 255 decimal. I even sent the raw data out the USART before, between and after for inspection, and it is clear the left shift and integer assignment is clearing the high byte? I tried the code below to make sure I was not trying to do much at once with the above syntax - same problem: // VBattNew = ADRESL; // Get low byte // temp = ADRESH; // temp << 8; // VBattNew = temp | VBattNew; The final fix which works is using the ADRES definition in the p18f4320.h file - which apparently is the entire 16-bit concatenation of the two bytes; as follows: VBattNew = ADRES; // This works But, I want to understand why my code did not work - I bet it is surprisingly simple (as always - its one bit screwing things up). By the way I did search the archives and found someone posted code the same way I did it - should be a slam dunk. Thanks for your time guys, Shawn

----- Original Message ----- From: "Leslie, Shawn J" <spam_OUTsjlesliTakeThisOuTsandia.gov> >The final fix which works is using the ADRES definition in the >p18f4320.h file - which apparently is the entire 16-bit concatenation of >the two bytes; as follows: >VBattNew = ADRES; // This works >But, I want to understand why my code did not work - I bet it is >surprisingly simple (as always - its one bit screwing things up). By the >way I did search the archives and found someone posted code the same way >I did it - should be a slam dunk. I don't know how this might affect you in C, but I believe that ADRESH and ADRESL are in different banks. You will need to look at the generated assembly code to see why your way didn't work and the ADRES way does. This is one reason that I always recommend that people program in assembler first. That way when (not if) the compiler fails to generate code that works, they can look at the assembler output and figure out what went wrong.

This is just speculation, but it sounds to me like the left shift operation (ADRESH << 8) is being performed on a single byte before being ORed and assigned to VBattNew. Shifting a byte-sized variable by eight bits will shift all of the data out of the variable, clearing it. Try the following code: VBattNew = ADRESH; VBattNew = (VBattNew << 8) | ADRESL; This should have the same result as your fix, without requiring a temp variable. It may be useful to examine the assembly code generated to see exactly what is going on. Regards, Alex Parkinson Leslie, Shawn J wrote: {Quote hidden}> I am using the PIC18F4320. I am writing in C-language and using MPLAB > C18 tool suite(compiler V2.2). I have a question about the ADC result > registers (ADRESL and ADRESH). I must use all 10 bits; therefore, I have > to do the simple task of taking the two 8-bit result registers and > combining them into a 16-bit integer. The following code is how I did > it: > > (obviously before this I set the ADC result to be right justified and > VBattNew is defined as an unsigned int) > > (I also send the raw values out the USART before the statements below - > getting exactly what one would expect in each of the bytes) > > //Sent data out USART here > VBattNew = ADRESL; // Get low byte > //Sent data out USART here > VBattNew |= (ADRESH << 8); // Combine with the high byte into 16 > int > //Sent data out USART here > > > The problem is the high byte continually gets cleared to 0x00 - I cannot > get past 255 decimal. > > I even sent the raw data out the USART before, between and after for > inspection, and it is clear the left shift and integer assignment is > clearing the high byte? > > I tried the code below to make sure I was not trying to do much at once > with the above syntax - same problem: > > // VBattNew = ADRESL; // Get low byte > // temp = ADRESH; > // temp << 8; > // VBattNew = temp | VBattNew; > > The final fix which works is using the ADRES definition in the > p18f4320.h file - which apparently is the entire 16-bit concatenation of > the two bytes; as follows: > > VBattNew = ADRES; // This works > > But, I want to understand why my code did not work - I bet it is > surprisingly simple (as always - its one bit screwing things up). By the > way I did search the archives and found someone posted code the same way > I did it - should be a slam dunk. > > Thanks for your time guys, > Shawn >

Shawn wrote: > VBattNew |= (ADRESH << 8); C's handling of expressions involving integers of different sizes is often not what you want. Here, the 8-bit quantity from ADRESH is being shifted left, still as an 8-bit quantity; the result is always zero, so VBattNew will never be modified. You need to extend ADRESH first, so there's room to shift it: VBattNew |= (((unsigned int)ADRESH) << 8); It may be possible to get rid of one of the sets of parentheses, but why risk it - the order of operations in C is also often not what you want, better to be explicit about it. Jason Harper

On Tue, 29 Mar 2005, Leslie, Shawn J wrote: > //Sent data out USART here > VBattNew = ADRESL; // Get low byte > //Sent data out USART here > VBattNew |= (ADRESH << 8); // Combine with the high byte into 16 > int > > The problem is the high byte continually gets cleared to 0x00 - I cannot > get past 255 decimal. C18 does not do the integer promotions by default. Your code is correct in standard C, but with C18 you need to either explicitly cast ADRESH to a 16-bit type, or add the integer promotion command line switch. > I tried the code below to make sure I was not trying to do much at once > with the above syntax - same problem: > > // temp << 8; That doesn't do anything, even in standard C. -- John W. Temples, III

Thanks for all the input - I understand all of your points and I will try your suggestions soon when I get back on the bench; then let ya'll know how it goes. I agree with Michaels input on programming in assembler first - compilers WILL mess you up from time to time. In a "perfect world" I would be more proficient at assembly; however, when programming maybe two or three different chips (Atmel, PICs, etc) over 1-2 years time, among "other" design responsibilities, the use of C-language allows me to be more efficient. Granted, imbedded programming requires knowing the chips details, but I assume it would be much harder to program in assembler and be efficient with changes etc, unless you really specialize. That being said, this forum is great and I really appreciate the time everyone lends to help a guy out that may not specialize as much. My bacon has been saved many times - and I understand the PICs so much better now. Thanks Again, Shawn {Original Message removed}

Just a quick note to say your suggestions worked. Thanks again. -----Original Message----- From: .....piclist-bouncesKILLspam@spam@mit.edu [piclist-bouncesKILLspammit.edu] On Behalf Of Alex Parkinson Sent: Tuesday, March 29, 2005 9:25 AM To: Microcontroller discussion list - Public. Subject: Re: [PIC]10-bit ADC integer formatting question This is just speculation, but it sounds to me like the left shift operation (ADRESH << 8) is being performed on a single byte before being ORed and assigned to VBattNew. Shifting a byte-sized variable by eight bits will shift all of the data out of the variable, clearing it. Try the following code: VBattNew = ADRESH; VBattNew = (VBattNew << 8) | ADRESL; This should have the same result as your fix, without requiring a temp variable. It may be useful to examine the assembly code generated to see exactly what is going on. Regards, Alex Parkinson Leslie, Shawn J wrote: {Quote hidden}> I am using the PIC18F4320. I am writing in C-language and using MPLAB > C18 tool suite(compiler V2.2). I have a question about the ADC result > registers (ADRESL and ADRESH). I must use all 10 bits; therefore, I > have to do the simple task of taking the two 8-bit result registers > and combining them into a 16-bit integer. The following code is how I > did > it: > > (obviously before this I set the ADC result to be right justified and > VBattNew is defined as an unsigned int) > > (I also send the raw values out the USART before the statements below > - getting exactly what one would expect in each of the bytes) > > //Sent data out USART here > VBattNew = ADRESL; // Get low byte > //Sent data out USART here > VBattNew |= (ADRESH << 8); // Combine with the high byte into 16 > int > //Sent data out USART here > > > The problem is the high byte continually gets cleared to 0x00 - I > cannot get past 255 decimal. > > I even sent the raw data out the USART before, between and after for > inspection, and it is clear the left shift and integer assignment is > clearing the high byte? > > I tried the code below to make sure I was not trying to do much at > once with the above syntax - same problem: > > // VBattNew = ADRESL; // Get low byte > // temp = ADRESH; > // temp << 8; > // VBattNew = temp | VBattNew; > > The final fix which works is using the ADRES definition in the > p18f4320.h file - which apparently is the entire 16-bit concatenation > of the two bytes; as follows: > > VBattNew = ADRES; // This works > > But, I want to understand why my code did not work - I bet it is > surprisingly simple (as always - its one bit screwing things up). By > the way I did search the archives and found someone posted code the > same way I did it - should be a slam dunk. > > Thanks for your time guys, > Shawn >