Please help/teach me >.<

[l_e_a_88]

hey guys,
i m doing project which use to control light and fan for my room.
but now i m facing problems in PIC programming, since this is my 1st time trying to do this kind of project.

i found a source code from internet. i try to compile it with CSC compiler.
but i there are errors with:
#include <dimmer1.c> and #include <dimmer2.c>

mean that i need to write extra code for the those dimmers?
can anybody help?


The code:

#include<16F72.h>
#use delay (clock=12000000)
#fuses XT, NOWDT, PROTECT
#use i2c(master, sda=PIN_C4, scl=PIN_C3)
#include<lcd420.c>

int   temph , templ;
int1   x=0 , y=0;
int   m;

#define   zcd   PIN_A4
#define   pir   PIN_B0
#define   ldr   PIN_A2
#define   dim1  PIN_A0
#define   dim2  PIN_A1

#include <temperature.c>
#include <dimmer1.c>
#include <dimmer2.c>

#int_ext

void main()
{
   output_low(dim1);
   output_low(dim2);
   X=0;
   Y=0;

   Setup_adc_ports(NO_ANALOGS) ;    //All are digital only
   
   temp_init() ;
   lcd_init() ;
   LCD_PUTC("POWER SAVER \f") ;
   delay_ms(2000) ;

   while(1)
      { 
      if (!input(pir))
      {
      output_high(dim1) ;
      while(!input(pir)){}
      X=~x ;   
      lcd_putc("PIR ON \n") ;
      delay_ms(5000) ;
      }

      if(x==0)
      {
         if(y==1)
         {
         delay_ms(20000);
         Y=0 ;
         }
      output_low(dim1);
      output_low(dim2);
      lcd_putc("STAND BY \n") ;
      delay_ms(300);
      }

      if(x==1)
      {
      y=1;
      temp_read();

         if(templ==0x80) templ=5;
            else   templ=0;
         printf(lcd_putc,"\n TEMP: %02d.%d dc",temph,templ);




         if(temph>=27 && temph<=28)
         {
back25:                   if(!input(zcd))goto back25;   //waiting to get zero start
                          delay_ms(5);
                          output_high(dim2);

back26:                   if(!input(zcd))goto back26;      //waiting to get zero start
                          output_low(dim2);
                          delay_ms(5);
                          output_high(dim2);

back27:                   if(!input(zcd))goto back27;
                          output_low(dim2);
         }



         if(temph>=29 && temph<=31)
         {
back20:                   if(!input(zcd))goto back20;   //waiting to get zero start
                          delay_ms(5);         //161
                          output_high(dim2);

back30:                   if(!input(zcd))goto back30;   //waiting to get zero start
                          output_low(dim2);
                          delay_ms(5);
                          output_high(dim2);

back32:                   if(!input(zcd))goto back32;
                          output_low(dim2);
         }



         if(temph>31 && temph<=33)
         {
back9:                    if(!input(zcd))goto back9;   //waiting to get zero start
                          delay_ms(5);         //161
                          output_high(dim2);

back10:                    if(!input(zcd))goto back10;   //waiting to get zero start
                           output_low(dim2);
                           delay_ms(5);
                           output_high(dim2);

back11:                    if(!input(zcd))goto back11;
                           output_low(dim2);
         }



         
         if(temph>33 && temph<=35)
         {
back12:                     if(!input(zcd))goto back12;   //waiting to get zero start
                            delay_ms(3);                    //161
                            output_high(dim2);

back13:                      if(!input(zcd))goto back10;   //waiting to get zero start
                             output_low(dim2);
                             delay_ms(3);
                             output_high(dim2);

back14:                      if(!input(zcd))goto back14;
                             output_low(dim2);
         }
   


         if(temph>3 && temph<=37)
         {
           
back15:                     if(!input(zcd))goto back15;   //waiting to get zero start
                            delay_ms(2);
                            output_high(dim2);

back16:                      if(!input(zcd))goto back16;   //waiting to get zero start
                             output_low(dim2);
                             delay_ms(2);
                             output_high(dim2);

back17:                     if(!input(zcd))goto back17;
                             output_low(dim2);
                    }



               if(temph>37)
            output_high(dim2);

               if(input(ldr))
            output_high(dim1);

         if(!input(ldr))
            output_low(dim1);

             }
   }

}

