[PIC] Simple But its a serious problem- Unable to Read ADRESH register in PIC16F88

[Vaisakhan Ku]

Hi all;
Please find a simple problem that I have here. Am trying to make an ADC library for PIC16F88. While reading the ADC value, I cant get the value of ADRESH, while the other register ADRESL gets fill over after 255 count and does like this for 4 times over the entire scale of 0 to 5V.

this is my main code

#include "ADC_Library for PIC16f88.h"

  int adc_v=0;
  char temp[7]=" ";
  int chnl_sel,clk_sel;
  
  // LCD Initialization
  sbit LCD_D4 at Rb0_bit;
  sbit LCD_D5 at Rb2_bit;
  sbit LCD_D6 at Rb3_bit;
  sbit LCD_D7 at Rb5_bit;
  sbit LCD_RS at Rb6_bit;
  sbit LCD_EN at Rb7_bit;
  // Pin direction
  sbit LCD_D4_Direction at TRISB0_bit;
  sbit LCD_D5_Direction at TRISB2_bit;
  sbit LCD_D6_Direction at TRISB3_bit;
  sbit LCD_D7_Direction at TRISB5_bit;
  sbit LCD_RS_Direction at TRISB6_bit;
  sbit LCD_EN_Direction at TRISB7_bit;
  

  void main()
    {
      lcd_init();
      lcd_cmd(_lcd_cursor_off);
      while(1)
        {
          adc_v=Adc_read_PIC16F88(0);
          //ltrim(adc_v);
          inttostr(adc_v,temp);
          lcd_out(1,1,temp);

          adc_v=Adc_read_PIC16F88(1);
          //ltrim(adc_v);
          inttostr(adc_v,temp);
          lcd_out(2,1,temp);
          delay_ms(500);

        }
    }

and this is the function which gets called from the main.

  int Adc_read_PIC16F88(int chanal)
    {
      unsigned int adc_value=0,temp_adc=0;
      int adcon_old;
      ansel=0b00000011;
      pie1.adie=0;  //ADC interrupt enabled.
      pir1.ADIF=0;  //Cleared the ADC interrupt.
      adresh=adresl=0; //Clear the ADC values
      delay_ms(10);
      chanal=chanal<<3;   //channal selection
      adcon_old=0b10000001;
      adcon_old=adcon_old|chanal;
      adcon0=adcon_old; //chanal selected wrt the variable chanal.
      delay_us(20);   //for getting enough Tad for 20Mhz
      adcon1=0b11000000;
      adcon0.GO_DONE=1;
      while(pir1.adif==0|| adcon0.GO_done==1)  //polling for adc to complet
      temp_adc=adresh;
      adc_value=adresl;
      temp_adc=temp_adc<<8;
      adc_value= adc_value|temp_adc;
      pir1.ADIF=0;

      return(adc_value);
      
    }

What I had observed is that if am not clearing the ADRSH registre inside the calling function am getting a value above 255 but erratic. I tried to read two different variable to avoid any speed reading problem. But in both cases am getting some random values to the lcd and to the variables. I think the ADRESH register is not getting updated once the ADRESL gets full and dont know the reason. If anyone come across this problem pls provide some lead!!!!

 

[betwixt]

It looks OK to me but I'm not sure which compiler you are using.
Try changing the end of "Adc_read_PIC16F88" to this:

      adc_value= (adresh * 256) + adreshl;
      pir1.ADIF=0;
      return(adc_value);

This kind of error is usually caused by a failure to promote a data type, see if my code works differently.

Brian.

 

[FvM]

Two problems:
- channel selection is wrong, should be chanal=chanal<<2
- unused ADRESH bits are specified as "x = Bit is unknown" in the datasheet.
You have to mask it out, temp_adc=adresh & 3;

adc_value= (adresh * 256) + adreshl;

According to C type conversion rules, adresh*256 is an 8-Bit expression and the result will be always 0. Must use something like (int)adresh*256.

