Another pair of eyes needed

I'm not sure if this the right place to post this, or even if it's the right thing to post, but I've been banging my head against the wall trying to figure out why the little green light won't go on. The code segment below is from a wireless project that's supposed to send a text message via Amazon's Simple Notification Service. There's some code that checks for a WLAN connection, and if it has been lost it does a reboot of the Adafruit CC3000 wireless breakout. Once a connection is established the code makes an NTP server query, and if successful publishes a connection message. There are 3 LEDs: a red LED that is lit at startup and gets turned off if the CC3000 is initialized successfully; a yellow LED that gets lit whle waiting for an IP from DHCP, and a green LED that gets lit, and stays lit after a successful NTP query. The problem is that if the connection is lost and is reestablished, the green LED is supposed to go out, and it is supposed to light again after the next NTP query. It all works as it's supposed to with the exception of the green LED not going back on. I can't use the serial monitor for debugging, but when I was testing it on the bench it seemed to work OK, but I added the LED code after it was installed. I concluded either it never queries the NTP server upon a reconnect, or it does but it doesn't light the light. I have been staring at this code for days, no luck. Anybody want to look this over and see what I've done wrong? I'd really appreciate the help.

The MCU is a Micro. I do get connection messages when it connects or reconnects to the WLAN, the green light comes on after a cold boot, and everything else does what it's supposed to. I've waited a couple of days for the green LED to come on after a reconnect, but it just doesn't happen.

#define     SWITCH_CHECK_SECONDS    1        // How long to wait (in seconds) between switch state checks.
#define     LOOP_COUNT              12 * 3600 / SWITCH_CHECK_SECONDS - 1 // Get NTP time every 12 hours

void setup(void) 
{
  pinMode(switchPin, INPUT_PULLUP);  // LOW on this pin triggers message generation
  pinMode(error1LED, OUTPUT);        // Set up the status lamps
  pinMode(error2LED, OUTPUT);
  pinMode(readyLED, OUTPUT);
  digitalWrite(error1LED, LOW);     // red:     Attemptiing to initialize CC3000
  digitalWrite(error2LED, LOW);     // yellow:  Attempting to connect to WLAN
  digitalWrite(readyLED, LOW);      // green:   WLAN connected and got NTP Time
  
  #ifdef DEBUG
  // Set up the serial port connection.
  Serial.begin(115200);
  delay(5000);  // Some time to get the monitor up
  Serial.println(F("SMS Mailbox v2")); 
  #endif
  // set up wireless network connection, query NTP server
  init_network();  // Sets status lights, should go from red to yellow 
}

void loop(void) 
{
  if (!cc3000.checkConnected())      // make sure we are still connected to wireless network
  {
    if (!init_network())    // try a cold start connect to WLAN
    {
      delay(15 * 1000);     // if we couldn't connect, try again later
      return;
    }
  }
  
  if (countdown == 0)     // Time to query NTP server?
  { 
    if (!queryTimeServer())    // This will turn on green LED if successful
    {
      cc3000.reboot();
      return;  // unable to get NTP server after repeated tries, give up
    }
    countdown = LOOP_COUNT;
  }
  else 
    {
      countdown--;                  // Don't poll; use math to figure current time
    }
  // Update the current time: last NTP time + timer0 - (timer0 at time of NTP query)
  unsigned long currentTime = lastPolledTime + (millis() - sketchTime) / 1000;

  if (newConnection)  //  Publish a message if we have just connected to the WLAN
  {
    snsPublish(SNS_TOPIC_ARN, "Mailbox%20on%20line", currentTime);
    newConnection = 0;
  }
 
// the  rest of my application code goes here

  delay(SWITCH_CHECK_SECONDS * 1000);  // This value pretty much sets the rep rate of loop()
}

int8_t init_network(void)
{
  cc3000.reboot();
  // Set up the CC3000, connect to the access point, and get an IP address.
  #ifdef DEBUG
  Serial.println(F("Initializing CC3000..."));
  #endif
  digitalWrite(readyLED, LOW);
  digitalWrite(error1LED, HIGH);
  digitalWrite(error2LED, LOW);
  
  if (!cc3000.begin())  // fatal
  {
    #ifdef DEBUG
    Serial.println(F("Couldn't begin()"));
    #endif
    while(1);
  }
  digitalWrite(error1LED, LOW);   // red off
  digitalWrite(error2LED, HIGH);  // yellow on
  if (!cc3000.connectToAP(WLAN_SSID, WLAN_PASS, WLAN_SECURITY)) {
    #ifdef DEBUG
    Serial.println(F("Failed to connect to AP!"));
    #endif
    return 0; // zero = failure.  also leaves yellow LED on
  }
  digitalWrite(error2LED, LOW);  // successful connect so turn off yellow LED
  #ifdef DEBUG
  Serial.println(F("Connected to Wireless Network!"));
  Serial.println(F("Request DHCP..."));
  #endif
  while (!cc3000.checkDHCP())
  {
    delay(100);
  }
  #ifdef DEBUG
  Serial.println(F("Got IP"));
  #endif
  newConnection = 0xff;  // set trigger for connect message
  return -1;  // non-zero = success
}