THANK YOU

[Graynomad]

I haven't looked at the code overall but you should get rid of all these

Code: [Select]

back20:                   if(!input(zcd))goto back20;   //waiting to get zero start
use

Code: [Select]

while (!input(zcd)) {};
All these

Code: [Select]

        if(temph>33 && temph<=35)
         {
back12:                     if(!input(zcd))goto back12;   //waiting to get zero start
                            delay_ms(3);                    //161
                            output_high(dim2);

back13:                      if(!input(zcd))goto back10;   //waiting to get zero start
                             output_low(dim2);
                             delay_ms(3);
                             output_high(dim2);

back14:                      if(!input(zcd))goto back14;
                             output_low(dim2);
         }
Do almost exactly the same thing. They should be removed and probably replaced with a function that takes the two delay periods.

Code: [Select]

        if(temph>3 && temph<=37)
 
I think this is a typo, should be 35 ?

Code: [Select]

if(!input(zcd))All these are potential places for your code to hang if the input never becomes true.



______
Rob

[l_e_a_88]

hi there, thank you so much...
but i would like to ask do you mean that i have to change all this

Code: [Select]

    if(temph>33 && temph<=35)
         {
back12:                     if(!input(zcd))goto back12;   //waiting to get zero start
                            delay_ms(3);                    //161
                            output_high(dim2);

back13:                      if(!input(zcd))goto back10;   //waiting to get zero start
                             output_low(dim2);
                             delay_ms(3);
                             output_high(dim2);

back14:                      if(!input(zcd))goto back14;
                             output_low(dim2);
         }

to

Code: [Select]

while (!input(zcd)) {};
isit?





and may i knw what do u mean by

probably replaced with a function that takes the two delay periods.

thank you =)


sorry i m 1st to programming so might gt too much question to ask.....

[Graynomad]

back12:                     if(!input(zcd))goto back12;

There is almost NEVER a reason to use a goto in C, this construct is better done as

Code: [Select]

while (!input(zcd)) {}Apart from anything else it saves you from thinking up a 100 names

probably replaced with a function that takes the two delay periods

Whenever you see blocks of code that are the same or even similar it's a flag that says "I should be using a function here". These blocks

Code: [Select]

{
back12:                     if(!input(zcd))goto back12;   //waiting to get zero start
                            delay_ms(3);                    //161
                            output_high(dim2);

back13:                      if(!input(zcd))goto back10;   //waiting to get zero start
                             output_low(dim2);
                             delay_ms(3);
                             output_high(dim2);

back14:                      if(!input(zcd))goto back14;
                             output_low(dim2);
         }
Do almost the same thing (some are exactly the same), so they should be replaced with a function. This makes the code smaller and easier to read, therefore less prone to errors.

Another reason to do this is that it concentrates a job in one place, so if a change needs to be made you only do it once.

I've change your program to show this

Code: [Select]

#include<16F72.h>
#use delay (clock=12000000)
#fuses XT, NOWDT, PROTECT
#use i2c(master, sda=PIN_C4, scl=PIN_C3)
#include<lcd420.c>

int   temph , templ;
int1   x=0 , y=0;
int   m;

#define   zcd   PIN_A4
#define   pir   PIN_B0
#define   ldr   PIN_A2
#define   dim1  PIN_A0
#define   dim2  PIN_A1

#include <temperature.c>
#include <dimmer1.c>
#include <dimmer2.c>

#int_ext

