Issue with millis and serial printing

Hi there!
Thank you for taking the time to look at my problem.

I am trying to make a "dog feeding program" for a school project.

What am I trying to achieve?
I want my program to save an input from serial monitor to a variable (succeeded as far as I see).
This input value represents seconds, as in "in how many seconds do you want to feed your dog?"
I want to use this value, converted to milliseconds, to turn on a small engine for a short time.

Right now I'm using a Serial.println to represent the "engine is turned on"- part of the program.

Now my problem is that the program just spams this Serial.println, instead of waiting for an input. I tried to make the program not spam by adding && x != 0, but to no avail.

Hope this is enough info to go on, thanks for your help!

unsigned long startMillis=0;
unsigned long currentMillis;
char x;

void setup()
{
  Serial.begin(9600);
}

void loop()
{
  if (Serial.available()>0)
  {
  x = Serial.read();
  Serial.print("The dog will be fed in ");
  Serial.print(x);
  Serial.println(" seconds");
  }
  
  currentMillis = millis();
  if (currentMillis - startMillis >= ((x *1000)&& x != 0))//*1000 to convert seconds to milliseconds
  {
    Serial.println("engine is turned on");
    startMillis = currentMillis;
  }
}
if (currentMillis - startMillis >= ((x *1000)&& x != 0))

First obvious thing is that you've got the parenthesis messed up here. You've grouped the x != 0 part with (x * 1000). You should probably have the whole time comparison in one group and the x != 0 separated completely:

if ((currentMillis - startMillis >= (x *1000))&& (x != 0))
{
  x = Serial.read();
  Serial.print("The dog will be fed in ");
  Serial.print(x);
  Serial.println(" seconds");
  }

If this will be when the timing should start, then startMillis should probably be set in there.

I'd also have a boolean called needToFeedDog and set it to true when I want the dog fed and back to false once he is fed. Then the timing code goes in an if statement so it only runs if the dog needs to be fed (ie needToFeedDog is true)

Delta_G:
First obvious thing is that you've got the parenthesis messed up here. You've grouped the x != 0 part with (x * 1000). You should probably have the whole time comparison in one group and the x != 0 separated completely:

if ((currentMillis - startMillis >= (x *1000))&& (x != 0))

Thanks, it has been fixed.

{

x = Serial.read();
 Serial.print("The dog will be fed in ");
 Serial.print(x);
 Serial.println(" seconds");
 }




If this will be when the timing should start, then startMillis should probably be set in there. 

**Well, the thought is that startMillis is just set to 0 at first as stated at the top of the code. Then the if statement runs. It should update the startMillis value to the currentMillis when it's through the if statement and then feed the dog again and again after another x seconds**



if(needToFeedDog = true){
   if ((currentMillis - startMillis >= (x *1000))&& (x != 0))//*1000 to convert seconds to milliseconds
   {
   Serial.println("engine is turned on for y seconds"); //representing feeding
   startMillis = currentMillis;
   }
 needToFeedDog = false;
 }




**Also, I added the bool, but I'm not sure what to do with it? I'm planning to have the dog fed at a set time. Let's say I put in 5 seconds, then I want it looped, over and over again. Every five seconds the dog should be fed (should actually be hours, not seconds, but that's a lot of waiting to test the code)**
**If you can help me code this in a better way I would be very grateful**
:)

I'd also have a boolean called needToFeedDog and set it to true when I want the dog fed and back to false once he is fed. Then the timing code goes in an if statement so it only runs if the dog needs to be fed (ie needToFeedDog is true)

So the full code now is the following:

unsigned long startMillis=0;
unsigned long currentMillis;
char x;
bool needToFeedDog = true;

void setup()
{
  Serial.begin(9600);
}

void loop()
{
  if (Serial.available()>0)
  {
  x = Serial.read();
  Serial.print("The dog will be fed in ");
  Serial.print(x);
  Serial.println(" seconds");
  }
  
  currentMillis = millis();
  if(needToFeedDog = true){
    if ((currentMillis - startMillis >= (x *1000))&& (x != 0))//*1000 to convert seconds to milliseconds
    {
    Serial.println("engine is turned on for y seconds"); //representing feeding
    startMillis = currentMillis;
    }
  needToFeedDog = false;
  }
}

Well, the thought is that startMillis is just set to 0 at first as stated at the top of the code. Then the if statement runs. It should update the startMillis value to the currentMillis when it's through the if statement and then feed the dog again and again after another x seconds

So you want the feeding to start immediately once the user enters a number? That's fine it's just not what the display says.

