Toggling a pin works the same way on every microcontroller I've ever used. API or no API. The result of the pin XOR 1 is always the inverse of the current pin state. Your comment about the code not being portable requires more specifics. The way I presented the function digitalToggle() is as portable as digitalWrite().
Yes, but toggling is a special case. It's not even very common, mainly seen in LED demos etc. I think it's the general case of reading and writing digital values that is the real issue.
It's unfortunate that C++ doesn't support a single bit type. It only does bit fields. Someday that may change.
Have you only ever used AVRs? with avr-gcc?
SREG is a AVR specific register that is defined in the avr-gcc toolset in common.h in the avr specific header files.
It is NOT portable across other processors.
Just to clarify. Portable means the code will work in multiple environments and is not specific to a given h/w.
Code that touches h/w typically is not portable since h/w across in different environments is not all the same. I.e. how you control a pin on an AVR is different than you control a pin on an ESP8266.
An even bigger difference is how you ensure atomicity of changing pin states in different h/w environments.
The code you posted will only work on the AVR.
It will not work on other processors.
Arduino is about portability. It defined APIs to create an abstraction for portability.
An API can be portable as it abstracts the h/w away from the user code/sketch, but the code that implements the API that touches the h/w will not and can not be portable since the h/w works differently in the different environments.
The thread started about and the discussion within in it is about portability.
As long as the code that implements an API complies with the API definition and the user code uses the API the way that it is documented, then portability is assured.
If either of those is not true, then there is no assurances of portability.
And for how this thread started, the OP stepped outside of the API definition, so the portability of his code is not assured.
i.e. since the API definition in the documentation never specifies what LOW/HIGH are, to assume something about them (even it is logical, or even based on looking at the underlying implementation), steps out side the API definition and does not assure portability to other platforms.
--- bill
[quote="madmark2150, post:121, topic:1104987]
You simply cannot return just a single bit, even tho that is the only bit with data since an internal register is used and therefore must return more than a single bit. So again, the returned type is an alias.
[/quote]
This is true (kinda because aliasing in C++ is something else) but I see what you mean. It's not relevant in the discussion though.
[quote="madmark2150, post:121, topic:1104987]
The point is that the actual data item returned is a convention because a single bit (or pointer to a single bit) cannot be returned
[/quote]
We agree.
The functions indeed do have to take a parameter or return something and the smallest unit is a byte. That's indeed a fact.
But you should not think only in terms of underlying memory as you program in C++. What also matters is the type of those bytes in memory as they will influence what operators do.
The convention as documented in the API is HIGH or LOW and you know these values do have the ability to be transformed into an integral type because of the signature of the functions but Arduino did not document the type nor the value.
So the problem is a theoretical one : when you write digitalWrite(x, !digitalRead(x)) you are making an assumption that you can apply the logic NOT operator to whatever is returned by digitalRead() and It was stated at least 10 times before it works because LOW is 0 and HIGH is 1. but you don't really know that will always be true...
It could be possible that they chose 8 for LOW and 24 for HIGH to represent the voltage (in deciVolts) thresholds for example. As long as you compare using == HIGH or == LOW you'll be fine but ! 8 is 0 and ! 24 is also 0 (due to boolean promotion as explained before) ➜ the logical NOT operator does not let you alternate between HIGH and LOW.
[quote="madmark2150, post:121, topic:1104987]
Can someone post an example of digitalWrite(x, !digitalRead(x)) NOT working?
[/quote]
Sure — we mentioned that before ➜ see this : Breakage caused by PinStatus and PinMode types · Issue #25 · arduino/ArduinoCore-API · GitHub
That happened when Arduino felt not committed to HIGH and LOW being of the int type and with values of 1 and 0 because the documentation never made that promise. In that core they tried to move to an enum which means suddenly HIGH and LOW had a specific type (PinStatus) and that type did not know how to apply the logic NOT operator (it could have been added as an overload for the enum)
So long story short no-one says digitalWrite(x, !digitalRead(x)) does not work as expected on an Arduino at the moment, but we say that if you want to respect the API (and types) then it's better to write the longer form, either
if (digitalRead(x) == HIGH)
digitalWrite(x, LOW);
else
digitalWrite(x, HIGH);
or with the ternary operator
digitalWrite(x, (digitalRead(x) == HIGH) ? LOW : HIGH);
because this way of writing the code stays true to what you called the convention. We only compare with HIGH and only return HIGH and LOW, not relying on some sort of bool to int promotion that would magically do the right thing.
That's why @mttr lost some points. Not because it did not work on his MEGA but because it did not respect the API.
Does it make sense?
Sorta, but i would expect the compiler to recognize that only a bit is needed by both digitalWrite() and digitalRead() need just a single bit and would collapse to a direct register value without being coerced into HIGH/LOW along the way.
So much is made of how smart the compilers are, why doesn't this optimize into IN/NOT/OUT ? Especially on a platform designed for IO.
\
How would it do that, given that C/C++ has no "single bit" type?
(and in the new API code, the compiler is specifically TOLD that the value has more than two possible values.)
OK, I get what you are saying now. No, I have used Zilog Z8 chips, but not a C compiler, NXP, which I think are some variant of Motorolla 6800 and Arm M0 chips, as well as a few different AVR. I agree that SREG and all that is part of the AVR-Core. An AVR specific method would be to write 1 to the Pinx register. Thank you for explaining about the API definition etc. All I wanted to add to this discussion is that an XOR truth table will illustrate that no matter what the pin state is, it will invert when XOR with 1.
This is a redefinition issue. Just as you can't name a var a reserved word, the compiler should have thrown a fit (or at least a "redefinition of constant" warning) when the library was included that redefined HIGH and LOW.
The compiler should know enough to collapse digitalWrite(x, !digitalRead(x)) into a single bit logical no matter HOW HIGH and LOW are defined.
Yes, n = dr() correctly returns an INT as the only "reasonable" type, but the compiler should recognize the underlying nature of the pairing and optimize it into a single bit XOR.
The discussion about cli() got me a little worried, since I've been using the 328P datasheet approach to reading TCNT1...
...and I didn't know about the potential cli() issue or the ATOMIC_BLOCK approach.
But if this post from Paul Stroffregen is correct, the issue is moot for my application (and for *out ^= bit;), since the execution "falls through" to the end:
(Argument from authority, I know. And no, I haven't compiled both and compared...)
I had heard about magic in the past but did not believe it...
nothing forces you to use digitalWrite or digitalRead. Those are supplied by the Arduino environment. feel free to use PORT registers or GPIO registers etc depending or you platform of choice. then you can do all the bitwise operations you want.
There are many reasonable types, int is probably not the best one.
- as you say it's only one bit that matters so you could see this as a truth value and return a bool type
- as you say it's only one bit that matters so you could return the smallest type possible a int8_t or uint8_t
- as you say it's only one bit that matters so you could have a convention and return a char 'H' for HIGH and 'L' for low
- as we code in C++, we could create an enumeration
enum class PinState : uint8_t {PIN_LOW, PIN_HIGH, PIN_FLOATING};to enforce strong typing
all those would use up only 8 bits whereas int is 16 or 32 bits depending on the platform, so I think we can argue it's not the only "reasonable" type
Which predates the modern C++ memory model. That would be the memory model used by modern C++ compilers like avr-gcc. In other words, the datasheet is woefully out-of-date.
He is correct-ish. At the time it was written, and possibly even today, he may be correct; that the generated code is identical. But that does not preclude the same code from being badly broken in newer versions of the compiler. Or possibly with the current version of the compiler. Or maybe with a certain set of optimization flags.
He is incorrect. The memory model is clear. The compiler is free to reorder the last two statements.
As an added incentive to do the right thing, I have seen compilers perform exactly that optimization; reuse a register for a return value. There is a strong incentive for a compiler to reorder statements leading into a return.
If LOW and HIGH were not defined as 0 and some other single bit value, the operation could never be optimized to a bit operation,
(i.e. LOW and HIGH can't be just any values to translate into bit operations)
Ignoring that, and even if LOW and HIGH are 0 and 1, it still can't be done since
the implementation of setting the bits in the port registers using the Arduino API functions is a bit complicated, probably more complicated then you are likely thinking. And because of this, it can't be optimized down to what you may be thinking of a simple bit operation to a register.
One big obstacle in getting optimized code to read/write h/w registers in the AVR is that bit operations to the i/o registers done from C/C++ are a total kludge in avr-gcc but it is due to the way the AVR h/w works. i.e. you can only do atomic bit operations to h/w when the address and bit number are constants known at compile time. And just to make it more interesting, even if you do know the address and bit up front at compile time, not all h/w registers support bit operations.
Also, in Arduino given the way the API was defined semantically and the way the Arduino.cc core was implemented, the addresses and bit are run time calculated.
This means that a C operation like
*port_addr |= bit;
down in digitaWrite() will not be a bit operation at the h/w level.
It will be multiple instructions that read / write all 8 bits of the port register.
Read the 8 bit port to a tmp register, set the bit in the tmp register, then write the tmp register back to the port. This is because the AVR does not have bit set or bit clear registers in the h/w and the AVR bit set instruction can't be used since the address and bit are calculated run time.
This multi-step multi-instruction operation to set/clear bits in the i/o registers is what causes atomicity issues because the sequence is interruptible which is why you see all the messing with masking interrupts in functions like digitalRead() and digitalWrite()
digitalWrite() and digitalRead() are in separate compilation units than the sketch.
This means that any optimizations that could happen would have to happen as part of LTO.
LTO can't collapses the code down to a bit flip in the hardware because digitalRead() and digitalWrite() are alot more complex than just simple bit operations to a hardware port register.
And then due to some very old decisions, digitalRead() disables PWM before doing the read of the port register. Because of this there are runtime checks on the pin to see if it is one of the pins that supports pwm and if so disable it.
And then there is a pin to port address and pin to bit mapping data lookups for the pin that are stored in flash. Flash data reads are painfully expensive on the AVR.
And because the i/o register address and bit are calculated runtime, AVR bit set, bit clear, and bit test, instructions cannot be used.
So lots of stuff going in in this functions. Way too much to be able to optimize it down to simple bit operation to the h/w i/o register.
--- bill
inline bool myDigitalRead (byte pin) { return HIGH == digitalRead (pin); }
Great answer. Yes, i haven't drilled down and coded at the port level, but from what I've read, seemed like an 8255, which I have programmed. Ditto general tristate logic. You confirmed my suspicion that there was an arbitrary -- possibly bad -- decision early in the initial design. Like the abortion of segment registers in the 80x86.
I dunno, I'm still learning Arduino (and this thread has been great) but i just figured that since Arduino was designed as a high performance IO engine, EVERYTHING would have been optimized for balls-to-the-wall performance. Seems odd that fundamentals like pin level IO are so far abstracted from <0.8V/>2.4V.
All of this, or a lot of it seems to be to hide hw differences to make porting to different platforms seamless - great idea. But not how I would expect it to be optimized. I've coded on lots of hw and all of it has idiosyncrasies, i expect issues on any porting to a new platform. But I would expect digitalWrite and digitalRead to be as hw optimized as x++ maps to inc x in .asm and as how pointers are just "register indirect" op codes.
Cx looks a LOT like an abstraction of the DEC machines it was developed on.
What would the tightest, non API compliant (single platform, say Mega 2560) flat out performance optimized replacement for digitalWrite and digitalRead look like? One that out = !in would be valid AND fast?
That assumption would be incorrect.
IMO, having been poking around and involved with Arduino for around 15 years, Arduino was originally designed and implemented by a group of very inexperience people and it shows.
Lots of rookie mistakes and lots of things that could have and should have been done differently and better.
In fact, I spotted an atomicity issue, just from reading the digitalWrite() code like the 3rd day I was playing with Arduino code.
(in the original code it was not masking interrupts when doing the register update which can cause register corruption).
It took more than a year to convince the Arduino.cc developers to fix this.
They simply didn't understand the issue.
In terms of performance, that was never a goal/priority.
Ease of use and portability has been the focus.
To a large extent those goals have been quite successful.
in terms of digitalWrite()/digitalRead() they can take several microseconds on an AVR due to the way the API was designed and the specific code implementation in the Ardino.cc AVR core.
It has gotten better over the years (from better gcc optimizations) going from a bit over 6us to around 4us or so to read or set the status of pin on a 16Mhz AVR. It could be as low as a 2 cycles or 125ns if things were handled differently when constants are involved.
In fact years ago Paul Stoffregen re-wrote the AVR core (using the same API) for his Teensy products and could get the timing down to 125ns when constants were used.
He even offered the code to Arduino.cc to put into the AVR core and they declined.
The biggest thing that hurts the i/o performance in digitalRead() / digitalWrite() is the actual API definition. The way it is defined, by allowing integer Arduino pin numbers, with a variable value parameter, it makes the implementation have to jump through hoops to to get to port registers and bit numbers particularly on the AVR.
There are numerous other ways that an API could have been defined that would have allowed an implementation to generate much more efficient/smaller/faster code to control pins, particularly on an AVR processor.
IMO, there have been quite a few NIH (not invented here moments) in Arduino.
(not wanting to take input from others)
Things are way better now in terms of them accepting input from people outside their small inner circle than it was years ago, but there are still some odd resistances. Like the absolute fight against adding a printf() method to the Print class. Every other 3rd party platform/core has done this but not the Arduino.cc platforms/cores.
--- bill