void main()
{
   output_low(dim1);
   output_low(dim2);
   X=0;
   Y=0;

   Setup_adc_ports(NO_ANALOGS) ;    //All are digital only
   
   temp_init() ;
   lcd_init() ;
   LCD_PUTC("POWER SAVER \f") ;
   delay_ms(2000) ;

   while(1)
      { 
      if (!input(pir))
      {
      output_high(dim1) ;
      while(!input(pir)){}
      X = ~x ;   
      lcd_putc("PIR ON \n") ;
      delay_ms(5000) ;
      }

      if(x==0)
  {
if(y==1)
{
delay_ms(20000);
Y=0 ;
}
  output_low(dim1);
  output_low(dim2);
  lcd_putc("STAND BY \n") ;
  delay_ms(300);
  }

      if(x==1)
  {
  y=1;
  temp_read();

if(templ==0x80)
templ=5;
else   
templ=0;

printf(lcd_putc,"\n TEMP: %02d.%d dc",temph,templ);

if(temph>=27 && temph<=28)
{
my_func (5, 5);
}

if(temph>=29 && temph<=31)
{
my_func (5, 5);
}

if(temph>31 && temph<=33)
{
my_func (5, 5);
}

if(temph>33 && temph<=35)
{
my_func (3, 3);
}

if(temph>35 && temph<=37)
{
my_func (2, 2);
}

if(temph>37)
output_high(dim2);

   if(input(ldr))
output_high(dim1);

if(!input(ldr))
output_low(dim1);

}
}

}

void my_func (int time1, int time2) {

while (!input(zcd)) {};
delay_ms(time1);
output_high(dim2);

while (!input(zcd)) {};
output_low(dim2);
delay_ms(time2);
output_high(dim2);

while (!input(zcd)) {};
output_low(dim2);
}


It's now half the size and a lot clearer. So much so that we now see the first three tests do the same thing, so let's consolidate them.

Code: [Select]

#include<16F72.h>
#use delay (clock=12000000)
#fuses XT, NOWDT, PROTECT
#use i2c(master, sda=PIN_C4, scl=PIN_C3)
#include<lcd420.c>

int   temph , templ;
int1   x=0 , y=0;
int   m;

#define   zcd   PIN_A4
#define   pir   PIN_B0
#define   ldr   PIN_A2
#define   dim1  PIN_A0
#define   dim2  PIN_A1

#include <temperature.c>
#include <dimmer1.c>
#include <dimmer2.c>

#int_ext

void main()
{
   output_low(dim1);
   output_low(dim2);
   X=0;
   Y=0;

   Setup_adc_ports(NO_ANALOGS) ;    //All are digital only
   
   temp_init() ;
   lcd_init() ;
   LCD_PUTC("POWER SAVER \f") ;
   delay_ms(2000) ;

   while(1)
      { 
      if (!input(pir))
      {
      output_high(dim1) ;
      while(!input(pir)){}
      X = ~x ;   
      lcd_putc("PIR ON \n") ;
      delay_ms(5000) ;
      }

      if(x==0)
  {
if(y==1)
{
delay_ms(20000);
Y=0 ;
}
  output_low(dim1);
  output_low(dim2);
  lcd_putc("STAND BY \n") ;
  delay_ms(300);
  }

      if(x==1)
  {
  y=1;
  temp_read();

if(templ==0x80)
templ=5;
else   
templ=0;

printf(lcd_putc,"\n TEMP: %02d.%d dc",temph,templ);

if(temph>=27 && temph<=33)
{
my_func (5, 5);
}

if(temph>33 && temph<=35)
{
my_func (3, 3);
}

if(temph>35 && temph<=37)
{
my_func (2, 2);
}

if(temph>37)
output_high(dim2);

   if(input(ldr))
output_high(dim1);

if(!input(ldr))
output_low(dim1);

}
}

}

void my_func (int time1, int time2) {

while (!input(zcd)) {};
delay_ms(time1);
output_high(dim2);

while (!input(zcd)) {};
output_low(dim2);
delay_ms(time2);
output_high(dim2);

while (!input(zcd)) {};
output_low(dim2);
}

Finally we see that the two times are always the same so that needs changing, and also why not make it table-driven?

Code: [Select]

#include<16F72.h>
#use delay (clock=12000000)
#fuses XT, NOWDT, PROTECT
#use i2c(master, sda=PIN_C4, scl=PIN_C3)
#include<lcd420.c>

int   temph , templ;
int1   x=0 , y=0;
int   m;

