Pages: [1]   Go Down
Author Topic: why does this code execute twice in quick succession, pray?  (Read 701 times)
0 Members and 1 Guest are viewing this topic.
Wales
Offline Offline
Full Member
***
Karma: 0
Posts: 243
Don't take things too seriously
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Code:

  if (currentMillis > 15000 && RTCInitiallySet < 1)
  {
    Serial.println("Sync the clock.");
    Serial.println();
    SetRTC = 1;
  }

 if (now.minute() == 59 && now.second() == 30)
  {
    if (SetRTC == 0)
    {
      SetRTC = 1;
      Serial.println("Syncing the clock ready for the pips: ");
      Serial.println();
    }
  }

Code:
if (SetRTC == 1 && EthernetRunning == true)
  {
    RTCInitiallySet = 1;              // RTC has been told to initially set
    Serial.print("RTC before: ");
    PrintDateTime(RTC.now());
    Serial.println();
    sendNTPpacket(timeServer);
    delay(1000);
    if ( Udp.parsePacket() )
    {
      Udp.read(pb, NTP_PACKET_SIZE);
      unsigned long t1, t2, t3, t4;
      t1 = t2 = t3 = t4 = 0;
      for (int i=0; i< 4; i++)
      {
        t1 = t1 << 8 | pb[16+i];
        t2 = t2 << 8 | pb[24+i];
        t3 = t3 << 8 | pb[32+i];
        t4 = t4 << 8 | pb[40+i];
      }

      float f1,f2,f3,f4;
      f1 = ((long)pb[20] * 256 + pb[21]) / 65536.0;
      f2 = ((long)pb[28] * 256 + pb[29]) / 65536.0;
      f3 = ((long)pb[36] * 256 + pb[37]) / 65536.0;
      f4 = ((long)pb[44] * 256 + pb[45]) / 65536.0;
      const unsigned long seventyYears = 2208988800UL;
      t1 -= seventyYears;
      t2 -= seventyYears;
      t3 -= seventyYears;
      t4 -= seventyYears;
      t4 += 1;
      if (f4 > 0.4) t4++;
      RTC.adjust(DateTime(t4));
      Serial.print("RTC after : ");
      PrintDateTime(RTC.now());
      Serial.println();
      Serial.println("Time sync successful.");
      Serial.println();
      playTone(10, 3000);
      SetRTC = 0;            // RTC has been set
    }
    else
    {
      Serial.println("Error: UDP Unavailable.");
      Serial.println("Cancelling sync.");
      SetRTC = 0;            // Cancels the sync
      RTCInitiallySet = 1;      // Cancels the sync
    }
  }

I have a theory...and that's because the sync causes the time to be second 30 twice by adjusting the clock...what's weird is that this happens EVERY SINGLE HOUR...and I would have thought it would only happen now and then...and it's odd that the RTC adds a second every hour.

Otherwise I'm somehow cancelling/setting the sync ilogically in the code.

THANKS!
Logged

UK
Offline Offline
Shannon Member
****
Karma: 184
Posts: 11173
-
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

No idea. It's very difficult to see why code is being called without seeing what's calling it. Try posting the whole sketch.
Logged

I only provide help via the forum - please do not contact me for private consultancy.

Wales
Offline Offline
Full Member
***
Karma: 0
Posts: 243
Don't take things too seriously
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

difficult to see why code is being called without seeing what's calling it.

I thought I included the code's calls?

Here is the entire sketch.

Code:
#include <Bounce.h>
#include <Wire.h>
#include "RTClib.h"
#include <Servo.h>
#include <SPI.h>
#include <Ethernet.h>
#include <EthernetUdp.h>
byte mac[] = {
  0x00, 0x11, 0x22, 0x33, 0xFB, 0x11};
EthernetClient client;
////////////NTP UPDATE////////////
unsigned int localPort = 8888; // NTP Port
byte timeServer[] = {
  193, 79, 237, 14}; // ntp1.nl.net NTP server
