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

ADC erratic readings
Goto page 1, 2, 3  Next
 
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion
View previous topic :: View next topic  
Author Message
murgui



Joined: 23 Dec 2015
Posts: 37

View user's profile Send private message

ADC erratic readings
PostPosted: Mon Jul 25, 2016 10:07 am     Reply with quote

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: 19195

View user's profile Send private message

PostPosted: Mon Jul 25, 2016 10:53 am     Reply with quote

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

View user's profile Send private message

PostPosted: Mon Jul 25, 2016 11:18 am     Reply with quote

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

View user's profile Send private message

PostPosted: Mon Jul 25, 2016 12:39 pm     Reply with quote

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: 9081
Location: Greensville,Ontario

View user's profile Send private message

PostPosted: Mon Jul 25, 2016 5:49 pm     Reply with quote

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

View user's profile Send private message

PostPosted: Mon Jul 25, 2016 6:01 pm     Reply with quote

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

View user's profile Send private message

PostPosted: Tue Jul 26, 2016 3:13 am     Reply with quote

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: 19195

View user's profile Send private message

PostPosted: Tue Jul 26, 2016 4:11 am     Reply with quote

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: 9081
Location: Greensville,Ontario

View user's profile Send private message

PostPosted: Tue Jul 26, 2016 5:12 am     Reply with quote

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

View user's profile Send private message

PostPosted: Wed Jul 27, 2016 5:57 am     Reply with quote

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: 19195

View user's profile Send private message

PostPosted: Wed Jul 27, 2016 7:14 am     Reply with quote

If you handle the I2c properly, the problem won't exist. You are trying to use it incorrectly.
Ttelmah



Joined: 11 Mar 2010
Posts: 19195

View user's profile Send private message

PostPosted: Wed Jul 27, 2016 7:45 am     Reply with quote

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

View user's profile Send private message

PostPosted: Fri Jul 29, 2016 9:29 am     Reply with quote

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: 9081
Location: Greensville,Ontario

View user's profile Send private message

PostPosted: Fri Jul 29, 2016 11:18 am     Reply with quote

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

View user's profile Send private message

PostPosted: Fri Jul 29, 2016 11:37 am     Reply with quote

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.
Display posts from previous:   
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion All times are GMT - 6 Hours
Goto page 1, 2, 3  Next
Page 1 of 3

 
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