 |
 |
| View previous topic :: View next topic |
| Author |
Message |
murgui
Joined: 23 Dec 2015
Posts: 37
|
| ADC erratic readings |
 Posted: Mon Jul 25, 2016 10:07 am |
 |
|
Hi,
I'm trying to read the voltage of a voltage divider's NTC. I'm sending the read variables with a USB<-->TTL to the PC by RS232. I read 6 values and I can't understand the values received. I use the compiler version 4.114.
The code:
Slave.h
| Code: | #include <16F1826.h>
#FUSES NOWDT
//No Watch Dog Timer
#FUSES INTRC_IO //Internal RC Osc, no CLKOUT
#FUSES WDT_SW
#FUSES NOPUT //No Power Up Timer
#FUSES MCLR //Master Clear pin enabled
#FUSES NOPROTECT //Code not protected from reading
#FUSES NOCPD //No EE protection
#FUSES NOBROWNOUT //No brownout reset
#FUSES NOCLKOUT
#FUSES IESO //Internal External Switch Over mode enabled
#FUSES FCMEN //Fail-safe clock monitor enabled
#FUSES NOWRT //Program memory not write protected
#FUSES PLL_SW
#FUSES STVREN //Stack full/underflow will cause reset
#FUSES BORV19
#FUSES NODEBUG //No Debug mode for ICD
#FUSES NOLVP //No low voltage prgming, B3(PIC16) or B5(PIC18) used for I/O
#device ADC=10
#use delay(int=16000000)
#define I2C_SDA PIN_B1
#define I2C_SCL PIN_B4
#use rs232(baud=9600,parity=N,xmit=PIN_B5,rcv=PIN_B2,bits=8,stream=PORT1,errors)
#use i2c(Slave,Fast,sda=PIN_B1,scl=PIN_B4,address=0xA0)
#define LED PIN_A1
#define DELAY 1000
|
Slave.c
| Code: | #include <Slave.h>
float T[6];
int16 mesura[6];
int8 f1[6],f2[6],a=0,i,q;
void main(){
setup_adc_ports(sAN2|sAN3|sAN4|sAN5|sAN6|sAN9|VSS_VDD);
setup_adc(ADC_CLOCK_DIV_32);//chequear esta [censored]
enable_interrupts (GLOBAL);
enable_interrupts (INT_SSP);
while(true){
for(i=2;i<10;i++){
if (i!=8 || i!=7){
a++;
set_adc_channel(i);
delay_us(20);
mesura[a]=read_adc();
T[a]=(mesura[a]/1024.0*5-13.971)/-0.0373;
/*f1[a]=floor(T[a]);//entero
f2[a]=floor(100*(T[a]-f1[a]));//decimal*/
}
a=0;
}
printf("Valor 1: \r");
printf("%Lu",mesura[0]);
printf("Valor 2: \r");
printf("%Lu",mesura[1]);
printf("Valor 3: \r");
printf("%Lu",mesura[2]);
printf("Valor 4: \r");
printf("%Lu",mesura[3]);
printf("Valor 5: \r");
printf("%Lu",mesura[4]);
printf("Valor 6: \r");
printf("%Lu",mesura[5]);
delay_ms(5000);
}
} |
The result, from Hercules Serial reader is:
[img]https://postimg.org/image/3vjdy4241/[/img]
As you can see, the variables become values higher than any possible with 10 bits. I suspect the problem may be related with the internal oscillator and the ADC acquisition times but I don't know how to proceed.
Thank you very much,
Murgui. |
|
 |
