Not understanding toggle code

This is pretty lame but if someone could explain how this toggle function works for me then it would be great. Right now i'm not sure why it's working.

if (state == LOW)
{
state = HIGH;
}

else
{
state = LOW;
}

digitalWrite(pin, state);
delay(1000);

Start loop

1 Check teh state
1 a ) is it LOW ? If yes, put it high
1 b ) If it is not,( then it is high, so) set it low

2 put the led to the state set

  1. wait a second

end loop

start loop again ....

It is clear now. Thanks for the help.

You could also just write:-
state = !state;
to toggle it.

Or

digitalWrite(pin, state^= HIGH);
delay(1000);

:wink:

While this :

state = !state;

and this:

may work today, it is bad programming practice and non portable as it is not guaranteed to work since the API definition
is defined using HIGH and LOW and the API makes no promises or guarantees as to what
the actual value of HIGH and LOW are.
While today HIGH is 1 and LOW is 0, so those code examples will work, they make an assumption as to the
value of the HIGH and LOW that is beyond the API definition.
The API does not guarantee that HIGH and LOW are any particular value
it merely states to use "HIGH" when you want the pin to be a logic level high and "LOW"
when you want the pin to be a logic level low.

It is best to always stick with the documented parameters which in this case for digitalWrite() is LOW and HIGH
rather than try to take advantage of knowing internal implementation details.

Using an API in way outside of the way it is defined, can burn you should the underlying
internal details change in the future.

So the first/original example is what I'd recommend since it is
the only one that uses the digitalWrite() API as it is defined and would not be affected
should underlying internal details ever change in the future.

--- bill

bperrybap:
may work today, it is bad programming practice and non portable as it is not guaranteed to work since the API definition
is defined using HIGH and LOW and the API makes no promises or guarantees as to what
the actual value of HIGH and LOW are.

I agree.

Personally I think it's unlikely that the Arduino runtime constants would ever be changed because there is so large a population of people who have followed bad practice and coded in assumptions about what the values are. Still, it's conceivable. In any case it's poor practice to code like that, and the more widespread this poor practice get, the more likely it is that somebody will carry that over to some other environment where they will get burned

Is it conceivable that in any implementation of any language the HIGH will not be the logical inverse of LOW? These are hardware concepts and these nimby pimby software types had better get their head round that fact!

Grumpy_Mike:
Is it conceivable that in any implementation of any language the HIGH will not be the logical inverse of LOW? These are hardware concepts and these nimby pimby software types had better get their head round that fact!

Complete and total agreement!

LOW and HIGH "may be 0 and 1 now" but they have been since the computing stone age and are based on DIGITAL HARDWARE that won't be changing any time soon.

How many programmers does it take to change a lightbulb? Can't be done, it's a hardware problem! Get a tech, weenie!

@bill: would you dare to show us how you would implement this with good programming style? I wonder how you would implement this?

IMHO the IF approach is not really good because it bloats the code in the direction of making of harder to read. But I think this is a matter of taste.

If you do not like my approach, what about

digitalWrite(pin, state^= (HIGH^LOW));

Harder to read but would not rely on the specific values of LOW and HIGH. But wait: it relies on HIGH and LOW being encoded in bits...

Or, to be "politically" correct but obscure it further, you could write this:

state = state==HIGH ? LOW : HIGH;

]:slight_smile:

Pete

Even more political correctness and confusion:

#define default LOW

state = state==HIGH ?LOW: state==LOW? HIGH: default;

To ensure that the out of range case can be biased according to needs ]:smiley:

But then again maybe

state = state==HIGH ?LOW: state==LOW? HIGH: state;

would be more political correct as it will not change an undefined state :wink: Probably for maximum political correctness interrupts must be switched of while processing this. So:

cli();
state = state==HIGH ?LOW: state==LOW? HIGH: state;
sei();

For maximum incorrectness and confusion go here: Multiple Modes Without Flicker | Blinkenlight :fearful:

Guys,
It isn't a matter of political correctness or programming style issue
which is subject to personal tastes and opinion but a matter of following the API specification.
(programming style != programming practices)

