Switch to using Print and Stream classes internally

[matthijskooijman]

Previously a mix of variables (pin number, override function) were used to determine where output should go to. These variables were checked whenever a byte was output to decide among a few possible output functions. The input stream to use was only selectable at compiletime, requiring changes to the library's files.

With these changes, the input and output are stored in a variable of type Stream* and Print* respectively. This allows users of the library to select the input stream to use from the sketch, at runtime, which makes things more flexible.

This pullrequest is based on top of pullrequest #23, but I couldn't find a way to tell github this. This means that the commits from "Remove conioh.h and mygetch.c, these were unused" up to and including "Replace char * with const char * where applicable" should be ignored when reviewing this pullrequest. This pullrequest has a cleaned up history, the original commit sequence before I squashed things together is still available here: https://github.com/Pinoccio/library-bitlash/tree/printstream-history

Changes in the API:

• A new initBitlash(Stream&) is added, to select a different stream to use as the main console. The existing initBitlash(int baud) still works, and sets up bitlash to use the default console (usuall just Serial).
• A new setOutpuHandler(Print&) is added, to redirect output to an arbitrary Print object. Again, the existing setOutputHandler that takes a function pointer still works.
• It is no longer possible to call setOutputHandler before initBitlash. If this is a problem, the old behaviour could be preserved, but requires a bit more complicated code due to optimizations.
• If you run "print #0, 123", the output will now always go to the pin (using the Serial object, assuming the Serial object transmits on pin 0), disregarding any output handler. Before, using pin number 0 would mean "don't redirect to a pin", so it would use the output handler. IMHO the new behaviour makes more sense, both in behaviour as well as making the code look more sane.
• Explicit support for the W5100 and adafruit ethernet shields is removed from the library. I expect that it's easy to still use bitlash on this hardware by just passing in a Client (which is also a Stream) object to initBitlash or setOutputHandler.

Furthermore, Arduino versions below 1.0 are no longer supported. I would say that 1.0 is old enough now (over 2 years) that almost all users will have upgraded and there is no point in supporting < 1.0 anymore. From your comments, it seems like you think otherwise. If you really think supporting older versions is necessary, we can see if we can do this, but it might require invasive changes that cancel some of the code clarity advantages of using the Stream and Print classes. In particular, it seems that the Stream class was only introduced in version 023, so just before 1.0. Perhaps we can get away by doing #define Stream HardwareSerial for older versions (and same for Print for even older versions).

Another thing that is different pre-1.0 seems to be .print() vs .write(). Apparently, pre-1.0 doing .print(uint8_t) wrote a raw single-byte instead of the number in ASCII digits. However, it seems that .write(uint8_t) has been available at least since 015, so it seems to me that just using that unconditionaly will work with version before and after 1.0 (or did they break this somewhere between 015 and 023, which I both checked?).

Finally, I still need to test and possibly update the examples. I'll add that later, but feel free to already review these commits :-)

[billroy]

Thank you. I will give the package a thorough review.

Quick comment re: Pre-Arduino 1.0: One of the values I'd like the Bitlash project to exemplify is a commitment to backwards compatibility. In the embedded environment it's not surprising for a project to require maintenance many years after the initial build.

Forcing a loyal and early-adopting Bitlash user to upgrade their toolchain to make a small change to their project, possibly to a toolchain and library set that contains additional years of space-gobbling but non-optional abstractions layered onto the core that will render their application un-buildable, is just something I am not willing to do if I can avoid it. If it compiled once on a toolset, we clearly have the code to make that work. Why throw it away?

So, awkward as it may be, I'd like to keep the pre-1.0 build artifacts in place. Perhaps with a little thought you can make it less ugly and complex than it admittedly was.

So, through this lens, even small changes like doCommand(const char *) that break existing code should be avoided, unless they provide definite value to the user of Bitlash.

(PS: Disclaimer/acknowledgement: Bitlash 2.0 broke backwards compatibility in a big way to upgrade the language significantly. This is what I mean by user-visible benefits.)

More later, and best regards,

-br

[matthijskooijman]

I updated the https://github.com/Pinoccio/library-bitlash/tree/printstream-history branch to work with Arduino 012 and upwards (which I think is even more versions than before). I tested this with 013, 015, 019 and 023 (012 should work, but that download is broken on the Arduino site). Versions from 018 and upwards define ARDUINO with their version number, for versions below that you have to add the version number to src/bitlash.h manually.

On thing that is still unsolved is that the defines in src/bitlash.h are not available in bitlash.h, meaning that versions < 019 now need a modification in bitlash.h to make sure DEFAULT_CONSOLE_ONLY is defined for them. This should be fixed by moving some defines around, but I'm not sure yet what the best approach for this is.

[billroy]

Superb. Thank you for your careful testing on the many legacy versions.

I’m interested to hear your thoughts for sorting out the remaining questions in bitlash.h and src/bitlash.h re: DEFAULT_CONSOLE_ONLY and the stream interface definitions.

-br

On Feb 7, 2014, at 4:06 AM, Matthijs Kooijman notifications@github.com wrote:

I updated the https://github.com/Pinoccio/library-bitlash/tree/printstream-history branch to work with Arduino 012 and upwards (which I think is even more versions than before). I tested this with 013, 015, 019 and 023 (012 should work, but that download is broken on the Arduino site). Versions from 018 and upwards define ARDUINO with their version number, for versions below that you have to add the version number to src/bitlash.h manually.

On thing that is still unsolved is that the defines in src/bitlash.h are not available in bitlash.h, meaning that versions < 019 now need a modification in bitlash.h to make sure DEFAULT_CONSOLE_ONLY is defined for them. This should be fixed by moving some defines around, but I'm not sure yet what the best approach for this is.

— Reply to this email directly or view it on GitHub.

[matthijskooijman]

I updated the pullrequest once more, with a few minor fixes and the restructuring of header files I previously suggested. This should now allow the API to depend on the build config and in particular allow the build to really work again on older Arduino versions.

See the printstream-history branch again for the changes since the last push (that I mostly squashed into earlier commits in this pullrequest).

There's three more things to do:

• Update examples
• Re-test older Arduino versions to confirm I didn't break them again
• Re-evaluate flash and ram size impact

[matthijskooijman]

Oh, one more important change: I previously said that Arduino < 015 was not supported by bitlash, but it turns out they were (I thought they weren't because beginSerial etc. were not defined for < 015, but it turns out in these old versions the Arduino environment itself defined these).

However, Arduino versions < 012 do not define the Print class, making it non-trivial to keep supporting these with my changes. The above means that we're now dropping support for versions that previously worked. I hope this is ok for you? Note that Arduino version 012 and below do not seem to be available for download anymore either (links give 404) and that version 011 is now nearly 6 years old...

[billroy]

Losing versions < 12 causes me no pain. They can go.

-br

On Feb 8, 2014, at 3:12 PM, Matthijs Kooijman notifications@github.com wrote:

Oh, one more important change: I previously said that Arduino < 015 was not supported by bitlash, but it turns out they were (I thought they weren't because beginSerial etc. were not defined for < 015, but it turns out in these old versions the Arduino environment itself defined these).

However, Arduino versions < 012 do not define the Print class, making it non-trivial to keep supporting these with my changes. The above means that we're now dropping support for versions that previously worked. I hope this is ok for you? Note that Arduino version 012 and below do not seem to be available for download anymore either (links give 404) and that version 011 is now nearly 6 years old...

— Reply to this email directly or view it on GitHub.