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

need a bit of help with this snippet of code

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



Joined: 25 Mar 2007
Posts: 19

View user's profile Send private message

need a bit of help with this snippet of code
PostPosted: Sun Apr 08, 2007 8:36 pm     Reply with quote

I'm still working on this time and temp clock, I'm trying to make it say the time and temp on the quarter hours now, so I've modified the code some. The problem I'm having is the program works when the first designated time is reached, x:00, x:15, x:30, x:45, however, it won't work again. The clock and temperature keep running fine, however my say_time, say_temp functions won't work. Any clues?

Code:
void Main(void)
{

   int announce=0;                 // holds # of time/temp announcements

   init();                         // initialize I/O & peripherals

   while(1)                        // loop forever
   {
     read_time();                  // read & display DS1307 time
     read_temp();                  // read & display DS1620 temp

     // announce time/temp on roll-over to 00, 15, 30, 45 minutes
     if(ds1307_regs[MINUTES_REG]==0x00 || ds1307_regs[MINUTES_REG]==0x15
      || ds1307_regs[MINUTES_REG]==0x30 || ds1307_regs[MINUTES_REG]==0x45)
     {
       if(!announce)               // say time & temp only if announce=0
       {
         say_time();               // say the current time
         delay_ms(3000);           // delay 3 seconds
         say_temp();               // say the current temperature
         announce +=1;             // increment announcement counts
       } // end inner if
     } // end outer if

     if(!ds1307_regs[MINUTES_REG]==0x00 && !ds1307_regs[MINUTES_REG]==0x15
      && !ds1307_regs[MINUTES_REG]==0x30 && !ds1307_regs[MINUTES_REG]==0x45)

        announce=0;                  // clear announcements until it's time
   } // end while loop
} // end main
jecottrell



Joined: 16 Jan 2005
Posts: 559
Location: Tucson, AZ

View user's profile Send private message

PostPosted: Sun Apr 08, 2007 8:48 pm     Reply with quote

Sure you don't mean:

Code:

   int1 announce=true;                 // holds status of announcements

     // announce time/temp on roll-over to 00, 15, 30, 45 minutes
     if(ds1307_regs[MINUTES_REG]==0x00 || ds1307_regs[MINUTES_REG]==0x15
      || ds1307_regs[MINUTES_REG]==0x30 || ds1307_regs[MINUTES_REG]==0x45)
     {
       if(announce)               // say time & temp if enabled
       {
         say_time();               // say the current time
         delay_ms(3000);           // delay 3 seconds
         say_temp();               // say the current temperature
         announce = false;             // disable announcements
       } // end inner if
     }
     else
     {
        announce=true;                  // enable announcements
     }


Also, I use parentheses to excess to eliminate any problems with order of operations errors on my part.

John
mcnscott2



Joined: 25 Mar 2007
Posts: 19

View user's profile Send private message

PostPosted: Sun Apr 08, 2007 9:09 pm     Reply with quote

I'll give that a shot anything is better than pulling my hair out Shocked
jecottrell



Joined: 16 Jan 2005
Posts: 559
Location: Tucson, AZ

View user's profile Send private message

PostPosted: Sun Apr 08, 2007 9:29 pm     Reply with quote

If you look at the order of operations in the compiler manual the ! operator is evaluated before the &&. Also, what you were trying to express was:

Code:
((ds1307_regs[MINUTES_REG]!=0x00) && (ds1307_regs[MINUTES_REG]!=0x15)
      && (ds1307_regs[MINUTES_REG]!=0x30) && (ds1307_regs[MINUTES_REG]!=0x45))


... I think.

John
mcnscott2



Joined: 25 Mar 2007
Posts: 19

View user's profile Send private message

PostPosted: Sun Apr 08, 2007 9:31 pm     Reply with quote

I gave that a shot, and tried to change the false true statements in a couple places but nothing worked, still says time and temp on the quarter hour then never again, I changed my times though 30, 31, 32 instead of 15, 30, 45. Here is the original code by Bruce Reynolds, I've confirmed this works. Every hour on the hour it says the time and temp.