int8_t queryTimeServer(void)
{
  uint8_t cnt = 30;
  unsigned long t = getTime();
  while (t == 0 && cnt != 0) 
  {
    // Failed to get time, try again every 10 seconds.
    delay(10 * 1000);
    t = getTime();
    cnt--;
  }
  if (!cnt)  // give up after 5 minutes
   {
     return 0;
   }
   // sucess!
  lastPolledTime = t;
  sketchTime = millis(); 
  digitalWrite(readyLED, HIGH);    // turn on green LED
  #ifdef DEBUG
  Serial.println(F("Got NTP Time"));
  Serial.print(F("Current UNIX time: "));
  Serial.print(t);
  Serial.println(F(" (seconds since 1/1/1970 UTC)"));
  Serial.println(F("Ready..."));
  #endif
  return -1;
}

Post your code.
Post your debug output.

There are 3 LEDs: a red LED that is lit at startup and gets turned off if the CC3000 is initialized successfully; a yellow LED that gets lit whle waiting for an IP from DHCP, and a green LED that gets lit, and stays lit after a successful NTP query.

  pinMode(error1LED, OUTPUT);        // Set up the status lamps
  pinMode(error2LED, OUTPUT);
  pinMode(readyLED, OUTPUT);

If your post referred to names in your code, we'd have some idea what you were talking about. Or, if your code used names that matched your post...

  unsigned long t = getTime();

Where is this function defined? Turning on the LED requires that this function actually work.

Your code doesn't compile.

sketch_sep23a.ino: In function ‘void setup()’:
sketch_sep23a:6: error: ‘switchPin’ was not declared in this scope
sketch_sep23a:7: error: ‘error1LED’ was not declared in this scope
sketch_sep23a:8: error: ‘error2LED’ was not declared in this scope
sketch_sep23a:9: error: ‘readyLED’ was not declared in this scope
sketch_sep23a.ino: In function ‘void loop()’:
sketch_sep23a:26: error: ‘cc3000’ was not declared in this scope
sketch_sep23a:35: error: ‘countdown’ was not declared in this scope
sketch_sep23a:39: error: ‘cc3000’ was not declared in this scope
sketch_sep23a:49: error: ‘lastPolledTime’ was not declared in this scope
sketch_sep23a:49: error: ‘sketchTime’ was not declared in this scope
sketch_sep23a:51: error: ‘newConnection’ was not declared in this scope
sketch_sep23a:53: error: ‘SNS_TOPIC_ARN’ was not declared in this scope
sketch_sep23a:53: error: ‘snsPublish’ was not declared in this scope
sketch_sep23a.ino: In function ‘int8_t init_network()’:
sketch_sep23a:64: error: ‘cc3000’ was not declared in this scope
sketch_sep23a:69: error: ‘readyLED’ was not declared in this scope
sketch_sep23a:70: error: ‘error1LED’ was not declared in this scope
sketch_sep23a:71: error: ‘error2LED’ was not declared in this scope
sketch_sep23a:82: error: ‘WLAN_SSID’ was not declared in this scope
sketch_sep23a:82: error: ‘WLAN_PASS’ was not declared in this scope
sketch_sep23a:82: error: ‘WLAN_SECURITY’ was not declared in this scope
sketch_sep23a:100: error: ‘newConnection’ was not declared in this scope
sketch_sep23a.ino: In function ‘int8_t queryTimeServer()’:
sketch_sep23a:107: error: ‘getTime’ was not declared in this scope
sketch_sep23a:120: error: ‘lastPolledTime’ was not declared in this scope
sketch_sep23a:121: error: ‘sketchTime’ was not declared in this scope
sketch_sep23a:122: error: ‘readyLED’ was not declared in this scope

http://snippets-r-us.com/

