SparkFun Forums 

Where electronics enthusiasts find answers.

For the discussion of Arduino related topics.
By mahimna27
#199202
Hello,

I am just joining the forum and the programming world as well. I read the "how to post" article and I am truly sorry if i am making any similar mistakes. Please let me know about my question asking style's improvements or any such issues, I will definitely try and improve.

About the question then,
I am doing a LED lighting program for rooms,with option for small or large room to be selected using a toggle switch.
According to that input, for LOW input room size is small and for HIGH it is large. I want initially to keep an AC bulb ON for 30 sec if small room is selected and 60 sec if large room is selected. After the initial small OR large rooms' respective light ON time, I want to start the blinking mode for the AC bulb irrespective of the selected room and keep in the toggle/blinking mode ( 10 sec ON and 10 sec OFF) till the power supply is cut.

After multiple trials and learning a tad bit thing every-time from my mistakes I was able to avoid the use of interrupt (used for monitoring the room size switch, if room size changed, I want to start the execution all over again) and delay() by the millis() function and the reference of "blink without delay" example. I am using arduino uno and a relay module to control the AC bulb.( RELAY MODULE I AM USING SWITCHES TO NORMALLY-OPEN WHEN GND IS APPLIED AS A SIGNAL. SO TO TURN ON THE BULB, " digitalWrite(ledPin, LOW); " IS USED AND TO TURN OFF " digitalWrite(ledPin, HIGH); " IS USED.)

Though according to my limited knowledge the code was perfect, it was not behaving so. As I have mentioned earlier the expected sequence of execution is; RUN FOR 30/60 sec - START BLINKING ( 10 sec ON and 10 sec OFF) FOR INFINITY UNLESS ROOM SIZE IS CHANGED - IF ROOM SIZE CHANGED,START THE SEQUENCE FROM FIRST STEP. My program is not portraying this role exactly.

When I was testing it on hardware for the HIGH input ( large room), it completed its first 60 sec turn ON fine as always and started toggling, which was also previously tested and always work fine. But after entering in the toggling/blinking part, after 10th blinking which was at light ON state (supposedly for 10 sec only ) , it didn't went OFF for 70 sec which it should have after 10 sec only and should have stayed OFF for 10 sec and blah blah. After that everything worked fine , toggling was again back to normal with 10 sec ON and 10 sec OFF. But same happened again after 25th and 70th switching, it remained ON for 70 sec at those times as well. So I checked it for several times and each time it was at different switching cycle, for two times it went wrong after the 31st reading sometimes after 107th toggling sometimes 97th etc (same 70 sec at each time), and sometimes multiple times in 100 switching. Same is the case for LOW input ( small room), it also behaves wrongly sometimes and not get toggled for 40 sec( 10 sec + 30 sec).

The two super weird things are, first, whenever it misbehaves, most of the times relay makes its "tuck" sound of internal throw switching right after 10 sec but light still remains ON and continues to be ON for NEXT ADDITIONAL 60 sec and hence total 70 sec for large room and 40 for small room. secondly, the abnormal behavior only happens when the toggle cycle of "bulb ON for 10 sec " is happening. It has never stayed OFF for 40 sec (30+10), it only happens in when the bulb is ON and should have gone OFF after 10 sec but doesn't.

I am really confused, I checked the unsigned long's range but it is fine with range more than 49 days, so why this sudden and random misbehaving is really out of my understanding capability. I have used boolean for flags to avoid garbage values as well.
Please let me know where have I went wrong in the code and why is this happening. I really want the solution for this problem to understand it thoroughly. your help is really appreciated as now its been several days I am stuck with this issues with no way out.

