CCS C Software and Maintenance Offers
FAQFAQ   FAQForum Help   FAQOfficial CCS Support   SearchSearch  RegisterRegister 

ProfileProfile   Log in to check your private messagesLog in to check your private messages   Log inLog in 

CCS does not monitor this forum on a regular basis.

Please do not post bug reports on this forum. Send them to support@ccsinfo.com

strange ADC problem

 
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion
View previous topic :: View next topic  
Author Message
sterob



Joined: 24 Jun 2011
Posts: 9

View user's profile Send private message

strange ADC problem
PostPosted: Thu May 24, 2012 7:48 am     Reply with quote

Hi, everyone, I am working on a project that requires sampling some 6 adc signals and output through 6 pins, if the input is greater than a reference (set in the code). The problem arises whenever i sample more than two signals, Pin_C2 (related to the third channel, in the code) flickers (i.e goes on and off repeatedly), whereas if a sample just 1, or two, the outputs won't flicker. Please, I need help. Am using compiler version 4.114. Here's the code:
Code:

#include <18F2550.h>
#DEVICE ADC=8

#FUSES NOWDT                    //No Watch Dog Timer
#FUSES WDT128                   //Watch Dog Timer uses 1:128 Postscale
#FUSES PLL1                     //No PLL PreScaler
#FUSES CPUDIV1                  //No System Clock Postscaler
#FUSES NOUSBDIV                 //USB clock source comes from primary oscillator
#FUSES XT                       //Crystal osc <= 4mhz for PCM/PCH , 3mhz to 10 mhz for PCD
#FUSES NOFCMEN                  //Fail-safe clock monitor disabled
#FUSES NOIESO                   //Internal External Switch Over mode disabled
#FUSES PUT                      //Power Up Timer
#FUSES NOBROWNOUT               //No brownout reset
#FUSES BORV20                   //Brownout reset at 2.0V
#FUSES VREGEN                   //USB voltage regulator enabled
#FUSES PBADEN                   //PORTB pins are configured as analog input inels on RESET
#FUSES LPT1OSC                  //Timer1 configured for low-power operation
#FUSES MCLR                     //Master Clear pin enabled
#FUSES STVREN                   //Stack full/underflow will cause reset
#FUSES NOLVP                    //No low voltage prgming, B3(PIC16) or B5(PIC18) used for I/O
#FUSES NOXINST                  //Extended set extension and Indexed Addressing mode disabled (Legacy mode)
#FUSES NODEBUG                  //No Debug mode for ICD
#FUSES NOPROTECT                //Code not protected from reading
#FUSES NOCPB                    //No Boot Block code protection
#FUSES NOCPD                    //No EE protection
#FUSES NOWRT                    //Program memory not write protected
#FUSES NOWRTC                   //configuration not registers write protected
#FUSES NOWRTB                   //Boot block not write protected
#FUSES NOWRTD                   //Data EEPROM not write protected
#FUSES NOEBTR                   //Memory not protected from table reads
#FUSES NOEBTRB                  //Boot block not protected from table reads

#use delay(clock=20000000)
#use rs232(baud=9600,xmit=PIN_C6,rcv=PIN_C7,bits=8)



void main()
{
   int i;i;
   float intake[6], scaled_intake[6], scale;
   scale = 2.00;
   setup_adc(ADC_CLOCK_INTERNAL); // ADC clock
   setup_adc_ports(AN0_TO_AN8); // Input combination
   while(1)
   { 
      for(i =0; i < 9; i++ ) // Read 8 inputs
      {
     // Since pic18f2550 does not have AN5,AN6, and AN7, this lines skips through!
         if(i==5)
         {
            i+=1;
         }
         if(i==6)
         {
            i+=1;
         }
         if(i==7)
         {
            i+=1;
         }
         delay_ms(1000); // Wait 1 sec
         set_adc_channel(i); // Select inel
         intake[i]= read_adc(); // Get input
         scaled_intake[i]=(intake[i])/51; // Scale input
       printf( " INPUT%u = %4.3f \r",i ,scaled_intake[i]); //,scaled_intake[1],scaled_intake[4],scaled_intake[11]); // Display intake
         if(input(PIN_D0))
         {
            if(scaled_intake[0]<scale)
            {
               output_high(PIN_C0);
            }
            else if(scaled_intake[0]>scale)
            {
               output_low(PIN_C0);
            }
            if(scaled_intake[1]<scale)
            {
               output_high(PIN_C1);
            }
            else if(scaled_intake[1]>scale)
            {
               output_low(PIN_C1);
            }
            if(scaled_intake[2]<scale)
            {
               output_high(PIN_C2);
            }
            else if(scaled_intake[2]>scale)
            {
               output_high(PIN_C3);
            }
         // Similarly repeated for the remaining 3 channels
         }
         else if(!input(PIN_D0))
         {
            delay_ms(10);     // switch debounce
            output_high(PIN_D2);
         }
      }
   }
}
Ttelmah



Joined: 11 Mar 2010
Posts: 19218

View user's profile Send private message

PostPosted: Thu May 24, 2012 8:14 am     Reply with quote

Tacq.....

