a simpler, shorter and faster write() implementation for SofftwareSerial

[cider101]

as the subjects say...a simpler, more effcient (less "if") implementation for write

[cider101]

no... tx_high_ are member-function pointers ;) which need to be called by operator "." or "->"

[matthijskooijman]

no... tx_high_ are member-function pointers ;) which need to be called by operator "." or "->"

Oh, right, indeed :-)

[cider101]

I had the same idea about the zero/one value and keeping the original tx_pin_write()
But then, within the tx_pin_write(), there is yet another if need to set it low or high.
So, basicaly, you do an if on the value, (invert it if eventualy) and then doing an if again to figure out what you really want to do...

I'm not aware, that function pointers will be any slower compared to direct invokation. But yes, they do need some extra space.

I also don't really like using a macro here, it makes the code harder to read.

The macros are just here for better readabilty - i don't mind to ommit them...

Worse, the macro tx_low() now might set the pin high if inverse logic is active, which is quite contra-intuitive to me.

Maybe tx_high/tx_low arent really good names - but better ones didn't come to my mind ;) But then again, in the inverted case they are actualy logical since high means low and low means high - exactly what do mem_fun do ;) And to be honest, having variables zero/one for high/low and switched for the inverted case isn't really different from the inverted member-functions ;)

This comment doesn't belong in this commit,

what do you mean by that ?

[matthijskooijman]

I had the same idea about the zero/one value and keeping the original tx_pin_write()
But then, within the tx_pin_write(), there is yet another if need to set it low or high.
So, basicaly, you do an if on the value, (invert it if eventualy) and then doing an if again to figure out what you really want to do...

Right, I see what you mean there. However, I don't think that extra if is bad for readability of the source. In fact, I'd rather have this extra if, then having to use macros to hide the (somewhat confusing and IMHO ugly) indirect member-call syntax.

As for the final performance of the code, if that's what you're concerned with, I don't really think that extra if will be bigger or slower than the indirect call. In fact, the compiler will probably inline the tx_pin_write function and perhaps even find a smart way to remove or simplify the if structure. And, as always, if you're making changes that aim to improve performance, you should always measure or analyze the effect (e.g., look at the disassembly).

I'm not aware, that function pointers will be any slower compared to direct invokation. But yes, they do need some extra space.

You need to load the address into the Z register (IIRC, could by X or Y), which takes some time and then IIRC the icall instruction is a cycle or so slower. Not the biggest problem, though.

As for the low/high vs one/zero. It's true that there's the concept of "logical low/high" vs "physical low/high", which can be inverted when doing inverse_logic. However, I'd say that referring to "logical low high" as "zero/one" would make the code a bit less confusing :-)

This comment doesn't belong in this commit,

what do you mean by that ?

I mean that the subject of this commit does not discuss this comment and the comment is not really related to the change at hand. Furthermore, having questions like these in the source really isn't very meaningful - they'll only confuse future readers of the source. If you have a question, a mailling list or other discussion medium is the way to answer it. If the check turns out to be unneeded, then the check can be removed. If not, a comment could be added explaining why it is needed (since it is apparently unclear now).

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}