While loop - works inverted ?

I've been fiddling a while loop and they seem to me to 'invetered'

while( digitalRead(inputPin) == HIGH );
{
Serial.println("Message");
}

With the input pin high (+5V) I thought the loop should printout the 'message' - The test expression would be TRUE so should print the text BUT it seems to work the other way print when LOW weird or is it me!

You have mistakenly included a semicolon after the condition.

while (condition) [glow];[/glow]
{
   task;
}

This means it does nothing while HIGH, and as soon as it's not high, it can continue to do the following code. Your code will print a message once. (If this is in your loop() function, then you will get one message per loop().)

If you remove the semicolon, you will get many printouts for as long as you hold the pin high.

Ah, thank you!!! that explans a LOT :slight_smile:

that's what I thought it should do - print loads of messages until the input pin goes LOW = FALSE = exit to rest of code...

No problem.

In terms of programming style, I find it important to make a difference between C keywords (if, while, for, return, ...) and C function names.

For keywords, I suggest you put a space after the keyword, and don't put a semicolon right after a closing parenthesis. They aren't functions. They don't calculate a value and return a value to be used in an expression. So make them look different.

if[glow]  [/glow](!favorable())
    task();
while[glow]  [/glow](isReady())
{
    task();
}
for[glow]  [/glow](int i = 0; i < 5; i++)
{
    task();
}

And so on.

In the rare case you really want to make a while statement with no task at all, put the semicolon on the next line to make it obvious to the reader (that's you in two years) that it was intentional.

while  (!isReady())
[glow]     [/glow];

Good idea - I was beginning to think I was going mad ! in my C book it makes no mention on the ;- I love these undocumented command additions

This means it does nothing while HIGH, and as soon as it's not high, it can continue to do the following code. Your code will print a message once. (If this is in your loop() function, then you will get one message per loop().)

In the same way is there a way of doing nothing while LOW ? - just a thought :slight_smile:

Hm, not a very good book then, eh?

The syntax for any of the conditional keywords above:

keyword  [glow]([/glow] expression [glow])[/glow]
    statement [glow];[/glow]

If you want to do more than one thing as a statement, you use curly braces INSTEAD of the usual semicolon after the statement.

keyword  [glow]([/glow] condition [glow])[/glow]
[glow]{[/glow]
    statement [glow];[/glow]
    statement [glow];[/glow]
    statement [glow];[/glow]
[glow]}[/glow]

is there a way of doing nothing while LOW?

Any expression used as a condition in the parentheses works the same way. The C language itself knows nothing about pins, that's what the Arduino functions and constants are for. All of the following are equivalent:

while ([glow]![/glow]digitalRead(pin)) ...
while (LOW == digitalRead(pin)) ...
while (digitalRead(pin) == LOW) ...
while (digitalRead(pin) != HIGH) ...

And so on. I used the ... to save space, it's not meant as code. The ! means "not," so it's while digitalRead() is NOT returning a zero value. The word LOW is an Arduino constant that just means zero.

If you want to do nothing until the condition is true, see my example above with a lonely semicolon on its own line.

Seems to work ok now, but I think there is better of doing what I would like to - part of my code

  void loop(){
  while  ( digitalRead(inputPin) == HIGH   ) // wait for falling edge (LOW)
  {
  }
  start = millis();                        // start counting ms's  (between falling edge and another including the LOW of the pulse)
  digitalWrite(ledPin,0);                  // turn off led - 
  
  while  ( digitalRead(inputPin) ==LOW  ) // trap in LOW period and wait again for rising edge (HIGH)
  {
  }
  
  while  ( digitalRead(inputPin) ==HIGH ) // wait again for falling edge (LOW)
  {
  }
  duration = millis() - start;             // add up count
  digitalWrite(ledPin,1);                  // turn on led   
  
   // display results on LCD and serial port

:smiley:

you can measure the duration of a pulse using the pulseIn function, there is an explanation and example code here: http://www.arduino.cc/en/Reference/PulseIn

Thanks Mem, that looks like more like what I need !

I think there's confusion on the pulseIn() function. According to the docs, it measures the portion of the signal that is HIGH or LOW, not the total period of the waveform which is what myozone drew in the picture.

What you have is good, myozone. If you need to do it often, make it a function that measures a waveform in that fashion, and then you can call it with a pin number for an argument.

unsigned long periodIn(int pin)
{
    byte prior = digitalRead(pin);
    while (digitalRead(pin) == prior)
        ;
    unsigned long start = millis();
    while (digitalRead(pin) != prior)
        ;
    while (digitalRead(pin) == prior)
        ;
    return millis() - start;
}

This will wait for the first EDGE TRANSITION (in either direction), then measure the time it will take to make the SAME edge transition again.

halley is quite correct, pulseIn only measures one part of the cycle. But digitalRead introduces significant error because its slow. FWIW, here is a routine modified from pulseIn that does measure the total period. I haven't tested it so it may need some tweeking

#include "pins_arduino.h"

unsigned long pulsecycleIn(uint8_t pin,  unsigned long timeout)
{
        // returns the total period for a pulse cycle   
      // cache the port and bit of the pin in order to speed up the
      // pulse width measuring loop and achieve finer resolution.  calling
      // digitalRead() instead yields much coarser resolution.
      uint8_t bit = digitalPinToBitMask(pin);
      uint8_t port = digitalPinToPort(pin);
      unsigned long width = 0; // keep initialization out of time critical area
      
      // convert the timeout from microseconds to a number of times through
      // the initial loop; it takes 16 clock cycles per iteration.
      unsigned long numloops = 0;
      unsigned long maxloops = microsecondsToClockCycles(timeout) / 16;
      uint8_t state = *portInputRegister(port) & bit; // get the current state
      // wait for the pulse to start
      while ((*portInputRegister(port) & bit) == state)
            if (numloops++ == maxloops)
                  return 0;
      
      // wait for the pulse change state
      while ((*portInputRegister(port) & bit) != state)
            width++;
        // wait for the pulse to terminate 
        while ((*portInputRegister(port) & bit) == state)
            width++;
      // convert the reading to microseconds. The loop has been determined
      // to be 10 clock cycles long and have about 16 clocks between the edge
      // and the start of the loop. There will be some error introduced by
      // the interrupt handlers.
      return clockCyclesToMicroseconds(width * 10 + 16); 
}

Thanks, mem. I knew digitalRead() has a lot of overhead on each call, but I'm not comfortable enough with the pin registers to write directly. Thanks for the improvement. Your routine requires a timeout and only times out on the initial edge wait, which is tricky to document. Maybe we could spruce it up and get a basic routine like that into the standard libs.

I think pulseIn behaves the same, both routines only time out while waiting for the pulse to start.

Please feel free to spruce this up.