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 CCS Technical Support

adc problem moving from 12f683 to 16f18313

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



Joined: 25 Jan 2022
Posts: 4

View user's profile Send private message

adc problem moving from 12f683 to 16f18313
PostPosted: Tue Jan 25, 2022 3:32 pm     Reply with quote

Hello,

I'm trying to get some old code that has been working on the PIC12F683 moved over to the newer and more available 8 pin PIC16F18313.

I've stripped it down to demonstrate the problem I'm having reading a pot on pin 7 and saving the value by writing to EEPROM.

Here is the code that works as expected on the 12F, with CCS version 3.245:
Code:


#include <12f683.h>
#FUSES INTRC_IO      //Internal RC Osc
#FUSES NOWDT      //No watch dog timer
#FUSES NOPUT      //No power up timer
#FUSES NOMCLR      //Master clear pin used for I/O
#FUSES NOPROTECT  //code not protected from reading
#FUSES NOCPD      //No EE protection TIMM
#FUSES NOBROWNOUT      //No brownout reset
#FUSES NOIESO      //Internal external switch over mode disabled
#FUSES NOFCMEN      //Fail-safe clock monitor disabled

#CASE
#DEVICE ADC=10
#use delay(clock=8000000) // using 8 MHz internal clock
#use fast_io(A)

#byte GPIO = 0x00C

#byte ADCON0 = 0x1F
#byte TRISIO = 0x85

#bit HALL = GPIO.0   // pin 7

typedef unsigned long int   uns16;

uns16 avg_adc()
{
   int32 sum;
   int count;
   
   for (count = 0; count < 8; count++)
   {
      delay_us(20);
      sum += read_adc();   //add the reading to the sum
   }
   sum = (sum >> 3);      //average

   return sum;
}

void main(void)
{
   #ZERO_RAM

   char e_data0;
   char e_data1;
   int16 pot_reading;

   GPIO = 0;

   ADCON0 = 0b01000001;
   TRISIO = 0b00001001;

   setup_port_a(sAN0); // pot is on pin 7

   setup_adc(ADC_CLOCK_DIV_2);

   delay_ms(2000);

   set_adc_channel(0);
   delay_us(20);

   pot_reading = avg_adc();
   e_data0 = (char)((pot_reading >> 8) & 0x00ff);
   e_data1 = (char)(pot_reading & 0x00ff);

   write_eeprom(0x00, e_data0);
   write_eeprom(0x01, e_data1);

}  // main


As expected, when I read back the EEPROM (0x00,0x01) I see a valid number around 512 as the pot is sitting near 2.5v.

When I swap in my 16F18313 chip, I'm always seeing zeros in those eeprom locations.

Any idea what I'm overlooking here? Thanks so much.

Here is my modified code for the 16F18313, using CCS demo compiler v5:

Code:

#include <16f18313.h>
#FUSES RSTOSC_HFINTRC_1MHZ
#FUSES NOWDT      //No watch dog timer
#FUSES NOPUT       //No power up timer
#FUSES NOMCLR      //Master clear pin used for I/O
#FUSES NOPROTECT    //code not protected from reading
#FUSES NOCPD       //No EE protection TIMM
#FUSES NOBROWNOUT   //No brownout reset
#FUSES NOFCMEN      //Fail-safe clock monitor disabled

#CASE
#DEVICE ADC=10
#use delay(clock=8mhz) // using 8 MHz internal clock
#use fast_io(A)

#byte GPIO = 0x00C

#byte ADCON0 = 0x09D
#byte TRISIO = 0x08C

#bit HALL = GPIO.0   // pin 7

typedef unsigned long int   uns16;

uns16 avg_adc()
{
   int32 sum;
   int count;
   
   for (count = 0; count < 8; count++)
   {
      delay_us(20);
      sum += read_adc();   //add the reading to the sum
   }
   sum = (sum >> 3);      //average

   return sum;
}

void main(void)
{
   #ZERO_RAM

   char e_data0;
   char e_data1;
   int16 pot_reading;

   GPIO = 0;

   ADCON0 = 0b01000001;
   TRISIO = 0b00001001;

   setup_port_a(sAN0); // pot is on pin 7

   setup_adc(ADC_CLOCK_DIV_2);

   delay_ms(2000);
   
   set_adc_channel(0);
   delay_us(20);

   pot_reading = avg_adc();
   e_data0 = (char)((pot_reading >> 8) & 0x00ff);
   e_data1 = (char)(pot_reading & 0x00ff);

   write_eeprom(0x00, e_data0);
   write_eeprom(0x01, e_data1);

}  // main

temtronic



Joined: 01 Jul 2010
Posts: 9221
Location: Greensville,Ontario

View user's profile Send private message

PostPosted: Tue Jan 25, 2022 4:14 pm     Reply with quote

the new PIC has PPS......
might want to confirm those settings...
also the SFRs are probably different ??
jeremiah



Joined: 20 Jul 2010
Posts: 1345

View user's profile Send private message

PostPosted: Tue Jan 25, 2022 7:14 pm     Reply with quote

is this actually correct?

Code:

setup_port_a(sAN0); // pot is on pin 7


I thought the function was setup_adc_ports()
Ttelmah



Joined: 11 Mar 2010
Posts: 19496

View user's profile Send private message

PostPosted: Wed Jan 26, 2022 2:45 am     Reply with quote

Yes Jeremiah, the setup_port_a syntax is an 'old' (antique!) syntax, which is
technically still supported, but has probably not been tested on recent
compilers at all. Shouldn't really be used. This is not in the current
manuals. Use setup_adc_ports instead. A look at the generated code
suggests it probably does work (on the latest compilers at least), but
sticking to standard syntax is much safer.

The correct syntax for an 8MHz internal clock, is just to use

#use delay(internal=8MHz)

This chip has the internal oscillator selectable to 1Mhz or 32MHz in the fuses
all other speeds need to have the divisor changed after boot. Using the
internal= syntax, the the compiler will add the code for this, and also set
the fuse to boot on the 1MHz oscillator.

Then what is probably causing the problem. The write to ADCON0. Get
rid of this. Problem here is the byte being sent actually starts a conversion.
You can't change the ADC configuration registers while a conversion is
being performed. So the subsequent setup_adc probably will not happen.
ADCON0 is automatically set by the compiler code. Don't fiddle....

There are then some warnings. As Jay says this is a PPS chip, so
PIN_SELECT will need to be used before using peripherals. Separately
though there is the use of GPIO. Beware. The old chip just has PORTA,
which this is mapped to. The newer chip has a separate LAT register
On chips with LAT, reads can be from the port register, but writes should
be to the LAT register. You currently don't show a write, but if the code
does this, then use LAT.

Generally, don't code registers using hand typed values. If you must use
registers (there is nothing here that needs to do this), then use register
names:

#byte ADCON0 = getenv("SFR:ADCON0")

etc..

This is vastly safer, especially when you do come to changing chips.

Then if running at 8MHz, the ADC divider needs to be DIV_8, not DIV_2.

Vastly more efficient in the code to use make8, rather than the rotations
and masking shown.

How are you testing this?.
As shown, a write takes place to the EEPROM every time the chip is
switched on. Presumably you are then switching off, and reading the
chip with your programmer to see what is in the EEPROM?.
If this is what you are doing, what programmer?.
The reason I'm querying this is the programmer access to the EEPROM
is very different between these chips. The older chip supports separate
commands to access the data memory. The newer chip does not. Instead
the EEPROM appears in the program memory 'space' at address 0xF000
and is read with the standard flash commands. Wonder if your programmer
is not actually handling this correctly....

As a further comment, the adc averaging function as written will work
OK _once_. Problem is if it is called a second time, 'sum' will probably
be in the same place in memory, and is not cleared.

Use:
int32 sum=0;

then it'll work if called again.

You don't actually need int32 for this. The ADC read returns 1023 max.
Eight of these can be summed fine in an int16 (sum = 8184 max). Will
make this function smaller and faster.
swanba



Joined: 25 Jan 2022
Posts: 4

View user's profile Send private message

PostPosted: Wed Jan 26, 2022 10:55 am     Reply with quote

Thanks so much for your insights. I inherited code that has been working on the PIC12F683 for many years but we ran into a major chip supply issue.

This is part of the elevation controller for an antenna system that boots up and needs to 'find itself' with an initial reading and then go through a lookup table of pre-calibrated EEPROM values to compare with and convert to a step motor count for running its elevation. Due to the fact that production workers don't always orient the elevation pot exactly the same, and also due to its mechanical design, its elevation range is not necessarily centered with the pot, and also not perfectly linear with step count/elevation angle. Hence the need for an EEPROM lookup table over the elevation range to help get it 'close enough'.

The very first time the system is freshly programmed and powered on, the code checks if its EEPROM is blank, and if so, it assumes the production operator has set elevation to zero as a calibration starting point. It then steps through its full range, saving adc readings to EEPROM that corresponds to its known calibration step count.

Forever more, when it boots up (after seeing the EEPROM table is already established) does an adc read at its bootup position to figure out its current elevation from the EEPROM lookup table.

I'm testing this on the full system, and it actually steps through its calibration while filling the EEPROM correctly. I remove the chip and read back EEPROM using MPLABX and a PICkit 3. It shows a filled EEPROM table of varying values. So I know the adc read *somehow* is working during this calibration loop.

However, the code that runs when it determines the calibration has already been done, which consists of one adc read averaging, always seems to fail. I convinced myself of this by adding a single adc reading (after calibration loop) and writing to a separate test EEPROM address - using the exact same read_adc() averaging routine used in the loop. I pull the chip and read it back using MPLABX/PICkit3 and see a full calibration table of values, and my separate EEPROM test address bytes are always zeros.

Ultimately, I removed nearly everything and tested the code I previously posted above to try and be as simple as possible to confirm the differences between the chips. I just powered it on (in the full system at mid-elevation) to run the code with the single read & EEPROM write, pull the chip, and check. I have a separate machine running old MPLAB/CCS v3.245/PICKit3 for the PIC12F683.

I realize there's some setup I need to figure out, as we've never used any recent chip/compiler. I was just confused as to how this adc read somehow works in the calibration table building part of the code, but not in the simplified code posted above.

I'm in the process of testing the fixes/changes you've mentioned and I really appreciate your suggestions! Thanks.
Ttelmah



Joined: 11 Mar 2010
Posts: 19496

View user's profile Send private message

PostPosted: Wed Jan 26, 2022 11:37 am     Reply with quote

OK. I rewrote it using the standard CCS functions and usage:
Code:

#include <16f18313.h>

#FUSES NOWDT      //No watch dog timer
#FUSES PUT       //Enable PUT - always needed if supply does not wake smoothly
#FUSES NOMCLR      //Master clear pin used for I/O
#FUSES NOPROTECT    //code not protected from reading
#FUSES NOCPD       //No EE protection TIMM
#FUSES BROWNOUT   //Enable brownout reset
//It is always much safer to run with brownout enabled.
#FUSES BORV24   //set to 2.4v - minimum 2.3v for EEPROM write
#FUSES NOFCMEN      //Fail-safe clock monitor disabled

#CASE
#DEVICE ADC=10
#use delay(internal=8mhz) // using 8 MHz internal clock
#use fast_io(A)

#define HALL PIN_A0
#ZERO_RAM
   
typedef unsigned long int   uns16;

uns16 avg_adc()
{
   int16 sum=0; //This is needed or sum will not be cleared
   int count;
   
   for (count = 0; count < 8; count++)
   {
      delay_us(5); //Tacq is just under 5uSec
      sum += read_adc();   //add the reading to the sum
   }
   return sum/8; //This is smaller than performing the divison then returning.
   //Also compiler 'knows' to perform /8 by just using shifts.''
}

void main(void)
{
   BYTE e_data0; //char is dangerous. On a lot of compilers (including PCD), char
   //is a signed type
   BYTE e_data1;
   int16 pot_reading;

   output_a(0); //clear LAT register
   set_tris_a(0b00001001); //A0 and A4 as inputs

   setup_adc_ports(sAN0, VSS_VDD); // pot is on pin 7 and supply as ref
   //This is slightly worrying, since this is the same pin you have named as
   //'HALL'....
   //This also sets ANSEL
   
   setup_adc(ADC_CLOCK_DIV_8); ///8 divisior for 8MHz
   set_adc_channel(0);
   
   delay_ms(2000);

   pot_reading = avg_adc();
   //Do you mean to store the MSB in the low byte in the EEPROM. Seems odd.
   e_data0 = make8(pot_reading, 1);
   e_data1 = make8(pot_reading, 0);

   write_eeprom(0x00, e_data0);
   write_eeprom(0x01, e_data1);

}  // main

The most likely thing causing erratic ADC values, is simply that you are
clocking it 4* faster than it's maximum rating.... Sad
swanba



Joined: 25 Jan 2022
Posts: 4

View user's profile Send private message

PostPosted: Wed Jan 26, 2022 7:03 pm     Reply with quote

After trying some updates with best practice CCS usage and not getting anywhere, I compiled the code rewrite above and tried it out. Thank you for that.

Unfortunately, when I run it and read back the EEPROM, I'm see zero values written at 0x00 and 0x01 as before. Replacing with the PIC12F683 (running the code I posted previously) writes EEPROM values consistent with the pot position so I'm at a loss for what's happening with the PIC16F18313.

I've been using the currently downloadable CCS demo compiler for my testing (which shows "version 5.0.0.0" I believe), is it likely that purchasing the latest compiler version may help?

Thanks.
PCM programmer



Joined: 06 Sep 2003
Posts: 21708

View user's profile Send private message

PostPosted: Wed Jan 26, 2022 10:22 pm     Reply with quote

That's not the compiler version. I suspect you're doing a right-click
on the icon and looking at properties.

Look at the top of the .LST file for your project. It will list the version.