I am attaching the code and similar circuit diagram used for simulation with the post. (the relay module I am using in the actual circuit switches to NORMALLY OPEN on the GND signal along with Vcc and GND provided from arduino's Vcc and GND. As to keep the code same in the circuit simulation I have attached the bulb on NORMALLY OPEN to portray the active low behavior of my relay module.)


Image

Code: Select all

const int ledPin = 10;
const int roomSize = 12;       // the number of switch input pin

const long interval = 10000;  // interval at which to blink / toggle

// Variables will change:
//bool ledState = false;           // ledState used to set the LED
int ledState = LOW;   

bool initialisation = false;     // basic initialisation completion Flag
bool initialDone = false;        // initial mode completion Flag
bool toggleDone = false;         // toggle mode completion Flag
bool room = false;               // save the room size selected

// Generally, you should use "unsigned long" for variables that hold time
// The value will quickly become too large for an int to store
unsigned long previousMillis = 0;        // will store last time LED was updated

void setup()
{

  // put your setup code here, to run once:
  pinMode(roomSize, INPUT);
  pinMode(ledPin, OUTPUT);

}

void loop()
{
  // put your main code here, to run repeatedly:

  unsigned long currentMillis = millis();

  if (digitalRead(roomSize) == LOW) {
    if (room == true) {   // if suddenly room size switch is changed to small room
      room = false;         // then start all over again with initial mode
      initialisation = false;
      initialDone = false;
      toggleDone = false;
      digitalWrite(ledPin, HIGH);
      previousMillis = currentMillis;
      ledState = LOW;
    }
  }

  if (digitalRead(roomSize) == HIGH) {
    if (room == false) {    // if suddenly room size switch is changed to Large room
      room = true;         // then start all over again with initial mode
      initialisation = false;
      initialDone = false;
      toggleDone = false;
      digitalWrite(ledPin, HIGH);
      previousMillis = currentMillis;
      ledState = LOW;
    }
  }

  if (initialisation == false) {
    digitalWrite(ledPin, LOW);
    initialisation = true;
  }

  if (currentMillis - previousMillis >= 30000 && toggleDone == false) {
    if (digitalRead(roomSize) == LOW) {
      digitalWrite(ledPin, HIGH);
      initialDone = true;
    }
  }

  if (currentMillis - previousMillis >= 60000 && toggleDone == false) {
    if (digitalRead(roomSize) == HIGH) {
      digitalWrite(ledPin, HIGH);
      initialDone = true; 
    }
  }

  if (currentMillis - previousMillis >= interval && initialDone == true) {
    // save the last time you blinked the LED

    toggleDone = true;      // this will avoid reassigning of values in intial mode
    previousMillis = currentMillis;

    //digitalWrite(ledPin, ledState);
    // if the LED is off turn it on and vice-versa:
    if (ledState == LOW) {
      ledState = HIGH;
    } else {
      ledState = LOW;
    }

    // set the LED with the ledState of the variable:
    digitalWrite(ledPin, ledState);

  }
}

Kind Regards,
Mahimna
By paulvha
#199206
your code has a lot of room for optimization on different parts.

When you are just at the end of blinking-ON 10 seconds and toggle the switch, you will get 30 or 60 seconds added to your blinking-on (making it 40 or 70 seconds).

Is your switch hard connecting to HIGH and LOW and/or use a pull-up resistor ? Consider implementing a de-bounce routine. (in a subroutine : read roomswitch, if changed, wait 500ms, read again to make sure the switch has really changed)
What happens if interval is defined as unsigned long (instead of const long)?
regards,
Paul
By mahimna27
#199211
Hello Paul,

Thank you for the prompt reply. I have had attached the circuit diagram but it seems to be failed, I will link it here again.

I am not exactly sure what do you mean by,
Is your switch hard connecting to HIGH and LOW and/or use a pull-up resistor ?
I have used the pull down resistor of 10k ohm, and the switch is directly interfaced to the arduino pin (if that's you are referring by the term " hard connecting" ) .

Yes THE DEBOUNCING, yesterday only after pondering, I thought that the unpredictable behavior maybe the result of loose or erroneous connection and as i dug into it I came across this de-bounce problem. The unpredictable and multiple fault occurring can be the result of the spikes getting processed lately (of course this is my naive brain's thinking and can be very wrong, but just sharing what I felt).

About declaring the interval in const long, because it was declared that way in the arduino official example and as my interval was not massive (only 10 sec) I didn't change it to unsigned long. I am not sure about what difference will it serve but I will surly try it as well.

About my sloppy programming skills, I am sorry for that. Here I am attaching the new code where I have tried to be optimized. Feel free to suggest any further optimization or programming practices I should be inculcating.
Code: Select all

const int  roomSize = 12;                   //input
const int ledPin = 10;                      //output

unsigned long interval = 10000;             //toggle interval

bool  initialDone = false;                  //flag

int ledState = LOW;                         //togglin variable
int previousRoom = LOW;                     //for room change detection and De-Bounce

unsigned long initialTime = 0;              //time for which initial mode to be run
unsigned long previousMillis = 0;           //for toggle without delay()

void setup() {
  
  // put your setup code here, to run once:
  pinMode(roomSize, INPUT);
  pinMode(ledPin, OUTPUT);
}

void loop() {

  unsigned long currentMillis = millis();
  int currentRoom  = digitalRead(roomSize);

  if (currentRoom == LOW) {
    if (currentRoom != previousRoom ) {
      delay(500);                                   //De-Bounce checking
      if (digitalRead(roomSize) != previousRoom ) {
        previousMillis = currentMillis;
        initialDone  = false;
        ledState = LOW;
        previousRoom = LOW;
      }
    }
    initialTime = 30000;
  }

  if (currentRoom == HIGH) {
    if (previousRoom != currentRoom ) {
      delay(500);                                 //De-Bounce checking
      if (digitalRead(roomSize) != previousRoom ) {
        previousMillis = currentMillis;
        initialDone  = false;
        ledState = LOW;
        previousRoom = HIGH;
      }
    }
    initialTime = 60000;
  }

  if (initialDone  == false) {
    if (currentMillis - previousMillis >= initialTime) {
      digitalWrite(ledPin, LOW);    //turn off
      initialDone  = true;
    } else {
      digitalWrite(ledPin, HIGH); //turn ON
    
    }
  }

  if (initialDone  == true) {
    if (currentMillis - previousMillis >= interval) {
      
      // save the last time you blinked the LED
      previousMillis = currentMillis;

      // if the LED is off turn it on and vice-versa
      if (ledState == LOW) {
        ledState = HIGH;
      } else {
        ledState = LOW;
      }

      // set the LED with the ledState of the variable
      digitalWrite(ledPin, ledState);
    
    }
  }
}

Thank you for your help, and looking forward to hear from you soon about this updated code.

Kind Regards,
Mahimna
You do not have the required permissions to view the files attached to this post.
By paulvha
#199212
A relay can easily cause spikes when power is applied or removed. Hence it can cause false triggers/detection. Not sure you still have the same issues, but couple of things to test:
1. when a room changed has been detected (after de-bounce) include a Serial.println() to show which room was detected. At least you start better understanding what is happening.
3. instead of a relay, connect a led and see whether that makes a difference.
4. if spikes are your issue, in many situations people put a diode across the relais coil (some have relays have this standard included). The negative side (with the ring) should face the + (pin 10). Maybe that reduces the spike. You could also try to remove the lamp (as the spikes could come from the contact moving from open to close, or change in current.
regards,
Paul
By mahimna27
#199216
Hello,

I am using the relay shield (it has the optocoupler and diodes already on it). I will try this "connect a LED option" apart from those have you found any flaw in the code? optimization or logic wise. I am really out of options with the problem so if you can come up with any possible solution please let me know.
Thank you for prompt reply,
Mahimna
By paulvha
#199224
I looked at the code and gave it a little update. Prevent writing the same code twice and better to breakdown in smaller subroutines for easier reading, debugging and maintenance later on.

Any progress so far on your project ?
regards,
Paul

Code: Select all
const int  roomSize = 12;                   //input
const int ledPin = 10;                      //output

unsigned long interval = 10000;             //toggle interval

bool  initialDone = false;                  //flag

int ledState = LOW;                         //togglin variable
int previousRoom = LOW;                     //for room change detection and De-Bounce

unsigned long initialTime = 30000;          //time for which initial mode to be run
unsigned long previousMillis = 0;           //for toggle without delay()

void setup() {
  Serial.begin(9600);
  pinMode(roomSize, INPUT);
  pinMode(ledPin, OUTPUT);
}

void loop() {

  unsigned long currentMillis = millis();

  // check whether room switch changed
  if (read_room_switch() == true)
  {
      previousMillis = currentMillis;
      initialDone  = false;
      ledState = HIGH;

      if (previousRoom == LOW)    // set correct timeout
        initialTime = 30000;
      else
        initialTime = 60000;
        
      digitalWrite(ledPin, HIGH); // turn ON
  }

  
  if (initialDone  == false) {   // perform initial time out
    if (currentMillis - previousMillis >= initialTime) {
      digitalWrite(ledPin, LOW);    //turn off
      initialDone  = true;
    }
  }
  else {                         // perform blinking
    if (currentMillis - previousMillis >= interval) {

      // save the last time you blinked the LED
      previousMillis = currentMillis;

      // if the LED is off turn it on and vice-versa
      ledState = !ledState;

      // set the LED with the ledState of the variable
      digitalWrite(ledPin, ledState);

    }
  }
}

/* check whether room switch has changed 
 * return:
 *  true : changed
 *  false: no change
 */
bool read_room_switch()
{
    int currentRoom  = digitalRead(roomSize);

    if (currentRoom != previousRoom ) {

        delay(500);     // de-bounce

        currentRoom  = digitalRead(roomSize);

        if (currentRoom != previousRoom )
        {
            
            if(0)   // debug : changed 0 to 1
            {
              Serial.print(F("switched room from "));
              Serial.print(previousRoom);
              Serial.print(F(" to "));
              Serial.println(currentRoom);
            }
            
            previousRoom = currentRoom;
            return(true);
        }
    }

    return(false);
}