#define   zcd   PIN_A4
#define   pir   PIN_B0
#define   ldr   PIN_A2
#define   dim1  PIN_A0
#define   dim2  PIN_A1

#include <temperature.c>
#include <dimmer1.c>
#include <dimmer2.c>

#int_ext

int times [] = {5,5,5,5,5,5,5,3,3,2,2}; // times for temp values from 27 to 37 inclusive

void main()
{
   output_low(dim1);
   output_low(dim2);
   X=0;
   Y=0;

   Setup_adc_ports(NO_ANALOGS) ;    //All are digital only
   
   temp_init() ;
   lcd_init() ;
   LCD_PUTC("POWER SAVER \f") ;
   delay_ms(2000) ;

   while(1)
      { 
      if (!input(pir))
      {
      output_high(dim1) ;
      while(!input(pir)){}
      X = ~x ;   
      lcd_putc("PIR ON \n") ;
      delay_ms(5000) ;
      }

    if(x==0)
  {
if(y==1)
{
delay_ms(20000);
Y=0 ;
}
  output_low(dim1);
  output_low(dim2);
  lcd_putc("STAND BY \n") ;
  delay_ms(300);
  }

if(x==1)
{
y=1;
temp_read();

if(templ==0x80)
templ=5;
else   
templ=0;

printf(lcd_putc,"\n TEMP: %02d.%d dc",temph,templ);

if (temph > 26 && temph < 38) {
my_func (temps[temph - 27]);
}

if(temph > 37)
output_high(dim2);

if(input(ldr))
output_high(dim1);
else
output_low(dim1);

}
}

}

void my_func (int time) {

while (!input(zcd)) {};
delay_ms(time);
output_high(dim2);

while (!input(zcd)) {};
output_low(dim2);
delay_ms(time);
output_high(dim2);

while (!input(zcd)) {};
output_low(dim2);
}

So now the whole program is 1/4 the size and arguably much clearer.

NOTE: Not tested or even compiled.

Hope that's not to big a leap for you, ask if you don't understand anything.

______
Rob

[arrrrgon]

No offense, but you should really get a Beginner's C programming book.  It's going to be hard to follow along with anyone trying to help you if you don't understand the language.  There are some languages where you might be able to pull it off, but C isn't one of those.  It's a great language to learn btw.

[waltr]

To add:

Check this tutorial for using and programming PICs:
http://www.gooligum.com.au/tutorials.html

The best C reference book is the original by K&R and readily available:
http://en.wikipedia.org/wiki/The_C_Programming_Language
http://www.amazon.com/Programming-Language-2nd-Brian-Kernighan/dp/0131103628

Online is "The C Book":
http://publications.gbdirect.co.uk/c_book/

[l_e_a_88]

THank you so much i will try again....

if still facing any problem i will gt help from u guys again.... =)
have a nice day

[l_e_a_88]

Hi Graynomad,
thank you so much..since i m stupid but u still willing to teach me patiently....>.<

i try to compile the code you modify with CCS ...

Code: [Select]

#include<16F72.h>
#use delay (clock=12000000)
#fuses XT, NOWDT, PROTECT
#use i2c(master, sda=PIN_C4, scl=PIN_C3)
#include<lcd420.c>

int   temph , templ;
int1   x=0 , y=0;
int   m;

#define   zcd   PIN_A4
#define   pir   PIN_B0
#define   ldr   PIN_A2
#define   dim1  PIN_A0
#define   dim2  PIN_A1

#include <temperature.c>
#include <dimmer1.c>
#include <dimmer2.c>

#int_ext

int times [] = {5,5,5,5,5,5,5,3,3,2,2}; // times for temp values from 27 to 37 inclusive