The ADC takes _time_ to acquire a signal after a channel is selected. Depends on the impedance of the source, but typically 10 to 20uSec. You _must_ wait for this time after selecting a channel, before reading. Otherwise the voltage from the _last_ selected channel will still largely be on the internal capacitor, and the channels will interfere with one another.....

Best Wishes
sterob



Joined: 24 Jun 2011
Posts: 9

View user's profile Send private message

PostPosted: Thu May 24, 2012 8:27 am     Reply with quote

Ttelmah wrote:
Tacq.....

The ADC takes _time_ to acquire a signal after a channel is selected. Depends on the impedance of the source, but typically 10 to 20uSec. You _must_ wait for this time after selecting a channel, before reading. Otherwise the voltage from the _last_ selected channel will still largely be on the internal capacitor, and the channels will interfere with one another.....

Best Wishes

I put a delay of 1sec inside the for loop to check for sampling time.
RF_Developer



Joined: 07 Feb 2011
Posts: 839

View user's profile Send private message

Re: strange ADC problem
PostPosted: Thu May 24, 2012 9:40 am     Reply with quote

There's lots wrong, or confused and confusing with this code. Here's a few ideas:

sterob wrote:


Code:

#include <18F2550.h>
#DEVICE ADC=8

// 18F2550 has a ten bit ADC. Why throw away 4 good bits of useful data? Better to set it to 10 bit mode and adjust your scaling.

#FUSES NOWDT                    //No Watch Dog Timer

// I'm assuming your fuses are OK. Could be wrong....

#use delay(clock=20000000)
#use rs232(baud=9600,xmit=PIN_C6,rcv=PIN_C7,bits=8)

// Best to include errors to ensure UART doesn't lock up on error.