The API for digitalWrite() says HIGH and LOW. It does not say 1 or 0 or zero or non-zero.
To use anything other than HIGH and LOW or even depending on HIGH and LOW
being bit opposites of each other
is using using the API or the API defines in a way that is not specified or
guaranteed to work.

Grumpy Mike and GoForSmoke as far as your comments go, I actually am a hardware type.
(I actually do both)
I have worked on many embedded systems and led many development teams
over the past 30 years, some of which have been used in Aircraft.
I have also designed several ASICs,
some that were used in 10's of millions of DSL modems,
so yes I am quite familiar with hardware concepts and I'm no rookie
with respect to these concepts.

I have had to deal with bugs created by other developers that were doing
the very kinds of things done in some of the examples in this thread.
It is not fun to track down these type of bugs.

And while is very likely that LOW and HIGH will be logical opposites
(if LOW is 0) there are many examples like this one:

state = !state;

That make the assumption that LOW is 0 and HIGH is 1.

The reason for having an API is to isolate the application from the internal details
of an implementation. This not only has the potential to make the interface easier
to use and understand but also allows the the implementation to change in the future
to extend functionality while still providing full backward compatibility.

By using the API or its defines in ways that are not defined, you are taking
a risk by taking advantage of knowledge of the internal implementation which may change out
from under you in the future.

We had a very similar lengthy discussion on this very same topic a few months ago.
And I'll site the same hypothetical example I did then..

Lets suppose that Arduino advances to a chip that supports real analog output vs just PWM.
So now things have to change a bit to support PWM vs true analog ouput.

Now assume that it becomes desirable to extend the digitalWrite() function to
support PWM as well as simple HIGH/LOW.
IF people had followed digitalWrite() API by always using HIGH and LOW,
Then digitalWrite() could be made "smarter" or extended to support PWM, by simply changing the value of HIGH
from 1 to 255.

That way the existing dititalWrite(pin, val) API could still work since LOW would give you a solid low pin and HIGH would
give you solid high pin but values other than HIGH & LOW would give you a PWM signal on the pin.

Those people that always used HIGH and LOW would see no difference or any issues for setting pin outputs
to solid low or solid high while those that assumed that
HIGH was 1 or took advantage that digitalWrite() would set a pin high for any non zero value
(which is what digitalWrite() does today) would have their code break when using the newer implementation.

It is also theoretically possible that there could be another implementation that used
LOW and HIGH as negative values or maybe LOW is negative and HIGH is a number that is very large
to indicate something different for solid outputs vs a PWM output.
Even in that case those that used HIGH and LOW would still continue to work fine.
(Udo: All the examples that use exclusive ORing, would break if LOW was not 0)

While all hypothetical, this is an example of why using an API outside of its specification or taking
advantage of internal behaviors is bad programming practice. It has nothing to do with
political correctness or programming style, it has to do with using the API as it is defined vs taking advantage
of internal implementation details.

Whenever you take advantage of an API's internal implementation details, you run the risk
that should those implementation details ever change in the future you run the risk
that your code will break.

For a moment, ignore that we are specifically talking about digitalWrite() and the definitions
of HIGH and LOW.
Would you still advise someone to ignore an API definition and use calculated values or constants
rather than the actual defines when using API? Probabaly not.

The goal should be to help guide less knowledgeable people increase their technical & programming skills
and try to steer them in the direction of good programming practices that they can use
on going into the future.
Showing them "clever" code that takes advantage of internal implementation details
or, IMHO, somewhat sarcastic implementations like this:

cli();
state = state==HIGH ?LOW: state==LOW? HIGH: state;
sei();

are not helpful.

I think for less technical folks that are just starting out trying to understand basic
concepts, IMHO, it is best to give them clear,
simple easy to understand examples which avoid the use of ternary macros
and that strictly follow the APIs.
So I would recommended either the original code:

if (state == LOW) 
{       
        state = HIGH;
}       
else    
{       
        state = LOW;
}       
digitalWrite(pin, state); 
delay(1000);

Or to save a bit of typing:

if (state == LOW) 
        state = HIGH;
else    
        state = LOW;

digitalWrite(pin, state); 
delay(1000);

What most of you seem to be overlooking is the bigger concept and that is of using an API
the way it is defined vs taking advantage of the internal implementation details of the API
or its defines.