void main()
{
   output_low(dim1);
   output_low(dim2);
   X=0;
   Y=0;

   Setup_adc_ports(NO_ANALOGS) ;    //All are digital only
   
   temp_init() ;
   lcd_init() ;
   LCD_PUTC("POWER SAVER \f") ;
   delay_ms(2000) ;

   while(1)
      { 
      if (!input(pir))
      {
      output_high(dim1) ;
      while(!input(pir)){}
      X = ~x ;   
      lcd_putc("PIR ON \n") ;
      delay_ms(5000) ;
      }

    if(x==0)
  {
if(y==1)
{
delay_ms(20000);
Y=0 ;
}
  output_low(dim1);
  output_low(dim2);
  lcd_putc("STAND BY \n") ;
  delay_ms(300);
  }

if(x==1)
{
y=1;
temp_read();

if(templ==0x80)
templ=5;
else   
templ=0;

printf(lcd_putc,"\n TEMP: %02d.%d dc",temph,templ);

if (temph > 26 && temph < 38) {
my_func (temps[temph - 27]);
}

if(temph > 37)
output_high(dim2);

if(input(ldr))
output_high(dim1);
else
output_low(dim1);

}
}

}

void my_func (int time) {

while (!input(zcd)) {};
delay_ms(time);
output_high(dim2);

while (!input(zcd)) {};
output_low(dim2);
delay_ms(time);
output_high(dim2);

while (!input(zcd)) {};
output_low(dim2);
}

 


but error found here

Code: [Select]

my_func (temps[temph - 27]); can you explain to me what is the function for this line?
and then isit a typo  "temps"?

[Graynomad]

oops, yes that's a typo on my part.

This is an array of integers

int times [] = {5,5,5,5,5,5,5,3,3,2,2}; // times for temp values from 27 to 37 inclusive

this line is supposed to get a number from that array

my_func (temps[temph - 27]);

Note that it should be "times" not "temps", the compiler was looking for an array called temps and of course it doesn't exist.

This line does to following

take temps between 27 and 37
subtract 27
now we have a number from 0 to 10
use that number to index into the times array
get a number from the times array
pass that to the my_func

Have you tracked down a book or online C reference yet?

If the book has good examples you will see all sorts of ways to do things, there will ba an example to flash a LED say but you will say "Ah, I could use that technique for my project".

BTW, I got a message from skype asking me to sign up which of course I immediately deleted as spam. Then while writing this I recognized the l_e_a_88 handle as being the same in the skype request so it wasn't spam after all. Anyway I doubt my bandwidth is up to using skype.

_____
Rob

[l_e_a_88]

thank you so much Graynomad.

oh yeah i m refering to some book now... i think i can understand the code u modify now...it's easier for me to undertand compare to the previous code.
but i have another question...

can i just ignore those #include<.c>?


: yea, that is my skype...it's fine then..
anyway can i email u if i face any problem? or u will always discuss here?

[Graynomad]

can i just ignore those #include<.c>?

Often these are just left over from a piece of code that was copied from another app.

You may be able to remove them, just comment the lines (one at a time) and see if you get compiler errors. If so uncomment the line and live with it

______
Rob

[l_e_a_88]

Hi graynomad, thanks again.

sorry, i have another question...
i get a warning like this in my temperature.c

Code: [Select]

temp_init()
{
   i2c_start();
   i2c_write(0x90);// device address
   i2c_write(0xac);// control register
   i2c_write(00);// write mode
   i2c_stop();
   delay_ms(10);
   i2c_start();
   i2c_write(0x90);// device address
   i2c_write(0xee);// control register
   i2c_stop();
}
 temp_read()
{
   i2c_start();
   i2c_write(0x90);
   i2c_write(0xaa);
   i2c_start();
   i2c_write(0x91);
   temph=i2c_read();
   templ=i2c_read(0);
   i2c_stop();
}

Function not void and does not return a value of temp_init
Function not void and does not return a value of temp_read

is it matter?
not necessary to return a value right?
do you mind to explain to me what the warning warn actually?
 
thank you :]


sorry, i keep asking you Questions.

[waltr]

You must define what the function returns and what is expected to be passed to the function such as:

int Afunct( char a)
{
.....
}

This says that it expects a variable of type char and named a
and the function then returns a variable of type int.
From here you should be able to figure out what is now required.

Look this up in "The C Book" I gave you a link to or in K&R if you bought that book.

[dhaval22891]

Have you run your project using this coding?