const int NTP_PACKET_SIZE= 48; // NTP time stamp is in the first 48 bytes of the message
byte pb[NTP_PACKET_SIZE]; // Buffer to hold incoming and outgoing packets
EthernetUDP Udp;
unsigned long currentMillis = 0;
////////////CONNECTIVITY////////////
boolean EthernetRunning = true;        
////////////SERVER////////////
IPAddress server(207,7,108,203); // Joyent
////////////SERVO////////////
Servo BellControlServo;
////////////MISC////////////
const int PiezoPin = 7; // Piezo buzzer pin 7


///////////SWITCHES///////////
const int switchPin1 = 1;     
long switchPin1TriggeredTime = 0; // When was the analog read triggered
//
const int switchPin2 = 2;     
//
const int switchPin3 = 3;     
//
///////PHONE RELAY
const int relay = 4; 
//
////////////THE GMT PIPS////////////
unsigned int pip1 = 0;
unsigned int pip2 = 0;
unsigned int pip3 = 0;
unsigned int pip4 = 0;
unsigned int pip5 = 0;
unsigned int pip6 = 0;
unsigned int SetRTC = 0;
unsigned int RTCInitiallySet = 0;
////////////RTC////////////

RTC_DS1307 RTC;
enum
{
  OFF, ON, FLASH
};
void setup()
{


  //////////// MISC////////////
  pinMode(PiezoPin, OUTPUT);
  pinMode(switchPin1, INPUT);     
  pinMode(switchPin2, INPUT);     
  pinMode(switchPin3, INPUT);
  pinMode(relay, OUTPUT);   
  BellControlServo.attach(6);
  Serial.begin(57600);
  Wire.begin();
  RTC.begin();

  Serial.println("Annunciator Panel v0.9d");
  Ethernet.begin(mac);
  if (Ethernet.begin(mac) == 0)
  {
    EthernetRunning = false;
    Serial.println("Error: No DHCP!");
  }
  if(EthernetRunning)
  {
    Udp.begin(localPort);
    Serial.println("DHCP is working - enabling UDP.");
  }
  Serial.println();
  if (! RTC.isrunning())
  {
    Serial.println("Error: The RTC is NOT available!");
  }
}
void loop()
{
  //Don't forget warning pips for when sound is turned off for more than 2hrs!//
  DateTime now = RTC.now();
  currentMillis = millis();
  NurseryVoltsRead = analogRead(NurseryPin);
  BathroomVoltsRead = analogRead(BathroomPin);
  TradesmenVoltsRead = analogRead(TradesmenPin);
  MasterBedroomVoltsRead = analogRead(MasterBedroomPin);
  DrawingRoomVoltsRead = analogRead(DrawingRoomPin);

  if (currentMillis > 15000 && RTCInitiallySet < 1)
  {
    Serial.println("Sync the clock.");
    Serial.println();
    SetRTC = 1;
  }
  if (SetRTC == 1 && EthernetRunning == false)
  {
    SetRTC = 0;            // Cancels the sync
    RTCInitiallySet = 1;              // Cancels the sync
    Serial.println("Error: RTC could not be synced because there is no internet connectivity.");
  }
  if (SetRTC == 1 && EthernetRunning == true)
  {
    RTCInitiallySet = 1;              // RTC has been told to initially set
    Serial.print("RTC before: ");
    PrintDateTime(RTC.now());
    Serial.println();
    sendNTPpacket(timeServer);
    delay(1000);
    if ( Udp.parsePacket() )
    {
      Udp.read(pb, NTP_PACKET_SIZE);
      unsigned long t1, t2, t3, t4;
      t1 = t2 = t3 = t4 = 0;
      for (int i=0; i< 4; i++)
      {
        t1 = t1 << 8 | pb[16+i];
        t2 = t2 << 8 | pb[24+i];
        t3 = t3 << 8 | pb[32+i];
        t4 = t4 << 8 | pb[40+i];
      }

      float f1,f2,f3,f4;
      f1 = ((long)pb[20] * 256 + pb[21]) / 65536.0;
      f2 = ((long)pb[28] * 256 + pb[29]) / 65536.0;
      f3 = ((long)pb[36] * 256 + pb[37]) / 65536.0;
      f4 = ((long)pb[44] * 256 + pb[45]) / 65536.0;
      const unsigned long seventyYears = 2208988800UL;
      t1 -= seventyYears;
      t2 -= seventyYears;
      t3 -= seventyYears;
      t4 -= seventyYears;
  t4 += 3600;     //British Summer Time
      t4 += 1;
      if (f4 > 0.4) t4++;
      RTC.adjust(DateTime(t4));
      Serial.print("RTC after : ");
      PrintDateTime(RTC.now());
      Serial.println();
      Serial.println("Time sync successful.");
      Serial.println();
      playTone(10, 3000);
      SetRTC = 0;            // RTC has been set
    }
    else
    {
      Serial.println("Error: UDP Unavailable.");
      Serial.println("Cancelling sync.");
      SetRTC = 0;            // Cancels the sync
      RTCInitiallySet = 1;      // Cancels the sync
    }
  }
 

  if (now.minute() == 59 && now.second() == 30)
  {
    if (SetRTC == 0)
    {
      SetRTC = 1;
      Serial.println("Syncing the clock ready for the pips: ");
      Serial.println();
    }
  }

}
void playTone(long duration, int freq)
{
  duration *= 1000;
  int period = (1.0 / freq) * 1000000;
  long elapsed_time = 0;
  while (elapsed_time < duration)
  {
    digitalWrite(PiezoPin,HIGH);
    delayMicroseconds(period / 2);
    digitalWrite(PiezoPin, LOW);
    delayMicroseconds(period / 2);
    elapsed_time += (period);
  }
}
void PrintDateTime(DateTime t)
{
  char datestr[24];
  sprintf(datestr, "%04d-%02d-%02d  %02d:%02d:%02d  ", t.year(), t.month(), t.day(), t.hour(), t.minute(), t.second());
  Serial.print(datestr);
}
unsigned long sendNTPpacket(byte *address)
{
  // set all bytes in the buffer to 0
  memset(pb, 0, NTP_PACKET_SIZE);
  // Initialize values needed to form NTP request
  // (see URL above for details on the packets)
  pb[0] = 0b11100011;   // LI, Version, Mode
  pb[1] = 0;     // Stratum, or type of clock
  pb[2] = 6;     // Polling Interval
  pb[3] = 0xEC;  // Peer Clock Precision
  // 8 bytes of zero for Root Delay & Root Dispersion
  pb[12]  = 49;
  pb[13]  = 0x4E;
  pb[14]  = 49;
  pb[15]  = 52;

  // all NTP fields have been given values, now
  // you can send a packet requesting a timestamp:
  Udp.beginPacket(address, 123); //NTP requests are to port 123
  Udp.write(pb,NTP_PACKET_SIZE);
  Udp.endPacket();
}
Logged

