Pages: [1] 2 3   Go Down
Author Topic: Not understanding toggle code  (Read 2393 times)
0 Members and 1 Guest are viewing this topic.
Nova Scotia
Offline Offline
Full Member
***
Karma: 4
Posts: 205
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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);
Logged

Offline Offline
Newbie
*
Karma: 0
Posts: 27
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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

3. wait a second

end loop

start loop again .... 
« Last Edit: January 19, 2012, 10:14:56 am by tomperdarwin » Logged

Nova Scotia
Offline Offline
Full Member
***
Karma: 4
Posts: 205
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

It is clear now. Thanks for the help.
Logged

Manchester (England England)
Offline Offline
Brattain Member
*****
Karma: 639
Posts: 34725
Solder is electric glue
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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

0
Offline Offline
Faraday Member
**
Karma: 24
Posts: 3499
20 LEDs are enough
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Or

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

smiley-wink
Logged

Check out my experiments http://blog.blinkenlight.net

Dallas, TX USA
Offline Offline
Faraday Member
**
Karma: 70
Posts: 2763
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

While this :
Code:
state = !state;

and this:

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

smiley-wink

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
Logged

UK
Offline Offline
Shannon Member
****
Karma: 223
Posts: 12630
-
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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
Logged

I only provide help via the forum - please do not contact me for private consultancy.

Manchester (England England)
Offline Offline
Brattain Member
*****
Karma: 639
Posts: 34725
Solder is electric glue
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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!
Logged

Pittsburgh, PA, USA
Offline Offline
Faraday Member
**
Karma: 99
Posts: 4837
I learn a bit every time I visit the forum.
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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!
Logged

I find it harder to express logic in English than in Code.
Sometimes an example says more than many times as many words.

0
Offline Offline
Faraday Member
**
Karma: 24
Posts: 3499
20 LEDs are enough
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

@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

Code:
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...
Logged

Check out my experiments http://blog.blinkenlight.net

Offline Offline
Edison Member
*
Karma: 50
Posts: 1699
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Or, to be "politically" correct but obscure it further, you could write this:
Code:
state = state==HIGH ? LOW : HIGH;
smiley-evil

Pete
Logged

0
Offline Offline
Faraday Member
**
Karma: 24
Posts: 3499
20 LEDs are enough
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Even more political correctness and confusion:

Code:
#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-twist

But then again maybe

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

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

Code:
cli();
state = state==HIGH ?LOW: state==LOW? HIGH: state;
sei();
« Last Edit: January 20, 2012, 11:03:43 am by Udo Klein » Logged

Check out my experiments http://blog.blinkenlight.net

0
Offline Offline
Faraday Member
**
Karma: 24
Posts: 3499
20 LEDs are enough
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

For maximum incorrectness and confusion go here: http://blog.blinkenlight.net/experiments/removing-flicker/multiple-modes-without-flicker/  smiley-eek-blue
Logged

Check out my experiments http://blog.blinkenlight.net

Dallas, TX USA
Offline Offline
Faraday Member
**
Karma: 70
Posts: 2763
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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:
Code:
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:
Code:
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:
Code:
if (state == LOW)
{      
        state = HIGH;
}      
else    
{      
        state = LOW;
}      
digitalWrite(pin, state);
delay(1000);

Or to save a bit of typing:
Code:
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
« Last Edit: January 20, 2012, 03:50:42 pm by bperrybap » Logged

Global Moderator
UK
Offline Offline
Brattain Member
*****
Karma: 310
Posts: 26631
I don't think you connected the grounds, Dave.
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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


(I'm with Mike on this one)
Logged

"Pete, it's a fool looks for logic in the chambers of the human heart." Ulysses Everett McGill.
Do not send technical questions via personal messaging - they will be ignored.

Pages: [1] 2 3   Go Up
Jump to: