Millis question

Ive been trying to turn a light on for 1 second when a 1 is sent through the Serial Monitor.But when i send a 1 the led stays on forever.My logic and code is below.

Logic:
When a 1 is recieved, start Millis
while Millis is less then interval turn on light
when interval has passed turn off light

const int ledpin = 13;
unsigned long currentmillis;
long previousmillis = 0;
long interval = 1000;

void setup()
{
  Serial.begin(9600);
  pinMode(ledpin,OUTPUT);
}
void loop()
{  
   LEDandMillis();
}

void LEDandMillis()
{
if(Serial.available())
{
   char x = Serial.read - 48;  
if(x == '1')
  {
    currentmillis = millis();  //assign time variable to currentmillis
    while(currentmillis - previousmillis < interval)  //while currentmillis is lees then 1 second have led ON
    {
      
      digitalWrite(ledpin,HIGH);
    }
    else  //when currentmillis pass's the interval turn OFF
       {
           digitalWrite(ledpin,LOW);
       }
 previousmillis = currentmillis;  //reassign value of currentmillis to previousmillis
}
}

you may as well simply use delay()

Have you posted the right program because it does not compile for a number of reasons.

Sorry i was at school and used my phone, heres the code. I would like to practice with millis function instead of the delay which causes the program flow to completly stop.

const int ledpin = 13;
unsigned long currentmillis;
long previousmillis = 0;
long interval = 1000;

void setup()
{
  Serial.begin(9600);
  pinMode(ledpin,OUTPUT);
}
void loop()
{  
   LEDandMillis();
}

void LEDandMillis();
{
if(Serial.available())
{
   char x = Serial.read() - 48;  
   if(x == '1')
  {
    currentmillis = millis();  //assign time variable to currentmillis
    while(currentmillis - previousmillis < interval)  //while currentmillis is lees then 1 second have led ON
    {
      
      digitalWrite(ledpin,HIGH);
    }
    
 previousmillis = currentmillis;  //reassign value of currentmillis to previousmillis
  }
 }
}

I don't see any line like digitalWrite(ledPin, LOW); to turn off the led whe the time expires.

Also previousMillis abd interval should be defined as unsigned long.

...R

I would like to practice with millis function instead of the delay which causes the program flow to completly stop.

You're just replacing the while loop in delay with your own while loop. You don't want any blocking loops.

Still wrong

void LEDandMillis();

Also

      while (currentmillis - previousmillis < interval) //while currentmillis is lees then 1 second have led ON
      {

        digitalWrite(ledpin, HIGH);
      }

How will this while loop ever end when nothing within it changes ?

Thanks for the replies everyone. Sorry for leaving some stuff out.But ive been reading up on the Do and While statements that were posted above.

But the LED still stays on,i rearranged the code to just start blinking on and off for 1 second w/o a certain value needed as I tried earlier with typing 1 into serial monitor.But the LED still stays on forevr.

const int ledpin = 13;
unsigned long currentmillis;
long previousmillis = 0;
long interval = 1000;

void setup()
{
  Serial.begin(9600);
  pinMode(ledpin,OUTPUT);
}
void loop()
{
       digitalWrite(ledpin,HIGH);//led ON
   do {
        currentmillis = millis(); //do this till While Cond. doesnt equal true
                 
    }  while(currentmillis - previousmillis > interval);  //loop while currentmillis is less then 1 second have led ON
    
    digitalWrite(ledpin,LOW);
    //led OFF
    previousmillis = currentmillis;

A while loop possibly never executes the body of the loop. A do...while loop executes the body of the loop at least once. Both can block.

what would you suggest for using the millis function

geryuu11: what would you suggest for using the millis function

Not blocking.

Let me give you something to gnaw on.

This does all sorts of simultaneous flashing and debounces a button to toggle. I think it is a good exercise from which to build other things.

// Blink without "delay()" - multi!

const int led1Pin =  13;    // LED pin number
const int led2Pin =  10;
const int led3Pin =  11;
int led1State = LOW;        // initialise the LED
int led2State = LOW;
int led3State = LOW;
unsigned long count1 = 0;   // will store last time LED was updated
unsigned long count2 = 0;
unsigned long count3 = 0;

// Have we completed the specified interval since last confirmed event?
// "marker" chooses which counter to check 
boolean timeout(unsigned long *marker, unsigned long interval) {
  if (millis() - *marker >= interval) { 
    *marker += interval;    // move on ready for next interval
    return true;       
  } 
  else return false;
}

// Deal with a button read; true if button pressed and debounced is a new event
// Uses reading of button input, debounce store, state store and debounce interval.
boolean butndown(char button, unsigned long *marker, char *butnstate, unsigned long interval) {
  switch (*butnstate) {               // Odd states if was pressed, >= 2 if debounce in progress
  case 0: // Button up so far, 
    if (button == HIGH) return false; // Nothing happening!
    else { 
      *butnstate = 2;                 // record that is now pressed
      *marker = millis();             // note when was pressed
      return false;                   // and move on
    }

  case 1: // Button down so far, 
    if (button == LOW) return false; // Nothing happening!
    else { 
      *butnstate = 3;                 // record that is now released
      *marker = millis();             // note when was released
      return false;                   // and move on
    }

  case 2: // Button was up, now down.
    if (button == HIGH) {
      *butnstate = 0;                 // no, not debounced; revert the state
      return false;                   // False alarm!
    }
    else { 
      if (millis() - *marker >= interval) {
        *butnstate = 1;               // jackpot!  update the state
        return true;                  // because we have the desired event!
      }
      else 
        return false;                 // not done yet; just move on
    }

  case 3: // Button was down, now up.
    if (button == LOW) {
      *butnstate = 1;                 // no, not debounced; revert the state
      return false;                   // False alarm!
    }
    else { 
      if (millis() - *marker >= interval) {
        *butnstate = 0;               // Debounced; update the state
        return false;                 // but it is not the event we want
      }
      else 
        return false;                 // not done yet; just move on
    }
  default:                            // Error; recover anyway
    {  
      *butnstate = 0;
      return false;                   // Definitely false!
    }
  }
}

void setup() {
  pinMode(led1Pin, OUTPUT);      
  pinMode(led2Pin, OUTPUT);      
  pinMode(led3Pin, OUTPUT);      
}

void loop() {
  // Act if the latter time (ms) has now passed on this particular counter,
  if (timeout(&count1, 500UL )) {
    if (led1State == LOW) {
      led1State = HIGH;
    }
    else {
      led1State = LOW; 
    } 
    digitalWrite(led1Pin, led1State);
  } 

  if (timeout(&count2, 300UL )) {
    if (led2State == LOW) {
      led2State = HIGH;
    }
    else {
      led2State = LOW; 
    } 
    digitalWrite(led2Pin, led2State);
  } 

  if (timeout(&count3, 77UL )) {
    if (led3State == LOW) {
      led3State = HIGH;
    }
    else {
      led3State = LOW; 
    } 
    digitalWrite(led3Pin, led3State);
  } 
}

You might try going back to the beginning nd have a look at the sample sketches blink v blink without delay, to understand how the delay v the “millis” method works, then apply the lesson to your own sketch

I'm inclined towards @RMurphy195's advice. The original problem seems to have been lost from view.

This simple code does what the OP wants - I have tested it. I have deliberately wriiten it as a lot of very small functions to try to make the different activities clear. Also, the individual functions could be extended for other purposes. No part of the code blocks or waits for anything else to finish. I have extended the delay to 5000 msecs and I have added code (in stopLed) which will immediately turn the LED off if you send a 2 from the serial monitor. This illustrates that the code is not blocking.

const int ledpin = 13;
unsigned long currentmillis;
unsigned long previousmillis = 0;
unsigned long interval = 5000;
byte inputNum;
boolean ledState = LOW;

void setup()
{
  Serial.begin(9600);
  pinMode(ledpin,OUTPUT);
}

void loop()
{  
    currentmillis = millis();
    readSerial();
    startLed();
    stopLed();
    switchLed();
}

void readSerial() {
    if(Serial.available() > 0) {
        inputNum = Serial.read();
    }
}

void startLed() {
    if (ledState == LOW && inputNum == '1') {
        ledState = HIGH;
        previousmillis = currentmillis;
    }
}

void stopLed() {
    if (currentmillis - previousmillis > interval) {
        ledState = LOW;
    }
    if (inputNum == '2') {
        ledState = LOW;
    }
}

void switchLed() {
    digitalWrite(ledpin, ledState);
}

...R