I've seen code examples out there that end up passing values other 0 or 1 to the DigitalWrite().
(they create other non-zero values and assume it works the same as HIGH)
While these work today, it is still very bad programming practice.

So I'll say it again, the best programming practice is to strictly stick to the API definition
and make no assumptions about the internal behaviors and values of the defines used the API.

That is the only way to ensure that your code can continue to work in the future
should the underlying mechanisms in the API change or be extended.

i.e. always use the defines from the API vs try to calculate them based on knowing what the define values are
or how they are used.

--- bill

Would you settle for state = (HIGH + LOW) - state;?

(I'm with Mike on this one)

AWOL:
Would you settle for state = (HIGH + LOW) - state;?

Interesting.....
I think that probably works.
Assuming state was initialized to LOW or HIGH, which I think
is an ok assumption for something like this.

Only reason I say "probably" is that I'm
not sure about if there is a overflow/underflow particularly
if state is an 8 bit value.

The if/else seems easier to understand for newbies.
I'm wondering how the compiler treats that vs
an if/else?

--- bill

Interestingly, even if the API says only HIGH or LOW are allowed, the implementation of 1.0 only tests for LOW.

AWOL:
Interestingly, even if the API says only HIGH or LOW are allowed, the implementation of 1.0 only tests for LOW.

But technically, the digitalWrite() API never says that HIGH and LOW are the only allowed values,
(it's current C code declaration can't stop you from passing other values)
The API only makes a commitment as to what behavior to expect when the values are HIGH or LOW.
The behavior for other values are undefined in the API.

Garbage in, Garbage out right????

It is why I mentioned earlier that there are "working" code examples out there in the forum posts
and in libraries that are using 0 and non zero to set the output pin state.

They work because the current AVR arduino code in digitalWrite() only checks for LOW
and sets the output pin to a logic high level when the value passed is not LOW.

With todays AVR Arduino dititalWrite() code implementation,
you can get away with assuming HIGH is 1 and LOW is 0
and that zero sets a output pin to low and any other non zero value sets the output pin to a logical
high.

But I like to write and encourage others to write portable code that does
not depend on internal implementation details that are not part of the formal API
definition.

This is why I stress that using internal implementation details is dangerous as
some other implementation on some other processor
say ARM, pic32, etc... might not implement the code the same way or make the same assumptions
or the underlying code might change in the future.

The reality of things at this point, is that there is so much existing code out there
that didn't stick to the formal APIs that if you now were to change something like this,
some amount of the code will break, and that is why the core developers like Diligent/chipkit and Maple
have implemented their DigitalWrite() routines with the same HIGH/LOW values and assumptions.

Still, it's never a bad thing to try to encourage folks to stick to API definitions.

That said, there are sometimes legitimate uses to violate an API spec or take advantage
or internal details.
So I will admit there are times when you do want to intentionally violate the API spec.
Perhaps by taking advantage of knowing how the API works or the values of its defines
you can optimize the code to some higher needed level.
But usually when deciding to do this it is a path carefully taken knowing that in doing this,
it may create a portability issue in the future.

That would apply in this particular case if you were looking to squeeze a few cycles or
flash bytes out.

If you look at the AVR code generated, you can generate better/faster code
if you can take advantage of knowing that HIGH is 1 and LOW is 0.
For example;

 state ^= HIGH;

and this

state ^= state;

Is more efficient than

if(state == HIGH)
  state == LOW;
else
  state == HIGH;

It will generate fewer AVR instructions which will be less/faster code.

(The subtract method gives a gcc warning as I assume
it is worried about overflows/underflows)

But doing things like this come with added risk that if ever the defines change,
the code may stop working as expected.
Or it may not generate better code on some other CPU architecture.

So, in general, that is why I try do discourage people from writing code that
takes advantage of internal API implementation details.
It just not good programming practice to ensure long term portability.

--- bill

Is there a binary logic chip made that uses other than 0 for FALSE/LOW and 1 for TRUE/HIGH?
Memory, controller, processor, register?

state ^= state;

This will set state to zero. I assume you meant this?:

state = !state;

and this:

if(state == HIGH)
  state == LOW;
else
  state == HIGH;

should be this:

if(state == HIGH)
  state = LOW;
else
  state = HIGH;

Pete