It's got me baffled too John, thanks for the help Smile I can't think of another way to put it in the code my psuedocode is:

1. if the time is x:00 or X:15 or X:30 or x:45
2. say time and temp
3. if the time and temp announcement has gone off
4. don't say it again until the next quarter hour.
(I messed with the || in place of the && for the second statment of the code and it kept saying the time and temp every 5 seconds instead of once.

Code:
void Main(void)
{
   int announce=0;                 // holds # of time/temp announcements

   init();                         // initialize I/O & peripherals

   while(1)                        // loop forever
   {
     read_time();                  // read & display DS1307 time
     read_temp();                  // read & display DS1620 temp

     // announce time/temp on roll-over from 59 to 00 minutes
     if(ds1307_regs[MINUTES_REG]==0x00)
     {
       if(!announce)               // say time & temp only if announce=0
       {
         say_time();               // say the current time
         delay_ms(3000);           // delay 3 seconds
         say_temp();               // say the current temperature
         announce +=1;             // increment announcement counts
       }
     }

     if(!ds1307_regs[MINUTES_REG]==0x00)
        announce=0;                // clear announcements until it's time
   }
}
jecottrell



Joined: 16 Jan 2005
Posts: 559
Location: Tucson, AZ

View user's profile Send private message

PostPosted: Sun Apr 08, 2007 9:43 pm     Reply with quote

Try this. I think you have an order of operations problem...

Code:

     int1 announce=true;                 // holds status of announcements

     // announce time/temp on roll-over to 00, 15, 30, 45 minutes
     if((ds1307_regs[MINUTES_REG]==0x00) || (ds1307_regs[MINUTES_REG]==0x15)
      || (ds1307_regs[MINUTES_REG]==0x30) || (ds1307_regs[MINUTES_REG]==0x45))
     {
       if(announce)               // say time & temp if enabled
       {
         say_time();               // say the current time
         delay_ms(3000);           // delay 3 seconds
         say_temp();               // say the current temperature
         announce = false;             // disable announcements
       } // end inner if
     }
     else
     {
        announce=true;                  // enable announcements
     }


I forget what format the values are in the ds1307_reg. You could have problem with that?
PCM programmer



Joined: 06 Sep 2003
Posts: 21708

View user's profile Send private message

PostPosted: Sun Apr 08, 2007 10:36 pm     Reply with quote

You may have problems with operator precedence, but your main
problem is that you've stuck a "0x" in front of all your time values.
This turns 15 minutes into 0x15 minutes, which is actually 21 minutes.
That's why it only works with 0x00. Even as a hex value, 0 is still 0.
mcnscott2



Joined: 25 Mar 2007
Posts: 19

View user's profile Send private message

PostPosted: Sun Apr 08, 2007 10:59 pm     Reply with quote

I know the hex numbering looks strange but it's not hex really, I have to put in the decimall minutes I want for the function to work, instead of 1E it has to be 30, 45 instead of 2D etc. Here is the code for the function that actually reads the time.

Code:
// read time & date from DS1307
void read_time(void)
{
   i2c_start();
   i2c_write(WRITE_CNTRL);

   // start i2c read at seconds register
   i2c_write(SECONDS_REG);

   i2c_start();
   i2c_write(READ_CNTRL);

   // read the 7 bytes from the ds1307. Mask off the unused bits
   ds1307_regs[SECONDS_REG]     = i2c_read() & 0x7f;
   ds1307_regs[MINUTES_REG]     = i2c_read() & 0x7f;
   ds1307_regs[HOURS_REG]       = i2c_read() & 0x3f;
   ds1307_regs[DAY_OF_WEEK_REG] = i2c_read() & 0x07;
   ds1307_regs[DATE_REG]        = i2c_read() & 0x3f;
   ds1307_regs[MONTH_REG]       = i2c_read() & 0x1f;
   ds1307_regs[YEAR_REG]        = i2c_read(0);

   i2c_stop();

   enable();                     // enable SLED4C data entry
   send_byte(0b11000001);        // all banks HEX decode
   disable();                    // write config value into config register

   enable();                     // enable SLED4C data entry
   send_nib(0x00);               // DIM display + DP's off
   send_byte(ds1307_regs[HOURS_REG]);   // display hour on banks 5/4
   send_byte(ds1307_regs[MINUTES_REG]); // display minutes on banks 3/2

   send_nib(0x02);               // turn colon on
   disable();                    // write data into display registers
   delay_ms(2000);               // wait 2 seconds between temp & time updates
}
PCM programmer



Joined: 06 Sep 2003
Posts: 21708

View user's profile Send private message

PostPosted: Sun Apr 08, 2007 11:19 pm     Reply with quote

You must be leaving the minutes in the BCD format used by the DS1307.

I made a test program and put in the same fix for operator precedence
that jecottrell suggested. It seems to work. It displays the time every
15 minutes. I didn't test it with all time values. You might test it further.
Quote:

Time = 00
say time
Time = 01
Time = 15
say time
Time = 16
Time = 30
say time
Time = 31
Time = 45
say time
Time = 59
Time = 00
say time
Time = 01
Time = 15
say time
Time = 16
Time = 30


Here's the test program. Tested with vs. 3.249 in MPLAB simulator.
Code:

#include <16F877.h>
#fuses XT, NOWDT, NOPROTECT, PUT, BROWNOUT, NOLVP
#use delay(clock=4000000)
#use rs232(baud=9600, xmit=PIN_C6, rcv=PIN_C7, ERRORS)

int8 ds1307_regs[8];

#define MINUTES_REG  1

//------------------------
void say_time(void)
{
printf("say time\r");
}

//------------------------
void say_temp(void)
{
//printf("say temp\r");
}


//------------------------
// Return some representative time values.
void read_time(void)
{
static int8 index = 0;
int8 time[8] = {0x00,0x01,0x15,0x16,0x30,0x31,0x45,0x59};

ds1307_regs[MINUTES_REG] = time[index++];
index &= 0x07;
}

//------------------------
void read_temp(void)
{
//printf("read temp\r");
}


//====================================
void main(void)
{
int announce=0;   

//   init();     

while(1)         
  {
   read_time();   
   printf("Time = %x\r", ds1307_regs[MINUTES_REG]);
   read_temp();   

   if(ds1307_regs[MINUTES_REG]==0x00 || ds1307_regs[MINUTES_REG]==0x15
      || ds1307_regs[MINUTES_REG]==0x30 || ds1307_regs[MINUTES_REG]==0x45)
     {
      if(!announce) 
        {
         say_time();
         delay_ms(30); 
         say_temp();   
         announce +=1; 
        }
     }


   if((ds1307_regs[MINUTES_REG] != 0x00) &&
      (ds1307_regs[MINUTES_REG] != 0x15) &&
      (ds1307_regs[MINUTES_REG] != 0x30) &&
      (ds1307_regs[MINUTES_REG] != 0x45))
     {
      announce=0;   
     } 

  }

mcnscott2



Joined: 25 Mar 2007
Posts: 19

View user's profile Send private message

PostPosted: Sun Apr 08, 2007 11:21 pm     Reply with quote

I'll give it a try thanks !

well your code worked for me, but it had me perplexed because I was sure, I had tried that operator arrangement many times, I finally figured out the problem.

I was trying to save time so instead of waiting 15 minutes for the message I had my time parameters set up at x:30, x:31,:x32, x:33 sooo.... The program never got a chance to play the next message because it saw that if the time was equal to x:31 it must have already played the message so it never played it, same for when the time got to x:32, then x:33. I can finally go to sleep! thanks everyone!
Ttelmah
Guest







PostPosted: Mon Apr 09, 2007 2:33 am     Reply with quote

Well done.
This little 'catch', is very common. It is why (for instance), you will see people who are aware of this, using an interrupt 'counter', and coding like:

(in the interrupt)

if (counter) --counter;

(in the main)

if (!counter) //operations here to be done when counter==0

Rather than checking for a counter getting 'to' a particular value. The risk of missing a particular 'count', gets even higher, when there are multiple asynchronous interrupts enabled, and this can be a pig to discover...

Best Wishes
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