Your problem is with the eeprom. Why bother with the ADC ?
Get the eeprom working. Write a test program that writes to the
eeprom.

How are you looking at the eeprom values ? I don't see any rs232
output. Your code is running off the end of main(). Is that really how
you're doing it, or is there other code that is not shown ?
Ttelmah



Joined: 11 Mar 2010
Posts: 19496

View user's profile Send private message

PostPosted: Thu Jan 27, 2022 3:12 am     Reply with quote

Yes
I asked in my long post what programmer he was using. He has since said
Pickit 3. If you go into the Pickit 3 help in MPLAB, and use the DTS, to see
if this chip is supported, it is not in the list of tools that Microchip say
will work.
I suspect it cannot read the EEPROM correctly from the chip. Can program
it, but not read this.
This shows sending the data out on a serial on A5, before stopping the
CPU:
Code:

#include <16f18313.h>

#FUSES NOWDT      //No watch dog timer
#FUSES PUT       //Enable PUT - always needed if supply does not wake smoothly
#FUSES NOMCLR      //Master clear pin used for I/O
#FUSES NOPROTECT    //code not protected from reading
#FUSES NOCPD       //No EE protection TIMM
#FUSES BROWNOUT   //Enable brownout reset
//It is always much safer to run with brownout enabled.
#FUSES BORV24   //set to 2.4v - minimum 2.3v for EEPROM write
#FUSES NOFCMEN      //Fail-safe clock monitor disabled

#CASE
#DEVICE ADC=10
#use delay(internal=8mhz) // using 8 MHz internal clock
#use fast_io(A)
#PIN_SELECT U1TX=PIN_A5
#PIN_SELECT U1RX=PIN_A5
//both to same pin so only one pin needed
#use rs232 (UART1, STREAM=SERIAL, BAUD=9600, ERRORS)

#define HALL PIN_A0
#ZERO_RAM
   
typedef unsigned long int   uns16;

uns16 avg_adc()
{
   int16 sum=0;
   int count;
   
   for (count = 0; count < 8; count++)
   {
      delay_us(5); //Tacq is just under 5uSec
      sum += read_adc();   //add the reading to the sum
   }
   return sum/8; //This is smaller than performing the division then returning.
   //Also compiler 'knows' to perform /8 by just using shifts.''
}

void main(void)
{
   BYTE e_data0; //char is dangerous. On a lot of compilers (including PCD), char
   //is a signed type
   BYTE e_data1;
   int16 pot_reading;

   output_a(0); //clear LAT register
   set_tris_a(0b00001001); //A0 and A3 as inputs

   setup_adc_ports(sAN0, VSS_VDD); // pot is on pin 7 and supply as ref
   //This is slightly worrying, since this is the same pin you have named as
   //'HALL'....
   //This also sets ANSEL
   
   setup_adc(ADC_CLOCK_DIV_8); ///8 divisior for 8MHz
   set_adc_channel(0);
   
   delay_ms(2000);

   pot_reading = avg_adc();
   //Do you mean to store the MSB in the low byte in the EEPROM. Seemds odd.
   e_data0 = make8(pot_reading, 1);
   e_data1 = make8(pot_reading, 0);

   write_eeprom(0x00, e_data0);
   write_eeprom(0x01, e_data1);
   //Now read this back and send to the serial
   
   e_data0=read_eeprom(0);
   e_data1=read_eeprom(1);
   
   fprintf(SERIAL, "LSB=%d, MSB=%d\n\r",e_data1, e_data0);
   //Pause to send before stopping the CPU. 2mSec minimum needed
   delay_ms(10);
}  // main


In the MPLAB simulator, this merrily reads back the value sent as the
adc value from the EEPROM.
swanba



Joined: 25 Jan 2022
Posts: 4

View user's profile Send private message

PostPosted: Thu Jan 27, 2022 10:52 am     Reply with quote

The .LST file shows "Version 5.107Pd"

I suspected EEPROM writes at first, before posting here. But I have had no problem writing values set explicitly, pulling the chip, and reading them back. I'm using a PICkit 3 to do this.

If I just test write_eeprom like:

Code:

for(cnt=0;cnt<10;cnt++)
{
  write_eeprom(cnt,cnt);
  delay_ms(100);
}

I read back and see the ten sequential incrementing values as expected. If I try as shown previously (with a value returned from the read_adc function) it's always '00' in EEPROM.

My adc is reading a pot in the 1v-4v range and replacing with a PIC12F683 chip running an equivalent test, I'm seeing all the expected values in EEPROM.
I've isolated this issue while trying to migrate a much larger program that has been working on the PIC12F683.

I did order a PICKit 4, so I will try that.

Thanks.
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