if(needToFeedDog = true){

= vs ==.

I post this as addition to Delta_G's first reply to indicate some possible issues that you might encounter.

You have your (s and )s wrong in the if statement (probably after adding the x!=0)

You can try this

if (x != 0 && (currentMillis - startMillis >= x * 1000))

Next problem is that the compiler by default does calculations with 16 bit; you need to force it to use unsigned longs.

Change the first part of loop() to make the problem visible

if (Serial.available() > 0)
  {
    x = Serial.read();
    Serial.print("The dog will be fed in ");
    Serial.print(x * 1000);
    Serial.println(" milliseconds");
  }

The result will surprise you :wink: Next change it to force calculations with unsigned longs.

if (Serial.available() > 0)
  {
    x = Serial.read();
    Serial.print("The dog will be fed in ");
    Serial.print(x * 1000UL);
    Serial.println(" milliseconds");
  }

The result will still surprise you because receive a char; so if you send '1', the Arduino receives 0x31 (49 decimal) so the time will be 49 seconds. You can subtract '0' before multiplying

  if (x !=0 && (currentMillis - startMillis >= (x - '0') * 1000)) //*1000 to convert seconds to milliseconds

I'll leave the polishing to you :wink:

Delta_G:
So you want the feeding to start immediately once the user enters a number? That's fine it's just not what the display says.

Well maybe it's my poor understanding of code, but I want the following to happen:
1) user puts in a number, x, in the serial monitor (between 0-60).
2) This number is being printed with the text "The dog will be fed in x seconds".
3) after x seconds has passed, an engine will turn on, signifying that food is being poured.

if(needToFeedDog = true){

= vs ==.

fixed this

sterretje:
I post this as addition to Delta_G's first reply to indicate some possible issues that you might encounter.

You have your (s and )s wrong in the if statement (probably after adding the x!=0)

You can try this

if (x != 0 && (currentMillis - startMillis >= x * 1000))

Thanks, I changed my code to your version :slight_smile:

Next problem is that the compiler by default does calculations with 16 bit; you need to force it to use unsigned longs.

Change the first part of loop() to make the problem visible

if (Serial.available() > 0)

{
    x = Serial.read();
    Serial.print("The dog will be fed in ");
    Serial.print(x * 1000);
    Serial.println(" milliseconds");
  }



The result will surprise you ;) Next change it to force calculations with unsigned longs.
**Hm, interesting, I got a negative value** :O



if (Serial.available() > 0)
  {
    x = Serial.read();
    Serial.print("The dog will be fed in ");
    Serial.print(x * 1000UL);
    Serial.println(" milliseconds");
  }



The result will still surprise you because receive a char; so if you send '1', the Arduino receives 0x31 (49 decimal) so the time will be 49 seconds. You can subtract '0' before multiplying


if (x !=0 && (currentMillis - startMillis >= (x - '0') * 1000)) //*1000 to convert seconds to milliseconds




**I tried changing it to UL, but times 1 instead of 1000 because I want to maintain "seconds", but the program does not print my "engine is turned on for y seconds"- text. And the (x - '0') doesn't seem to do anything for me.**

I'll leave the polishing to you ;)
**Oh boy... :P**

My full code thus far:

unsigned long startMillis;
unsigned long currentMillis;
char x;
bool needToFeedDog = true;

void setup()
{
  Serial.begin(9600);
}

void loop()
{
  if (Serial.available()>0)
  {
  x = Serial.read();
  Serial.print("The dog will be fed in ");
  Serial.print(x * 1UL);
  Serial.println(" seconds");
  }
  
  currentMillis = millis();
  if(needToFeedDog == true)
  {
    if (x != 0 && (currentMillis - startMillis >= (x - '0') * 1000))//*1000 to convert seconds to milliseconds
    {
    Serial.println("engine is turned on for y seconds"); //representing feeding
    startMillis = currentMillis;
    }
    
  needToFeedDog = false;
  }
}

How long is ‘a’ seconds?
Or even ‘?’ seconds?

TheMemberFormerlyKnownAsAWOL:
How long is ‘a’ seconds?
Or even ‘?’ seconds?

Are you referring to the fact that both “a” and “?” can be sent in the serial monitor?

I’m hoping that our teacher will be nice and lets us put only numbers in :slight_smile:

What about pauses between feeding longer than 9 seconds?

I’m hoping that our teacher will be nice and lets us put only numbers in

Mine never were.

TheMemberFormerlyKnownAsAWOL:
What about pauses between feeding longer than 9 seconds?
I found a tutorial here on the forum by Robin2 called "Serial Input Basics - updated", and he has an example there. "Example 2 - Receiving several characters from the Serial Monitor", I'll try to incorporate that when I can get this basic code to work. So far, no luck on getting the program to print "the engine is turned on".

Mine never were.
Ouf, tough. My course is a pretty small one, like scratching the surface (unless you aim for high grades, which I've come to realize I will not be).