The getTime() function is lifted untouched from an Adafruit sketch. I know it works for two reasons; the Amazon notification service requires a time stamp which must be accurate to 15 minutes, and also because I tested this with the serial console and DEBUG defined (but with no LEDs connected). Since there is no source of clock time other than the NTP query I think it very unlikely the time stamp would be within specs if it was just a random value. Both initialization messages and application messages are successfully published, even when the LED is off. The LEDs work as expected after a cold boot, but once a network connection is lost and reestablished the LED in question does not come on again.

The Micro I am using in this project has a damaged USB connector, so I can't easily debug with the console. I've been programming it via ICSP with a Micro running the ISP sketch. Swapping it out with another Micro would be a real PITA.

PaulS: It doesn't compile because I didn't include the #defines and the library files in my post. It successfully compiles when all the code is there. The #defines include my SSID, WEP key, the Amazon encryption key and other stuff I did not want to post.

If your post referred to names in your code, we'd have some idea what you were talking about. Or, if your code used names that matched your post...

Good point. I've been staring at this code so intently I assume everyone knows the code as well as I do. The LED in question is readyLED.

Do you have any spare pins? Can you debug using SoftwareSerial? Even a single output pin would be enough.

I suppose, but again, it’s a PITA that I’d like to avoid. But here’s something interesting. I changed LOOP_COUNT to 4 hours, the value I used when bench testing it. It reconnected this morning, and the readyLED came on an hour or two later. The counter is an unsigned long, so I don’t think I have an overflow problem or anything like that.

I changed LOOP_COUNT to 4 hours,

12 * 3600 = ?
4 * 3600 = ?

#define     SWITCH_CHECK_SECONDS    1        // How long to wait (in seconds) between switch state checks.
#define     LOOP_COUNT              4 * 3600 / SWITCH_CHECK_SECONDS - 1 // Get NTP time every 4 hours

Which is, if I did my arithmetic properly, is 4 hrs - 1 second. The -1 second comes from the original Adafruit sketch, and I think it's spurious, but I suppose it causes no harm. The actual value is not really critical. It just keeps the sketch from hitting the NTP servers with a flood of requests.

The value SWITCH_CHECK_SECONDS is used as a blocking delay at the end of loop(). It's not really critical either. I originally had it set for 5 seconds, but my letter carrier is pretty quick and I missed a couple of events.

if I did my arithmetic properly, is 4 hrs - 1 second.

It may well be.

But 12 * 3600 is not twelve hours.

60 seconds/hr * 60 minutes/hr * 12 should be 12 hours in seconds, no? The loop runs once per second (more or less) and decrements a counter that is initialized with that value, so it should reach zero in 12 hours. I did mention a disclaimer about correct arithmetic, so....

60 seconds/hr * 60 minutes/hr * 12 should be 12 hours in seconds, no?

On an architecture with 32 bit "int"s, it would be, yes.
Sadly, you’re in 16 bit.

Now I am truly confused. An unsigned 16 bit int has a max value of 64K. 12*3600 is 43,200, which I think is less than 65,535. Of course, since I did not include the declaration of countown in my code snippet, I suppose you would not have known it is declared as such, unless you were reading my mind.

An unsigned 16 bit int has a max value of 64K. 12*3600 is 43,200, which I think is less than 65,535.

Perfectly true.
However, a 16 bit "int" has a range from -32768 to +32767

unsigned long countdown = 0;

Even so, I think there's something about what you said. All the values I used during testing were less than 32767. An unsigned long in this environment is 32 bits, which far exceeds those values, but there is something suspicious going on here. I just wish I knew what it is.

Well spotted, AWOL.

Even so, I think there's something about what you said. All the values I used during testing were less than 32767. An unsigned long in this environment is 32 bits, which far exceeds those values, but there is something suspicious going on here. I just wish I knew what it is.

The "it" is 16 bit signed arithmetic.

Barry914:

unsigned long countdown = 0;

Even so, I think there's something about what you said. All the values I used during testing were less than 32767. An unsigned long in this environment is 32 bits, which far exceeds those values, but there is something suspicious going on here. I just wish I knew what it is.

Yes well if you read my link you will see that assigning to unsigned long (or testing against it) does not force the RHS to be unsigned long.

Well, that was an eye opener AWOL & Nick. I learned three things:

  1. I now know how the compiler treats literals.
  2. If i am going to ask a question, I probably should pay close attention to the answer.
  3. Stubbornness is not a positive attribute.

I appreciate the help. Thank you very much.

+1 All... Great lesson

Doc