View previous topic :: View next topic |
Author |
Message |
swapnil
Joined: 15 Dec 2016 Posts: 13 Location: delhi
|
Code is working fine in proteus but on hardware lcd not work |
Posted: Thu Dec 15, 2016 9:52 am |
|
|
Code: | #include <16c72a.h>
#device adc=8
#fuses XT
#use delay (clock=4000000) // 4MHz clock
#use fast_io(c)
#define use_portb_lcd TRUE
#include <lcd.c>
#define MOSFET PIN_C3 // Charging LED Pin No.13
#define CHARGING_LED PIN_C4 // Charging LED Pin No.15
#define PCU_LOAD PIN_C5 // Low Battery LED Pin No.16
#define LOAD PIN_C6 // Load Pin No.17
#define LOW_BATT_LED PIN_C7 // Load Pin No.18
float mean_adc(byte channel) // Reads the adc port 30 times and gives the mean value
{
int i,mean_total = 30;
float mean = 0,mean1 = 0;
set_adc_channel(channel);
delay_us(100);
for (i=1; i<=mean_total; i++)
{
mean =mean + read_adc();
delay_us(100);
mean1=(mean/mean_total);
}
return(mean1);
}
//********** Main Program*******
void main ( )
{
const byte solar_voltage_channel=0;
const byte battery_voltage_channel=1;
float battery_voltage,bout,solar_voltage,sout;//,bin,sin;
set_tris_c(0b00000111);
setup_adc_ports(ALL_ANALOG); // setting up all port a as analog
setup_adc(adc_clock_div_32);
enable_interrupts(INT_AD);
enable_interrupts(GLOBAL);
lcd_init();
// delay_ms(1000)
while(TRUE)
{
solar_voltage=mean_adc(solar_voltage_channel);
sout=((solar_voltage*5)/255)*34;
battery_voltage=mean_adc(battery_voltage_channel);
bout=((battery_voltage*5)/255)*34;
//batt = vin;
printf(lcd_putc,"\fSOLAR =%2.1fV",sout);
printf(lcd_putc,"\nBATTERY =%2.1fV",bout);
//delay_us(1000);
if ( bout <= 115 )
{
//output_high(MOSFET);
output_high(MOSFET);
delay_ms(4000);
output_low(MOSFET);
delay_ms(4);
output_high(CHARGING_LED);
delay_ms(300);
output_low(CHARGING_LED);
delay_ms(300);
} else output_low(MOSFET);
if((sout > bout)&&(bout > 115.1))//&& (bout <= 116 ))
{
output_high(CHARGING_LED);
output_high(MOSFET);
delay_ms(3000);
output_low(MOSFET);
delay_ms(3000);
// printf(lcd_putc,"\fBATTERY CHARGING");
//printf(lcd_putc,"\n TRICKLE MODE ");
//delay_ms(2000);
}
/////////////////////////////LOAD ////LED INDICATION ///////////////////////////////////////////////////////
//if ((sin < 3 ) && (bin > 6.2))
if (bout >= 88)
{
output_high(LOAD);
}
if(bout <= 84.0)
{
output_high(LOW_BATT_LED);
output_low(LOAD);
}
else
output_low(LOW_BATT_LED);
// Mains SNS PCU Load On
if((input(PIN_C2)==1) && (sout > bout) && (bout > 116))
{
output_high(PCU_LOAD);
}
else if(bout < 104)
{
output_low(PCU_LOAD);
}
}
} |
|
|
![](templates/subSilver/images/spacer.gif) |
asmboy
Joined: 20 Nov 2007 Posts: 2128 Location: albany ny
|
|
Posted: Thu Dec 15, 2016 10:36 am |
|
|
show your circuit schematic.
consider using a 16F72 for development-
you only get one try with the 'C72
and
don't trust proteus
GET RID of the comments on the same line as the #DEFINE
only define text up to the carriage return for compat with older compilers
everywhere you use CONST BYTE ??
get rid of it and use the assigned constant VALUE in the program- thus getting fast at-compile-time code
instead of that CONST BYTE var assignment.
Code: |
#include <16c72a.h>
#device adc=8
#fuses XT
#use delay (clock=4000000) // 4MHz clock
#use fast_io(c)
#define use_portb_lcd TRUE
#include <lcd.c>
#define MOSFET PIN_C3 // Charging LED Pin No.13
#define CHARGING_LED PIN_C4 // Charging LED Pin No.15
#define PCU_LOAD PIN_C5 // Low Battery LED Pin No.16
#define LOAD PIN_C6 // Load Pin No.17
#define LOW_BATT_LED PIN_C7 // Load Pin No.18
float mean_adc(byte channel) // Reads the adc port 30 times and gives the mean value
{
int i,mean_total = 30;
float mean = 0,mean1 = 0;
set_adc_channel(channel);
delay_us(100);
for (i=1; i<=mean_total; i++)
{
mean =mean + read_adc();
delay_us(100);
mean1=(mean/mean_total);
}
return(mean1);
}
//********** Main Program*******
void main ( )
{
const byte solar_voltage_channel=0;
const byte battery_voltage_channel=1;
float battery_voltage,bout,solar_voltage,sout;//,bin,sin;
set_tris_c(0b00000111);
setup_adc_ports(ALL_ANALOG); // setting up all port a as analog
setup_adc(adc_clock_div_32);
enable_interrupts(INT_AD);
enable_interrupts(GLOBAL);
lcd_init();
// delay_ms(1000)
while(TRUE)
{
solar_voltage=mean_adc(solar_voltage_channel);
sout=((solar_voltage*5)/255)*34;
battery_voltage=mean_adc(battery_voltage_channel);
bout=((battery_voltage*5)/255)*34;
//batt = vin;
printf(lcd_putc,"\fSOLAR =%2.1fV",sout);
printf(lcd_putc,"\nBATTERY =%2.1fV",bout);
//delay_us(1000);
if ( bout <= 115 )
{
//output_high(MOSFET);
output_high(MOSFET);
delay_ms(4000);
output_low(MOSFET);
delay_ms(4);
output_high(CHARGING_LED);
delay_ms(300);
output_low(CHARGING_LED);
delay_ms(300);
} else output_low(MOSFET);
if((sout > bout)&&(bout > 115.1))//&& (bout <= 116 ))
{
output_high(CHARGING_LED);
output_high(MOSFET);
delay_ms(3000);
output_low(MOSFET);
delay_ms(3000);
// printf(lcd_putc,"\fBATTERY CHARGING");
//printf(lcd_putc,"\n TRICKLE MODE ");
//delay_ms(2000);
}
/////////////////////////////LOAD ////LED INDICATION ///////////////////////////////////////////////////////
//if ((sin < 3 ) && (bin > 6.2))
if (bout >= 88)
{
output_high(LOAD);
}
if(bout <= 84.0)
{
output_high(LOW_BATT_LED);
output_low(LOAD);
}
else
output_low(LOW_BATT_LED);
// Mains SNS PCU Load On
if((input(PIN_C2)==1) && (sout > bout) && (bout > 116))
{
output_high(PCU_LOAD);
}
else if(bout < 104)
{
output_low(PCU_LOAD);
}
}
} |
|
|
![](templates/subSilver/images/spacer.gif) |
PCM programmer
Joined: 06 Sep 2003 Posts: 21708
|
|
Posted: Thu Dec 15, 2016 12:46 pm |
|
|
Quote: | set_tris_c(0b00000111);
setup_adc_ports(ALL_ANALOG); // setting up all port a as analog
setup_adc(adc_clock_div_32);
enable_interrupts(INT_AD);
enable_interrupts(GLOBAL); |
Your program enables A/D interrupts, but has no interrupt handler for it.
This will make your program crash soon after you call read_adc().
You don't need A/D interrupts. Delete the line in bold above. |
|
![](templates/subSilver/images/spacer.gif) |
ckielstra
Joined: 18 Mar 2004 Posts: 3680 Location: The Netherlands
|
|
Posted: Sat Dec 17, 2016 8:53 am |
|
|
asmboy wrote: | GET RID of the comments on the same line as the #DEFINE
only define text up to the carriage return for compat with older compilers | I don't agree with this suggestion. Perhaps there once were buggy compilers, but that is something from a long time ago and not to bother new programmers with.
Quote: | everywhere you use CONST BYTE ??
get rid of it and use the assigned constant VALUE in the program- thus getting fast at-compile-time code
instead of that CONST BYTE var assignment. | This is a coding style thing. Personally I prefer the CONST construction as it allows the compiler for type checking. In a good compiler the CONST construct will result in just as efficient code as when using a #DEFINE.
What I do have a problem with is that the coding style is inconsistent. #defined values are written in capitals (nice) but the const values are in lower case (bad).
The other thing is that many of the #defined values could have been written as const or the other way around. Please choose one method and stick to that.
Code: | #use delay (clock=4000000) // 4MHz clock |
Good code, but better to write code that doesn't need comments:
Code: | #use delay (clock=4MHz) |
Code: | float mean_adc(byte channel) // Reads the adc port 30 times and gives the mean value
{
int i,mean_total = 30;
float mean = 0,mean1 = 0;
set_adc_channel(channel);
delay_us(100);
for (i=1; i<=mean_total; i++)
{
mean =mean + read_adc();
delay_us(100);
mean1=(mean/mean_total);
}
return(mean1);
} |
Variable mean_total has a bad name as it isn't the mean total but it is the number of averaging cycles, a better suggestion is averaging_cycle_count. Also, it isn't a variable but a constant.
You calculate mean1 every time inside the loop. This can be optimized to one time outside the loop.
Calculations with floats are very CPU intensive. Try to use integers wherever possible. Convert to float as late as possible.
Here a more optimized version. I did some other optimizations as well. Study the differences with your code and learn from it.: Code: | float mean_adc(int8 channel) // Reads the adc port 30 times and gives the mean value
{
const int8 AVERAGING_CYCLE_COUNT = 30
int8 i;
int16 mean_total = 0;
float mean = 0;
set_adc_channel(channel);
for (i = 1; i <= AVERAGING_CYCLE_COUNT; i++)
{
delay_us(100);
mean_total += read_adc();
}
mean = ((float)mean_total) / AVERAGING_CYCLE_COUNT;
return mean;
} |
|
|
![](templates/subSilver/images/spacer.gif) |
temtronic
Joined: 01 Jul 2010 Posts: 9174 Location: Greensville,Ontario
|
|
Posted: Sat Dec 17, 2016 10:05 am |
|
|
Floats are a huge waste of time. Remember the PIC (like all computers except humans) think in binary so if you really want to speed up the entire program sample the ADC input 32 times NOT 30.
That way the average can be calculated in ONE instruction, a simple rotate.
As a self-learning-exercise, dump (print) your current programs listing ( the .lst file) then do the '32' code/compile/dump listing.
Look at how small and hence fast the '32' program is.
Jay |
|
![](templates/subSilver/images/spacer.gif) |
gjs_rsdi
Joined: 06 Feb 2006 Posts: 468 Location: Bali
|
|
Posted: Wed Dec 28, 2016 1:55 am |
|
|
Jay
Can you explain how to do:
Quote: | that way the average can be calculated in ONE instruction, a simple rotate.
|
I am making a complicated averaging with FIFO to keep the data updated and would like to improve on it.
It is complicated because I am keeping mul/div by 2 and + only not to make complex calculation that takes a lot of time.
Best wishes
Joe |
|
![](templates/subSilver/images/spacer.gif) |
temtronic
Joined: 01 Jul 2010 Posts: 9174 Location: Greensville,Ontario
|
|
Posted: Wed Dec 28, 2016 6:51 am |
|
|
Code: | float mean_adc(int8 channel) // Reads the adc port 32 times and gives the mean value
{
const int8 AVERAGING_CYCLE_COUNT = 32
int8 i;
int16 mean_total = 0;
float mean = 0;
set_adc_channel(channel);
for (i = 1; i <= AVERAGING_CYCLE_COUNT; i++)
{
delay_us(10);
mean_total += read_adc();
}
mean = ((float)mean_total) / AVERAGING_CYCLE_COUNT;
return mean;
} |
All I did was change the 'count' from 30 to 32. ANY division using a 'binary' number (2,4,8,16,32,64) is a LOT faster.
Also I decreased the 'inter-reading' time from 100 to 10us.
BTW batteries, solar panels and temperature are all 'slow response' systems, you do not need ultra fast sampling.
Jay |
|
![](templates/subSilver/images/spacer.gif) |
gjs_rsdi
Joined: 06 Feb 2006 Posts: 468 Location: Bali
|
|
Posted: Wed Dec 28, 2016 4:13 pm |
|
|
Thanks Jay for the fast answer
Best wishes
Joe |
|
![](templates/subSilver/images/spacer.gif) |
|