My suggestion:

adc_value= ((int(adresh) & 3) <<8) + adreshl;

 

[betwixt]

My mistake - FvM is correct about the conversion rules but I think shifting the channels by 3 places is correct on that device. The CHS bits are bits 5:3 in ADCON0.

Brian.

 

[FvM]

Right, I was referring to 16F688. 16F88 datasheet tells also that most significant ADRESH bits are always zero. I so far my explanation doesn't apply.

 

[Vaisakhan Ku]

Hi all,

I checked this

adc_value= ((int(adresh) & 3) <<8) + adreshl;

but this is in vain. No change at all.
what i cant understand is this, why ADRESH is not getting updated. I checked the value of that register without any conversion, mean directly took the adresh value to LCD, but its always 0 when am clearing the value of ADRESH prior to the start of ADC conversion !!!!. Any guess, or is there anything that am missing or is there any error in my logic here!!!!

 

[betwixt]

I'm not sure if it has anything to do with it but why are you using both the interrupt flag and the GO_DONE bit?
You should use one or the other, I can't see any reason to do it the way you have. Try removing the interrupt check as you are not using the ADC interrupt anyway and also watch out for capitalization of letters in the bit name!

Brian.

 

[milan.rajik]

1 adc_value is unsigned int type and function is returning int type. Make it return unsigned int.

2 temp should be minimum temp[17] for using with IntToStr(). Read mikroC help file.

3 adc_v is int type. If you fix 1 then change adc_v to unsigned int type.

 

[Vaisakhan Ku]

Hi all,
Brain:The pooling for ADC completion was done with two parameters, go_done and with ADIF, to check if any of them was creating the problem, am now polling only for Go.Done flag, despite that i am getting the same result.

Milan: Implemented the changes, but the result is the same.
Do you have any other suggestion?
I guess this could be a simple problem, but cant find out what is that!!!!

- - - Updated - - -

Brian: As the last problem this will also be a simple typo or conversion error somewhere, hope you remember about the global variable not updating problem that I had, and you helped me to solve that!!!!.....

 

[betwixt]

Unless you have already corrected it, you use:

    adcon0.GO_DONE=1;
      while(pir1.adif==0|| adcon0.GO_done==1)  //polling for adc to complet

note that 'DONE' is used in one line and 'done' in the other. I'm suprised the compiler didn't complain unless both are defined somewhere.

This is an exact copy of some code which is in use in a commercial application, it also uses 16F88:

//*******************************************************************************
//  measure the selected analog input and return it's value
unsigned int ReadADC(char channel)
{
	ADCON0 &= 0xC7;					// clear the CHS bits
	ADCON0 |= ((channel & 0x07) << 3);	// select measurment channel

	ShortDelay(10);		// time for new input to settle

	ADCON0 |= (1 << GO);		// start conversion
	while(ADCON0 & (1 << GO))	// kill time until ADC finshed
	{
		#asmline clrwdt
	}
	return ((unsigned int)ADRESH << 8) + (unsigned int)ADRESL;	
}

I wrote it using a different compiler but I think you can see how it works. The "ShortDelay(10)" is a 10uS delay routine and "#asmline clrwdt" inserts an assembly instuction to clear the watchdog timer.

Brian.

 

[FvM]

note that 'DONE' is used in one line and 'done' in the other. I'm suprised the compiler didn't complain unless both are defined somewhere.

The comment points to a basic problem of the discussion:
- we don't know the involved compiler, e.g. if it's case insensitive by default
- we dont know the referenced include files

Apart from cases with obvious coding faults, it's unlikely to get the solution by posting necessarily incomplete information at Edaboard. Instead I would suggest this debugging method:
- Inspect the generate assembly code
- Debug the code by stepping through it in MPLAB

 

[milan.rajik]

The compiler he is using is mikroC PRO PIC. He could easily use ADC_Read(x) function.