|
|
View previous topic :: View next topic |
Author |
Message |
skoberlink
Joined: 27 May 2010 Posts: 52
|
odd A/D conversion behavior |
Posted: Wed Jan 12, 2011 8:48 am |
|
|
I am seeing some bizarre A/D conversion behavior. My current project is moving a stepper to a position specified as a percentage. The motor turns one way until it hits a limit, then turns the other. It then calculates where it needs to move based on an analog in representing a percentage.
On MCLR it will initialize as described above. On power on it will read stored initialization values (found on the last MCLR reboot) from the EEPROM. What I have noticed is that on power on (not MCLR) the motor first moves to an incorrect position then corrects itself to the expected position based on the given input. The incorrect position is always the same using the same version of my code (i.e. it will always move to 30% and then correct itself to the input position). However if I change the delay in my a/d conversion function, then the incorrect position changes to something else. It will remain there until I change the delay again.
Here is the A/D conversion function:
Code: |
//analog_in
//no params
//RETURN: the converted analog value
//reads the analog signal on channel 0 and returns the value
unsigned long analog_in(void){
extern unsigned long result;
result = 0;
ADCON0 = 0b00000001; //enable with channel 0
delay_us(100);
ADGO = 1; //start conversion
while(ADGO){continue;} //wait for conversion to finish
result = (ADRESH<<8);
result |= ADRESL;
return result;
}
|
what could cause this weird behavior? It always corrects itself to the right position after moving to the incorrect position first.
I only see this odd behavior when I am NOT debugging. If I am debugging, everything works exactly as expected. The only difference between debugging and not debugging that I am aware of is time. This reflects the same behavior that I mentioned before about adding delays.
I am using MPLab IDE 8.63 and CCS C compiler 4.114. I'm using identical code that has worked in the past which is what makes this so weird.
Thanks in advance for your help! |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19513
|
|
Posted: Wed Jan 12, 2011 9:42 am |
|
|
First, use the standard functions. You have bought a compiler, let it do it's job...
Code: |
int16 analog_in(void) {
int16 result;
set_adc_channel(0);
delay_us(20); //You do not need 100uSec.....
result=read_adc();
return result;
}
|
So long as you have #device ADC=10,, this will return the value you require.
Now, on your declaration, are you linking this in multiple compilation units?. If not, why use the extern declaration. Even if you are using multiple compilation units, it is 'odd' programming, to use an external variable to contain a value that is then returned. Why bother?. Let the variable be local, and have the function return it....
Now, you don't show how your code uses this. Do you perform any form of initialisation of position before the value is read?. Where do you configure the analog multiplexer?. What ADC clock are you using?. What channels have you got set as analog?. What is your clock rate?.
Then, really better to select the ADC channel once. Unless you are using multiple channels, just select it during initialisation, at the same time as you setup the clock rate, and channels. Then assuming it has been some uSec since you last read the input, you don't need any delays, just read the value.
Best Wishes |
|
|
skoberlink
Joined: 27 May 2010 Posts: 52
|
|
Posted: Wed Jan 12, 2011 10:13 am |
|
|
Ttelmah wrote: |
So long as you have #device ADC=10,, this will return the value you require.
|
I have done this. The result seems to be getting returned correctly as the motor corrects itself. What appears to be happening is the conversion is wrong the first time and then is fine after that.
Ttelmah wrote: |
Now, on your declaration, are you linking this in multiple compilation units?. If not, why use the extern declaration. Even if you are using multiple compilation units, it is 'odd' programming, to use an external variable to contain a value that is then returned. Why bother?. Let the variable be local, and have the function return it....
|
I also agree that this is odd programming practice. It was being used as a global variable because For some reason, I can't get locals to show up in the MPLab watch windows for this project. So I temporarily had a few locals declared global so that I could see them. I just happened to miss this one. I fixed it with no effect on the behavior.
Ttelmah wrote: |
Now, you don't show how your code uses this. Do you perform any form of initialisation of position before the value is read?. Where do you configure the analog multiplexer?. What ADC clock are you using?. What channels have you got set as analog?. What is your clock rate?.
|
my initialization for the registers look like this:
Code: |
ADCON0 = 0b00000001; //enable with channel 0
ADCON1 = 0b00001110; //VSS/VDD as reference, only AN0 is analog
ADCON2 = 0b10010111;
|
As for motor initialization, on MCLR it will initialize the motor by finding limit switches. It moves one direction the max then moves to the min counting steps. This maximum steps variable is stored in EEPROM to be recalled later. Current step is then set to 0.
On Power On, the maximum and current position (which is updated in EEPROM each time the motor moves) are read from EEPROM and used in place of actual initialization.
Ttelmah wrote: |
Then, really better to select the ADC channel once. Unless you are using multiple channels, just select it during initialisation, at the same time as you setup the clock rate, and channels. Then assuming it has been some uSec since you last read the input, you don't need any delays, just read the value.
|
I have changed the code to reflect this as well as changing it to use the built-ins. Since I don't know exactly what the built-ins are doing i feel better if I write the code myself but since I am completely baffled by this problem I broke down and used them. I seem to still be having the same problem.
So thanks for the help, I hope I have addressed all of your ideas. Since the problem still exists, are there any other suggestions given this new info? |
|
|
temtronic
Joined: 01 Jul 2010 Posts: 9226 Location: Greensville,Ontario
|
|
Posted: Wed Jan 12, 2011 10:44 am |
|
|
Do you have 'PUT' enabled ?
Without it you might have a 'race condition' as the PIC and peripherals get 'sorted' out.
What happens if you put a small delay( say 1/4 second) in Main before the overall 'loop' runs?
Since it appears to be a repeatable problem, it should be able to find the incorrect or buggy setup....
Would need more info to help further. Headers,includes,init routines,etc.(basically the first 20-30 lines of code) |
|
|
skoberlink
Joined: 27 May 2010 Posts: 52
|
|
Posted: Wed Jan 12, 2011 10:51 am |
|
|
I have the #fuse NOPUT which I understood to mean that the power up timer is disabled. I thought of that as well and checked it but I may be incorrect. I have trouble with the fuses since there doesn't seem to be a nice list defining them.
Here is my register initialization code:
Code: |
void PIC_init(void){
/*ADCON0 = 0b00000001; //enable with channel 0
ADCON1 = 0b00001110; //VSS/VDD as reference, only AN0 is analog
ADCON2 = 0b10010111;*/
setup_adc(ADC_CLOCK_INTERNAL);
setup_adc_ports(AN0,VSS_VDD);
set_adc_channel(0);
TRISA = 0b11111111; //All inputs
PORTA = 0; //clear PORTA
TRISB = 0b00011111; //B7:5 out, B4:0 in
PORTB = 0; //clear PORTB
TRISC = 0b10000010; //B7,1 in, B6:2,0 out
PORTC = 0; //clear PORTC
}
|
And this is the code for initialization on power up (not MCLR):
Code: |
if(!POR){
POR=1;
max_steps = 0;
curr_step = 0;
max_steps = EEPROM_read32(0);
curr_step = EEPROM_read32(4);
}
|
let me know if there's something more you need. I really appreciate the help guys. |
|
|
temtronic
Joined: 01 Jul 2010 Posts: 9226 Location: Greensville,Ontario
|
|
Posted: Wed Jan 12, 2011 11:51 am |
|
|
I'd....
enable PUT as it allows the PIC to 'get going right'.
trash the TRIS statements, let the compiler handle it...
same holds true for the ADCON=....,again use the compiler commands...
might..be a compiler version problem, but seeing the whole program , or a small one that fails would help. Seeing only fragments it's hard to see the 'big picture'.
reread the OP, maybe go back to the last known working version,make a change, save as version x+1, try it,if it works, keep making small changes,save as newer version, try it.... sooner or later you'll stumble onto the code that makes it 'go weird'.... |
|
|
skoberlink
Joined: 27 May 2010 Posts: 52
|
|
Posted: Wed Jan 12, 2011 12:04 pm |
|
|
temtronic wrote: | I'd....
enable PUT as it allows the PIC to 'get going right'.
trash the TRIS statements, let the compiler handle it...
same holds true for the ADCON=....,again use the compiler commands...
might..be a compiler version problem, but seeing the whole program , or a small one that fails would help. Seeing only fragments it's hard to see the 'big picture'.
reread the OP, maybe go back to the last known working version,make a change, save as version x+1, try it,if it works, keep making small changes,save as newer version, try it.... sooner or later you'll stumble onto the code that makes it 'go weird'.... |
Should I still enable PUT if I'm using the internal clock?
Also how can I let the compiler handle the TRIS statements? I've looked through the manual and can't find a function that does it and I thought something has to set that. |
|
|
temtronic
Joined: 01 Jul 2010 Posts: 9226 Location: Greensville,Ontario
|
|
Posted: Wed Jan 12, 2011 2:29 pm |
|
|
Yes, good practice says always use PUT....
Just get rid of all TRIS statements, the compiler defaults to
#use standard_IO. It's not common to use TRIS statements unless you're trying to cut code space or increase speed.
It is in the manual...page 139 of the aug 2009 version...that's about 1/2 way through the manual. |
|
|
skoberlink
Joined: 27 May 2010 Posts: 52
|
|
Posted: Wed Jan 12, 2011 3:00 pm |
|
|
Ok I added PUT (and clarified my understanding and I get why it should be used now). The percent it tried to move to changed. This is the same thing that I always see when I change the delay. To me this seems like something that I should have done anyway but it hasn't fixed the problem.
I also tried removing the TRIS statements and the motor didn't turn at all. I wasn't getting output where I should have been. This isn't a big deal to me just wondered if you had any ideas why.
The major issue here is still whatever is wrong with my AD conversion that takes it so long to settle.
So I really appreciate the help, I've made a lot more progress today than I have in the rest of this week. Thanks you guys. But have you got any more ideas? |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19513
|
|
Posted: Wed Jan 12, 2011 3:44 pm |
|
|
I have to say 'ouch', to storing the motor position every time it moves. If you are updating the position at all rapidly, this could kill the EEPROM quite quickly.
Either save the position _only_ if the motor stays still for a reasonable time, or save the positions when the power goes off (do some searching here about how to detect this...).
Are you selecting 'fast_io' somewhere?. If not, removing the TRIS statements, should leave the motor still moving. Suggests you may be doing this...
I doubt if the problem is A/D conversion. However, what is feeding the A/D with this voltage?. If the impedance of this circuit is _much_ too high, then the internal ADC capacitor _will_ take significant time to charge (and the results will be poor). This could explain your problems. Ideally post a link to the circuit that is driving the ADC.
As a comment still, 'extern' is not needed to make a variable global. Just declare it outside the code. 'Extern', says that the variable is declared in another compilation unit, and declaring variabels like this, when they are not, will result in screwy behaviour....
Best Wishes |
|
|
skoberlink
Joined: 27 May 2010 Posts: 52
|
|
Posted: Wed Jan 12, 2011 3:58 pm |
|
|
Interesting. I had not considered saving to EEPROM only on power down. I will look into this as that would be much better.
I also misunderstood what extern meant then. I will change my code to reflect this.
I have not selected fast_io anywhere that I am aware of. Is that shared with any other bit or something? Or could it have been selected in a built-in function without my knowledge?
We were beginning to wonder if this was a hardware issue instead of a software issue. The A/D voltage is coming from a 4-20 mA which is converted to 0-5 volts. I don't have the impedance specs or the circuit design with me right now but as soon as I have the answer I can let you know. Your description of the cause fits the symptoms. In the meantime, what would your fix involve assuming this is the problem? |
|
|
temtronic
Joined: 01 Jul 2010 Posts: 9226 Location: Greensville,Ontario
|
|
Posted: Wed Jan 12, 2011 6:01 pm |
|
|
5v/20ma is 250 ohms...
However you have to watch out for ground loops and offsets if the 0-5v output is not isolated from the 4-20ma device.
That all depends on what the device is, who made it, etc. The documentation for the device should explain everything in detail, if not, contact the mfr. for more info. |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19513
|
|
Posted: Thu Jan 13, 2011 3:21 am |
|
|
Also, and obvious, if this is something like an isolated 4-20 to 0-5v converter, it may take significant time to wake up. This then would explain why the behaviour is 'time dependant'....
Best Wishes |
|
|
skoberlink
Joined: 27 May 2010 Posts: 52
|
|
Posted: Thu Jan 13, 2011 1:56 pm |
|
|
Well I am told the impedance is acceptable. Also I discovered something new that points to that not being the problem. I put in a delay of around 20 seconds and it still did it. The cap should have been charged by then without question right?
Also I have been unable to track down how to know if I have lost power in time to write the eeprom. Could you point me in the right direction on that as well?
EDIT: I also just realized I forgot to mention something.
I am currently using a pic18F2620. Virtually identical code was used without this problem on a pic16F88. The pic18 code was written from scratch but performs the same function and is based on the same control flow as the pic16. Is there anything there that could point to a solution? |
|
|
|
|
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
|