Ttelmah
Joined: 11 Mar 2010
Posts: 19515
|
|
| Posted: Mon Jul 25, 2016 10:53 am |
|
|
Key thing that will be causing enormous problems is having INT_SSP enabled without a handler.
The result of this could be completely erratic code behaviour.....
The second problem is you are writing out the end of the 'mesura' array.
You start with a=0. then increment it before you use it. The inner loop is executed six times, so you write to index 1, 2, 3, 4, 5, 6. The array maximum index is 5 (0...5 for six elements). So whatever is in memory immediately after this will get corrupted. Garbage....
Simplify. Use an array for the channel numbers.
const int8 channels[] = {2,3,4,5,6,9};
Then just do everything with a count from 0 to 5. |
|
|
murgui
Joined: 23 Dec 2015
Posts: 37
|
|
| Posted: Mon Jul 25, 2016 11:18 am |
|
|
Hi, thank you for your time.
I implemented your suggestions and the code right now is as follows.
Slave.c(slave.h unmodified)
| Code: | #include <Slave.h>
//#include <math.h>
float T[6];
int16 mesura[6],filtrado[6];
int8 f1[6],f2[6],a=0,i,q,v=0;
const int8 channels[] = {2,3,4,5,6,9};
void main(){
setup_adc_ports(sAN2|sAN3|sAN4|sAN5|sAN6|sAN9|VSS_VDD);
setup_adc(ADC_CLOCK_DIV_32);//chequear esto
enable_interrupts (GLOBAL);
//enable_interrupts (INT_SSP);
while(true){
for(i=0;i<6;i++){
set_adc_channel(channels[i]);
delay_us(20);
if (v=0){
filtrado[i]=read_adc()*7;
v=1;}
filtrado[i]+=read_adc();
mesura[i]=filtrado[i]/8;
filtrado[i]-=mesura[i];
T[i]=(mesura[i]/1024.0*5-13.971)/-0.0373;
/*f1[a]=floor(T[a]);//entero
f2[a]=floor(100*(T[a]-f1[a]));//decimal*/
}
printf("Valor 1: \r");
printf("%Lu",mesura[0]);
printf("Valor 2: \r");
printf("%Lu",mesura[1]);
printf("Valor 3: \r");
printf("%Lu",mesura[2]);
printf("Valor 4: \r");
printf("%Lu",mesura[3]);
printf("Valor 5: \r");
printf("%Lu",mesura[4]);
printf("Valor 6: \r");
printf("%Lu",mesura[5]);
delay_ms(5000);
}
} |
The result is different in terms of what values are being read but the apparent random numbers are still the same.
[img]https://postimg.org/image/hutq6bo9t/[/img]
I clarify that all voltages are exactly the same. How can some values be greater than 1023?
Thank you for the help. |
|
|
murgui
Joined: 23 Dec 2015
Posts: 37
|
|
| Posted: Mon Jul 25, 2016 12:39 pm |
|
|
| Without the filtering, the results were perfect. Now, I've been running the program for a few time and the values are getting near the real values. It seems the implemented filter has a slow transient. Also, the filter seems to get unstable after some time passed. |
|
|
temtronic
Joined: 01 Jul 2010
Posts: 9226
Location: Greensville,Ontario
|
|
| Posted: Mon Jul 25, 2016 5:49 pm |
|
|
One problem I see , if I read your code correctly, is that you only actually read a sensor twice
one time then multiply value * 7 then one time which you add to the first result.
IF the first reading is bad, it becomes 7 * BAD !
I'd rather you actually read the sensor 8 times. It doesn't really take very long and it reduces the chance of a bad 'raw' reading.
Since you're reading an NTC, I wouldn't think sensor reading time is too critical.
Also whenevr doing 'high level math', it's a good idea to printout actual raw readings, and computed numbers for every operation. That way you can SEE if it's a bad 'raw' reading or some 'funny' math problem( like missing braces).
Jay |
|
|
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
| Posted: Mon Jul 25, 2016 6:01 pm |
|
|
Look at your compiler warnings.
| Quote: | | >>> Warning 201 "pcm_test.c" Line 25(1,1): Assignment inside relational expression |
What's wrong with the if() statement shown in bold below:
| Quote: | if (v=0){
filtrado[i]=read_adc()*7;
v=1;} |
|
|
|
murgui
Joined: 23 Dec 2015
Posts: 37
|
|
| Posted: Tue Jul 26, 2016 3:13 am |
|
|
Thank you for all the support. You resulted really helpful. The program functions but I haven't managed to make the filter work. The values are unstable, at the beginning they are nonsense, passed 5 minutes they round a right value and finally it gets unstable again. I don't know what may be happening but I guess I will let the filter away.
Thank you for the help.
For anybody interested, the final code with working ADC channels' swap and rs232 communication is the following.
| Code: | #include <Slave.h>
//#include <math.h>
int16 T[6],mesura[6],filtrado[6];
int8 f1[6],f2[6],a=0,i,q,v=0;
const int8 channels[] = {2,3,4,5,6,9};
void main(){
setup_adc_ports(sAN2|sAN3|sAN4|sAN5|sAN6|sAN9|VSS_VDD);
setup_adc(ADC_CLOCK_DIV_32);//chequear esto
enable_interrupts (GLOBAL);
//enable_interrupts (INT_SSP);
while(true){
for(i=0;i<6;i++){
set_adc_channel(channels[i]);
delay_us(20);
/*if (v==0){
delay_ms(5);
filtrado[i]=read_adc();
filtrado[i]+=read_adc();
filtrado[i]+=read_adc();
filtrado[i]+=read_adc();
filtrado[i]+=read_adc();
filtrado[i]+=read_adc();
filtrado[i]+=read_adc();
v=1;}
filtrado[i]+=read_adc();
mesura[i]=filtrado[i]/8;
filtrado[i]-=mesura[i];*/
mesura[i]=read_adc();
T[i]=(((mesura[i]/1024.0*5-13.971)/-0.0373)-273.15)*100;
f1[i]=T[i]/100;
f2[i]=T[i]-f1[i]*100;
}
printf("---------------------\r");
printf("ADC 1: ");
printf("%Lu \r",mesura[0]);
printf("ADC 2: ");
printf("%Lu \r",mesura[1]);
printf("Temperatura 2: ");
printf("%Lu \r",T[1]);
printf("Mensajes 2: ");
printf("%u %u \r",f1[1],f2[1]);
printf("ADC 3: ");
printf("%Lu \r",mesura[2]);
printf("ADC 4: ");
printf("%Lu \r",mesura[3]);
printf("ADC 5: ");
printf("%Lu \r",mesura[4]);
printf("ADC 6: ");
printf("%Lu \r",mesura[5]);
printf("---------------------\r");
delay_ms(10000);
}
}
/*
#INT_SSP
void ssp_interrupt ()
{
int8 state;
for(q=0;q<7;q++){
state = i2c_isr_state();
//agafar bit R/W del address
while(state<0x80){//Master pidiendo info
state = i2c_isr_state();}
i2c_write(f1[q]);
state = i2c_isr_state();
while(state<0x81){//transmision completada y ack recibido
state = i2c_isr_state();}
i2c_write(f2[q]);
}
}
*/
|
|
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19515
|
|
| Posted: Tue Jul 26, 2016 4:11 am |
|
|
No.
Don't do that. Fix the problem first, or it'll be hidden away beneath the filter, waiting to bite you.
The obvious things to look at are the supply itself (generally, if you are using the PIC's supply to feed the reference to the ADC, you will never do better than perhaps 1%). The PIC generates noise, as does everything else you have attached to the supply. This is 'why' separate a separate Vref input is available. Then the ground between the PIC, and the analog source(s). In particular it's design, and the routing of signals. Remember one count of the ADC, using a 5v supply, is just 5mV....
Then 'look again' at your INT_SSP. Look at the examples. What you are doing is wrong. As it stands, the interrupt says _one_ transaction is taking place. You must not loop inside the interrupt. Doing so, means your main code will be hung, and if then something goes wrong with the I2C transactions, could be hung forever..... Each interrupt should handle just the one transaction. |
|
|
temtronic
Joined: 01 Jul 2010
Posts: 9226
Location: Greensville,Ontario
|
|
| Posted: Tue Jul 26, 2016 5:12 am |
|
|
OK, Is it just me ? I can't see where the variable 'V' is getting reset to 0, zero, so I'm not sure what the purpose of V is...
Also....
this code
T[i]=(((mesura[i]/1024.0*5-13.971)/-0.0373)-273.15)*100;
f1[i]=T[i]/100;
Can you not get rid of both the *100 in the first line and the /100 in the next line?
It'd save a LOT of PIC processing time so it'd be faster.
Jay |
|
|
murgui
Joined: 23 Dec 2015
Posts: 37
|
|
| Posted: Wed Jul 27, 2016 5:57 am |
|
|
Hi, answering Ttelmah, the unstabilities I talk about are not little variations but really big numbers, even higher to overflow the 10 bit limit of the adc. I dare to discard the error of the power supply. About the interruptions use, I get your point but I think that if the possibility of IIC getting stuck exists, it will also exist in the main program, am I wrong?
Answering Temtronic, the purpose of this is doing that part just one time to have enough information to start. Also, the product and division by 100 helps me separate the integer and decimal part in order to ease the information transmission by IIC.
Thank you very much,
Murgui. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19515
|
|
| Posted: Wed Jul 27, 2016 7:14 am |
|
|
| If you handle the I2c properly, the problem won't exist. You are trying to use it incorrectly. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19515
|
|
| Posted: Wed Jul 27, 2016 7:45 am |
|
|
First, forget the adc, put your code in a simple loop, counting from 0 to 1023, and display the results.
Your problem may be nothing to do with erratic results from the ADC, but be from erratic results on the serial, or even the RAM being corrupted by EMI.
The ADC cannot return a value above 1023, so you have something else wrong...
Then it is worth 'thinking' about your maths. It is a simple linear conversion, and can be done in integer, without getting involved in float at all.
| Code: |
//your header stuff
#include <stdlib.h>
//variables for the demo
signed int16 val;
ldiv_t parts;
//Then for just one channel
val=read_adc()*13;
val=10104-val;
//This then gives val=10140 for 0 on the ADC
//to -3195 for 1023 on the ADC. - as your existing maths, *100
printf("%6.2lw",val); //will display this as 101.40 to -31.95
parts=ldiv(val,100);
//gives parts.quot, and parts.rem holding the stuff before and after the decimal
|
|
|
|
murgui
Joined: 23 Dec 2015
Posts: 37
|
|
| Posted: Fri Jul 29, 2016 9:29 am |
|
|
Hi, following your advices I made the filter work. now I'm stuck with the communications. The Master appears to receive the information correctly but the information received is just not what it should be receiving. I looked the variables the slave should send and they look fantastic. I don't know if the slave is not sending what it should, the master is not reading as it should or what may be wrong. I tried to comment more explicitly everything, looking to other forum's fellows I realized it is necessary.
The variables sent by serial communication are always the same. Even when the data is supposed to change.
Master.c
| Code: | #include <Master.h>
#use i2c(Master,Fast,sda=PIN_C4,scl=PIN_C3)
#include <can-18xxx8.c>
#define LED PIN_B7
/*******************************************************
* ·IIC link: Check working
* ·
* ·CAN reading: Check working
* ·CAN writing:Check working
* Description: The system consists on a multislave single master
* where all 7 slaves gather temperature information and send it
* trough I2C to the master. The master sends all the information
* by CAN bus to an external system. The information consists on a
* 2 byte data pack where the first byte is the integer part of the
* temperature and the second byte the decimal part.
*******************************************************/
//Variable Declaration
int8 temp[7][6][2];
const int8 adr[7] = {0xA1,0xAB,0xB5,0xBF,0xC9,0xD3,0xDD};
int8 n,i,Dir,cont,subcontador,stack=0,cobid,lengthCAN;
int32 cid=0x320;//cobid base de temperatura
//Function Declaration
void read(int8 Dir);
void write();
void canrx0_int();
#int_canrx0
void canrx0_int ( ) {//Sincronism msg detection
cobid=((unsigned int16)RXB0SIDH << 3)|((RXB0SIDL & 0xE0)>>5);
lengthCAN = (unsigned int8)RXB0DLC & 0xF;
if (cobid == 0x80){
subcontador++;
if (subcontador==20){
cont=1;
subcontador=0;
}
}
}
void main(){
if(1==1){//CAN ini
can_init();
disable_interrupts(GLOBAL);
can_set_mode(CAN_OP_CONFIG);
BRGCON1.brp=1;
BRGCON1.sjw=1;
BRGCON2.prseg=2;
BRGCON2.seg1ph=7;
BRGCON2.sam=FALSE;
BRGCON2.seg2phts=FALSE;
BRGCON3.seg2ph=6;
BRGCON3.wakfil=FALSE;
can_set_mode(CAN_OP_NORMAL);
enable_interrupts(int_canrx0);
enable_interrupts(int_canrx1);
enable_interrupts(int_cantx0);
enable_interrupts(int_cantx1);
enable_interrupts(int_cantx2);
enable_interrupts(GLOBAL);}
while(1){
printf("hi:\r");//performed to show by serial the beginning of the task
//for (n=0;n<7;n++){
read(adr[0]);//working with just 1 slave
for (i=0;i<6;i++){
//printing all received DATA
printf("lectura %u :",i);
printf("%u %u\r",temp[0][i][0],temp[0][i][1]);
}
if (cont==1){//sincronism msg,still not used
cont=0;
write();}
delay_ms(500);
}
}
void read(int8 Dir){//I2C Enlace
i2c_start();
i2c_write(Dir);// slave direction & slave->Master(1)
for (i=0;i<5;i++){
temp[n][i][0]=i2c_read(1);//Integer read and ack
temp[n][i][1]=i2c_read(1);}//Decimal read
temp[n][6][0]=i2c_read(1);
temp[n][6][1]=i2c_read(0);//final value and NACK for finishing
i2c_stop();
}
void write(){//CAN writing
if (stack==7){
stack=0;}
can_putd((cid+stack*10),&temp[stack][0][0],8,1,FALSE,FALSE);//identificador,puntero de localizacion,longitud en bytes,prioridad,extended use, rtr(request->true) mensaje->false
can_putd((cid+stack*10+1),&temp[stack][1][2],4,1,FALSE,FALSE);
stack++;
} |
Slave.c
| Code: | #include <Slave.h>
#use i2c(Slave,Fast,sda=PIN_B1,scl=PIN_B4,address=0xA0)
//Variable declaration
int16 T[6],mesura[6],sum[6];
int8 f1[6],f2[6],a=0,i,q,z,v[6]={0,0,0,0,0,0};
const int8 channels[] = {2,3,4,5,6,9};
//function declaration
void i2c_interrupt_handler();
#INT_SSP
void ssp_interrupt ()
{
i2c_interrupt_handler();
}
void main(){
setup_adc_ports(sAN2|sAN3|sAN4|sAN5|sAN6|sAN9|VSS_VDD);
setup_adc(ADC_CLOCK_DIV_32);
while(true){
delay_ms(10);
//read of 6 ADC channels, for 6 NTC
for(i=0;i<6;i++){
set_adc_channel(channels[i]);
delay_us(40);
//The first time, it won't have enough information in order to let the filter
//work so it gathers 7 extra ADC readings only during the first program run
if (v[i]==0){
sum[i]=read_adc();
for (z=0;z<6;z++){
sum[i]+=read_adc();
}
v[i]=1;
enable_interrupts (GLOBAL);
enable_interrupts (INT_SSP);
}
sum[i]+=read_adc();
mesura[i]=sum[i]>>3;
sum[i]-=mesura[i];
//adapts the reading to a mathematic function to find th temperature
T[i]=(((mesura[i]/1024.0*5-13.971)/-0.0373)-273.15)*100;
f1[i]=T[i]/100;
f2[i]=T[i]-f1[i]*100;
printf("lectura %u :",i);
printf("%u %u \r",f1[i],f2[i]);//checking these values, everything works fine
}
delay_ms(5000);
}
}
//i2c interruption handler, sends the information gathered.
void i2c_interrupt_handler()
{
int8 state;
for(q=0;q<6;q++){
state = i2c_isr_state();
//agafar bit R/W del address
while(state<0x80){//Master asking info
state = i2c_isr_state();}
i2c_write(f1[q]);
state = i2c_isr_state();
while(state<0x81){//transmission completed and ack received
state = i2c_isr_state();}
i2c_write(f2[q]);
}
}
|
What may be causing the errors in communication?
Thank you very much for the help, any post I create helps me learn a lot. |
|
|
temtronic
Joined: 01 Jul 2010
Posts: 9226
Location: Greensville,Ontario
|
|
| Posted: Fri Jul 29, 2016 11:18 am |
|
|
quick look...
In the master, you've got 5 CAN ISRs enabled but only one 'handler', that I can see.
THAT is a huge 'nono'. Never, ever enable an ISR without the handler code for it as ALL sorts of weird, stange, unpredictable things WILL happen.
If your only problem is the CAN code to PC, then use 'dummy, known' data as transmit it to the PC get rid of ALL the other code, focus ONLY on the PIC to PC via CAN routines.
Also, your function names should better describe what they do.
say... write_via_CAN() as opposed to write(), read_via_I2C instead of read(). It makes the code 'self documenting' and 3 days or 3 months from now, you and others will KNOW what's supposed to happen.
Jay |
|
|
murgui
Joined: 23 Dec 2015
Posts: 37
|
|
| Posted: Fri Jul 29, 2016 11:37 am |
|
|
Hi, I will disable now all the unused CAN ISRs.
I probably explained myself wrong: the communication with the PC is simply to check if the received data from I2C is fine, and it isn't. I have still not tried the CAN communication. The PC protocol is RS232 with a USB to TTL adapter. |
|
|
|
|
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
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