void main()
{
   int i;i;

// Why is i; repeated? Most compilers would raise an error for that.
// Delete the repeat.


   float intake[6], scaled_intake[6], scale;

// Are all these really needed to be floats? No, intake should really be an int16 for 10 or 8 bit ADC mode, or int8 for 8 bit mode. Also intake doesn't need to be an array, as its a temporary variable.

   scale = 2.00;

// Ah, so scale doesn't change, therefore it is a constant. Use
// const float scale = 2.0f;  or #define SCALE 2.0f

   setup_adc(ADC_CLOCK_INTERNAL); // ADC clock

// Nearly always a bad idea to use the internal RC clock for the
// ADC. Better to use a clock divided from the main oscillator.


   setup_adc_ports(AN0_TO_AN8); // Input combination

  while(1)
   { 
      for(i =0; i < 9; i++ ) // Read 8 inputs

// No, that loops NINE times, 0, 1, 2... 7, 8. Should be
//       for(i =0; i < 8; i++ ) // Read 8 inputs

      {
     // Since pic18f2550 does not have AN5,AN6, and AN7, this lines skips through!
         if(i==5)
         {
            i+=1;
         }
         if(i==6)
         {
            i+=1;
         }
         if(i==7)
         {
            i+=1;
         }

// What? You really wanted to loop only six times, so loop six times and only do something special on the last loop as its not on the expected ADC channel. Lets try that again:

      for(i =0; i < 6; i++ ) // Read 6 inputs
      {
         delay_ms(1000); // Wait 1 sec
// No, this delay, which is far too long anyway, must be after the set_adc_channel() and before the read_adc(). You can configure the ADC hardware to do it for you using ADC_TAD_MUL_x.
         if (i == 5)
         {
             set_adc_channel(8); // Select inel
         }
         else
         {
             set_adc_channel(i); // Select inel
         }

         intake[i]= read_adc(); // Get input
         scaled_intake[i]=(intake[i])/51; // Scale input

// Why put the brackets aroiund intake[i]? They are not needed.
// Why have intake as a float? The ADC result is an integer, storing
// it as a float just wastes processor time and memory. In this case
// storing it at all is a waste. Simply read the ADC and convert in
// one go like this:

        scaled_intake[i] = read_adc() / 51.0f;

// Note the ".0f" making sure the constant is a float. Division takes
// longer than multipication, so I prefer to make all my scaling
// constants multipliers:

        scaled_intake[i] = read_adc() * 0.0196078f;

// Its not precisely the same but its more than close enough for
// this application.


       printf( " INPUT%u = %4.3f \r",i ,scaled_intake[i]); //,scaled_intake[1],scaled_intake[4],scaled_intake[11]); // Display intake
         if(input(PIN_D0))
         {
            if(scaled_intake[0]<scale)
            {
               output_high(PIN_C0);
            }
            else if(scaled_intake[0]>scale)
            {
               output_low(PIN_C0);
            }
            if(scaled_intake[1]<scale)
            {
               output_high(PIN_C1);
            }
            else if(scaled_intake[1]>scale)
            {
               output_low(PIN_C1);
            }
            if(scaled_intake[2]<scale)
            {
               output_high(PIN_C2);
            }
            else if(scaled_intake[2]>scale)
            {
               output_high(PIN_C3);
            }

// Wait a moment, in this loop you are only measuring one
// channel each time. There's no point is checking every channel in
// every loop.  Try using a switch block here, one case for each loop possibility.
         // Similarly repeated for the remaining 3 channels
         }
         else if(!input(PIN_D0))
         {

// the if(!input(PIN_D0)) is almost, but not quite redundant here. If // PIN_D0 was 0 we'll execute this code. So why check it again?
// Unless there's some strange possibility that it may have
// changed between then and now (less than a microsecond) AND
// its important, which is isn't. Else means else.

            delay_ms(10);     // switch debounce
            output_high(PIN_D2);
         }
      }
   }
}


RF Developer
SherpaDoug



Joined: 07 Sep 2003
Posts: 1640
Location: Cape Cod Mass USA

View user's profile Send private message

PostPosted: Fri May 25, 2012 6:00 am     Reply with quote

If you just rearrange these lines you will solve your immediate problem:
Code:

         set_adc_channel(i); // Select inel
         delay_ms(1000); // Wait 1 sec
         intake[i]= read_adc(); // Get input

You need a delay AFTER selecting the A/D channel and BEFORE reading it.
_________________
The search for better is endless. Instead simply find very good and get the job done.
sterob



Joined: 24 Jun 2011
Posts: 9

View user's profile Send private message

PostPosted: Sat May 26, 2012 5:10 am     Reply with quote

Hi everyone, thanks for your suggestions. I've been able to fix the problem, i observed that the channel 8 was not responding, probably due to my "for() loop", so I decided to use a 40pin chip, instead of skipping, through 5, 5,and 7. Also, I changed the "for loop" to scan through only six channels. I also applied your suggestions, Thanks a million buck! Here is an excerpt of the changes made:
Code:

for(i=0; i<6; i++ ) // scan from 0 through 5
         {
            set_adc_channel(i); // Select channel
            delay_ms(100); // Wait 1 sec
            intake [i]= read_adc(); // Get input
            takn[i]=((intake[i])*scale); // Scale intake
            printf( " takin%u = %4.3f \r", i  , takn[i]); // Display displays each
         }
dyeatman



Joined: 06 Sep 2003
Posts: 1912
Location: Norman, OK

View user's profile Send private message

PostPosted: Sat May 26, 2012 8:07 am     Reply with quote

BTW, the delay after setting the channel only needs to be 20 to 30
microseconds not milliseconds depending on input impedance. In
your case just use 30 to be safe. If you want a pause between the reads
put a line with the desired delay (i.e 500ms) after the printf.
_________________
Google and Forum Search are some of your best tools!!!!
SherpaDoug



Joined: 07 Sep 2003
Posts: 1640
Location: Cape Cod Mass USA

View user's profile Send private message

PostPosted: Sat May 26, 2012 4:27 pm     Reply with quote

Usually right after I read the A/D I select the next channel. By the time I have scaled the result and stored it away the A/D is ready for the next reading without any explicit delay. I still count the cycles to make sure though.
_________________
The search for better is endless. Instead simply find very good and get the job done.
PCM programmer



Joined: 06 Sep 2003
Posts: 21708

View user's profile Send private message

PostPosted: Sun May 27, 2012 12:12 pm     Reply with quote

Quote:
#include <18F2550.h>
#DEVICE ADC=8

#FUSES NOWDT
#FUSES WDT128
#FUSES PLL1
#FUSES CPUDIV1
#FUSES NOUSBDIV
#FUSES XT // Crystal osc <= 4mhz for PCM/PCH ,

#use delay(clock=20000000)

Did you notice the comment about the XT fuse, and then compare it
to your #use delay() frequency ?

What is your real crystal frequency ? Is it 4 MHz or 20 MHz ?
If it's 20 MHz, the XT fuse won't work. The PIC won't run with that fuse.
It requires the HS fuse for 20 MHz.

Because you have this mistake, I suspect that this is really a Proteus
project. Proteus doesn't care about mistakes like that, but real hardware
won't work at all.
sterob



Joined: 24 Jun 2011
Posts: 9

View user's profile Send private message

PostPosted: Sun May 27, 2012 5:49 pm     Reply with quote

Quote:
Did you notice the comment about the XT fuse, and then compare it
to your #use delay() frequency ?

What is your real crystal frequency ? Is it 4 MHz or 20 MHz ?
If it's 20 MHz, the XT fuse won't work. The PIC won't run with that fuse.
It requires the HS fuse for 20 MHz.

Because you have this mistake, I suspect that this is really a Proteus
project. Proteus doesn't care about mistakes like that, but real hardware
won't work at all.


I am using a 4MHz crystal, yeah you are right, I simulated with proteus!
PCM programmer



Joined: 06 Sep 2003
Posts: 21708

View user's profile Send private message

PostPosted: Sun May 27, 2012 5:52 pm     Reply with quote

If you have a 4 MHz crystal and you set the #use delay() at 20 MHz,
all delay_ms() and delay_us() statements will run 5 times longer
than they should.

You must match the oscillator frequency and the #use delay().
Display posts from previous:   
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion All times are GMT - 6 Hours
Page 1 of 1

 
Jump to:  
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum


Powered by phpBB © 2001, 2005 phpBB Group