Go Down

Topic: why does this code execute twice in quick succession, pray? (Read 890 times) previous topic - next topic

Dane

Code: [Select]

  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: [Select]
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!

PeterH

No idea. It's very difficult to see why code is being called without seeing what's calling it. Try posting the whole sketch.
I only provide help via the forum - please do not contact me for private consultancy.

Dane


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: [Select]
#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();
}

MarkT

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!!!
[ I won't respond to messages, use the forum please ]

Dane


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-sweat:  :P

No - seriously, my bad.

Basically the code snippet I posted initially

Code: [Select]
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.

cyclegadget


  I am just guessing but, do you need to make this false after testing it for true?
Code: [Select]
if ( Udp.parsePacket() )
    {
Good links: Eagle tutorial= http://www.youtube.com/playlist?list=PLDE1858BD83D19C70
General Arduion tutorials = http://tronixstuff.wordpress.com
http://www.gammon.com.au/forum/bbshowpost.php?bbtopic_id=123

jraskell

Code: [Select]

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?

Dane

#7
Apr 20, 2012, 08:27 pm Last Edit: Apr 20, 2012, 08:37 pm by Dane Reason: 1

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

Yes - it is. :)

PeterH



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.
I only provide help via the forum - please do not contact me for private consultancy.

Dane


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

Go Up