![](templates/subSilver/images/CCSLogo.jpg) |
![CCS C Software and Maintenance Offers](templates/subSilver/images/forumAd6.jpg) |
View previous topic :: View next topic |
Author |
Message |
sterob
Joined: 24 Jun 2011 Posts: 9
|
strange ADC problem |
Posted: Thu May 24, 2012 7:48 am |
|
|
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);
}
}
}
}
|
|
|
![](templates/subSilver/images/spacer.gif) |
Ttelmah
Joined: 11 Mar 2010 Posts: 19358
|
|
Posted: Thu May 24, 2012 8:14 am |
|
|
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 |
|
![](templates/subSilver/images/spacer.gif) |
sterob
Joined: 24 Jun 2011 Posts: 9
|
|
Posted: Thu May 24, 2012 8:27 am |
|
|
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. |
|
![](templates/subSilver/images/spacer.gif) |
RF_Developer
Joined: 07 Feb 2011 Posts: 839
|
Re: strange ADC problem |
Posted: Thu May 24, 2012 9:40 am |
|
|
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 |
|
![](templates/subSilver/images/spacer.gif) |
SherpaDoug
Joined: 07 Sep 2003 Posts: 1640 Location: Cape Cod Mass USA
|
|
Posted: Fri May 25, 2012 6:00 am |
|
|
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. |
|
![](templates/subSilver/images/spacer.gif) |
sterob
Joined: 24 Jun 2011 Posts: 9
|
|
Posted: Sat May 26, 2012 5:10 am |
|
|
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
}
|
|
|
![](templates/subSilver/images/spacer.gif) |
dyeatman
Joined: 06 Sep 2003 Posts: 1924 Location: Norman, OK
|
|
Posted: Sat May 26, 2012 8:07 am |
|
|
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!!!! |
|
![](templates/subSilver/images/spacer.gif) |
SherpaDoug
Joined: 07 Sep 2003 Posts: 1640 Location: Cape Cod Mass USA
|
|
Posted: Sat May 26, 2012 4:27 pm |
|
|
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. |
|
![](templates/subSilver/images/spacer.gif) |
PCM programmer
Joined: 06 Sep 2003 Posts: 21708
|
|
Posted: Sun May 27, 2012 12:12 pm |
|
|
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. |
|
![](templates/subSilver/images/spacer.gif) |
sterob
Joined: 24 Jun 2011 Posts: 9
|
|
Posted: Sun May 27, 2012 5:49 pm |
|
|
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! |
|
![](templates/subSilver/images/spacer.gif) |
PCM programmer
Joined: 06 Sep 2003 Posts: 21708
|
|
Posted: Sun May 27, 2012 5:52 pm |
|
|
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(). |
|
![](templates/subSilver/images/spacer.gif) |
|
|
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
|