0
Offline Offline
Shannon Member
****
Karma: 161
Posts: 10438
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

We don't know what you mean by "code execute twice in quick succession", nor why that is wrong.  Code is executing continuously in quick sucession all the time!!!
Logged

[ I won't respond to messages, use the forum please ]

Wales
Offline Offline
Full Member
***
Karma: 0
Posts: 243
Don't take things too seriously
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

We don't know what you mean by "code execute twice in quick succession", nor why that is wrong.  Code is executing continuously in quick sucession all the time!!!

Sorry to have provoked a code red three exclamation mark response! smiley-wink  smiley-sweat  smiley-razz

No - seriously, my bad.

Basically the code snippet I posted initially

Code:
RTCInitiallySet = 1;              // RTC has been told to initially set
    Serial.print("RTC before: ");
    PrintDateTime(RTC.now());
    Serial.println();
    sendNTPpacket(timeServer);
    delay(1000);
    if ( Udp.parsePacket() )
    {
      Udp.read(pb, NTP_PACKET_SIZE);
      unsigned long t1, t2, t3, t4;
      t1 = t2 = t3 = t4 = 0;
      for (int i=0; i< 4; i++)
      {
        t1 = t1 << 8 | pb[16+i];
        t2 = t2 << 8 | pb[24+i];
        t3 = t3 << 8 | pb[32+i];
        t4 = t4 << 8 | pb[40+i];
      }

      float f1,f2,f3,f4;
      f1 = ((long)pb[20] * 256 + pb[21]) / 65536.0;
      f2 = ((long)pb[28] * 256 + pb[29]) / 65536.0;
      f3 = ((long)pb[36] * 256 + pb[37]) / 65536.0;
      f4 = ((long)pb[44] * 256 + pb[45]) / 65536.0;
      const unsigned long seventyYears = 2208988800UL;
      t1 -= seventyYears;
      t2 -= seventyYears;
      t3 -= seventyYears;
      t4 -= seventyYears;
      t4 += 1;
      if (f4 > 0.4) t4++;
      RTC.adjust(DateTime(t4));
      Serial.print("RTC after : ");
      PrintDateTime(RTC.now());
      Serial.println();
      Serial.println("Time sync successful.");
      Serial.println();
      playTone(10, 3000);
      SetRTC = 0;            // RTC has been set

is called twice when I would expect it to be called only once. It sets the RTC from an NTP server twice in quick succession. This is undesirable.

I am trying to call that chunk of code once per hour on the 30th second. Somehow - either because of the theory I described in my first post - or because I have a logic error, the code runs twice around the 30th second.

If this is still unclear, please let me know - I will do my best to clarify.
Logged

Greenville, IL
Offline Offline
Edison Member
*
Karma: 11
Posts: 1309
Warning Novice on board! 0 to 1 chance of errors!
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset


  I am just guessing but, do you need to make this false after testing it for true?
Code:
if ( Udp.parsePacket() )
    {
Logged


New Hampshire
Offline Offline
God Member
*****
Karma: 13
Posts: 779
There are 10 kinds of people, those who know binary, and those who don't.
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Code:
if (now.minute() == 59 && now.second() == 30)
  {
    if (SetRTC == 0)
    {
      SetRTC = 1;
      Serial.println("Syncing the clock ready for the pips: ");
      Serial.println();
    }
  }

Is the block of code in that conditional getting executing twice as well?
Logged


Wales
Offline Offline
Full Member
***
Karma: 0
Posts: 243
Don't take things too seriously
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Is the block of code in that conditional getting executing twice as well?
Yes - it is. smiley
« Last Edit: April 20, 2012, 01:37:41 pm by Dane » Logged

UK
Offline Offline
Shannon Member
****
Karma: 184
Posts: 11173
-
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset


Basically the code snippet I posted initially ... is called twice when I would expect it to be called only once. It sets the RTC from an NTP server twice in quick succession. This is undesirable.


The logic looks quite convoluted, but from the symptoms I assume that the loop() function is finding SetRTC == 1 on multiple iterations.

One reason might be that it manages to execute twice while (now.minute() == 59 && now.second() == 30). I see that you have a delay(1000) in there but I'd try increasing that to something like delay(2000) just to rule out this sort of thing. If that doesn't resolve the problem I suggest you do some major simplification on your control logic. Doing something once at startup and again each time we get 30 seconds away from the hour shouldn't need that much complexity.
Logged

I only provide help via the forum - please do not contact me for private consultancy.

Wales
Offline Offline
Full Member
***
Karma: 0
Posts: 243
Don't take things too seriously
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

"The logic looks quite convoluted"
"I suggest you do some major simplification on your control logic"
"shouldn't need that much complexity".

Hi Peter

I didn't think my logic was that convoluted or complex - are you talking about the where I posted the code for the entire project?

I think what I'll do is include a millis-debounce check to only run the if ( Udp.parsePacket() ) if it hasn't been run in the last 25 minutes.
Logged

Pages: [1]   Go Up
Jump to: