Serial output does not use Arduino's Print class

[matthijskooijman]

While looking over the code, I found that the serial output code is using an "sp" function to print strings to various locations, optionally doing software serial on arbitrary pins.

Is there any reason you're not using Arduino's Print class, possibly combined with the SoftwareSerial library to do this stuff? Sounds like you'd get cleaner code and also get printing numbers, String objects, Printable objects, etc. for free? Also, you could then just pass in a Print& on startup, instead of selecting a serial port through defines etc.

Mind you, I haven't looked long enough to know wether this is actually feasible, but before I look closer and invest more time, I thought I'd ask around first.

Perhaps one reason for not doing this is compatibility with older Arduino versions, but perhaps those are not longer so relevant nowadays?

[billroy]

Hi, Mattjijs:

Thanks for your note. Your observation that Bitlash doesn’t use the Print class is quite correct. When I first wrote Bitlash in 2008 the Print class didn’t exist, and anyway the ATMega168 didn’t have room for Bitlash plus other large libraries.

Your suggestion to use the Print class is interesting. There is enough space on the newer Arduinos for the code. Of course any integration would need to be optional since there are users out there who need every available byte.

I wonder how many of the benefits would be available without rewriting all the output generating routines, including func_printf (which includes some custom functionality compared with regular printf). Do you have any thoughts on this?

-br

On Jan 23, 2014, at 2:29 PM, Matthijs Kooijman notifications@github.com wrote:

While looking over the code, I found that the serial output code is using an "sp" function to print strings to various locations, optionally doing software serial on arbitrary pins.

Is there any reason you're not using Arduino's Print class, possibly combined with the SoftwareSerial library to do this stuff? Sounds like you'd get cleaner code and also get printing numbers, String objects, Printable objects, etc. for free? Also, you could then just pass in a Print& on startup, instead of selecting a serial port through defines etc.

Mind you, I haven't looked long enough to know wether this is actually feasible, but before I look closer and invest more time, I thought I'd ask around first.

Perhaps one reason for not doing this is compatibility with older Arduino versions, but perhaps those are not longer so relevant nowadays?

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

[matthijskooijman]

Thanks for clarifying. I actually expect that using the Print and Stream (which is the Print equivalent for input) classes won't take up much extra space, since you'll be already using them if you use the existing Serial classes (so the code has to be there anyway).

I just had a closer look at the existing bitlash code to figure out what combinations of serial input and output are supported now. Some relevant observations:

• Output is normally to the first hardware serial port
• Default output can be redirected to a TCP client when using an ethernet shield
• Output can be redirected to any pin using bitlash syntax, which will use Serial1 if its TX pin is used, or a software serial implementation otherwise
• Output can be redirected to an arbitrary output function by the sketch.
• When TINY_BUILD is enabled, the baudrate is hardcoded to 9600, ignoring the baudrate given. Why is this?

I also found the SOFTWARE_SERIAL_RX define, which seems to be defined only for the Adafruit Ethernet shield. It suggests that it should enable software serial reception, presumably to get terminal input on. However, since the code for it is defined in bitlash-serial.c and is never actually referenced (the #define serialRead softSerialRead there is only seen by chkbreak not by e.g. runBitlash), I suspect that this particular feature is not actually working right now?

Regarding using the official SoftwareSerial library, it seems that it does not have any support for running in a TX-only mode. This means that if we'd use it, RAM for the RX buffer will be always allocated. So we'd probably better implement a custom version of software serial (using the existing code in the bitlash library) and use that (can just be a subclass of Print for example).

Regarding func_printf, I guess we should just keep that and just have it print to the output Print class.

I'll see if I can come up with a proof-of-concept, to see how this might look.

[matthijskooijman]

A not-so-related question: Is there any particular reason to use .c files instead of .cpp? I'm missing function overloading and references :-)

[matthijskooijman]

Wait, that's actually very much related: The Print and Stream classes can of course not be used without using C++...

[matthijskooijman]

Oh, it seems the main source file is bitlash.cpp, which includes all .c files, so they are really compiled as C++ and not C. I guess the naming is just misleading (and confuses my automatic syntax checker as well), so I'll rename them in a first commit.

[matthijskooijman]

One more question (sorry for the flood...): What are the supported usecases for compiling outside of the Arduino environment? I see the unix stuff, which should work on a random unix machine, but also things like:

// MEGA is auto-enabled to build for the Arduino Mega or Mega2560
// if the '1280/'2560 define is present
//
#if defined(__AVR_ATmega1280__) || defined(__AVR_ATmega2560__)
#define MEGA 1
#endif

#if defined(MEGA)
#define beginSerial Serial.begin

Which suggest to me that perhaps it is supported to compile for an Arduino mega
board without actually including the entire Arduino codebase (OTOH, in that
case "Serial" wouldn't be defined either...).

[billroy]

Many questions, many answers...

• TINY_BUILD is an artifact from a project to build Bitlash on the ATTiny85. There was not room for baud rate selection.
• SOFTWARE_SERIAL_RX was for an early experiment and as you have observed seems unused.
• Re: proof of concept: Bravo. I will be very interested in the comparison of before/after code size. There are users who need every byte free to integrate other libraries, so I sort of have to be tight about code size increases.
• .c vs. .cpp: I'm not particularly enthusiastic about using C++ and standard OO techniques in Bitlash because of the code size differential. So Bitlash is mostly old-school vanilla C. This may be why some parts of the code seem to make you itch a little. But it fits (or did) in 16k. Anyway, I believe bitlash.cpp was required to be a .cpp so the Arduino IDE would recognize the library.
• Build environments: Outside the Arduino environment, I compile Bitlash on Ubuntu linux and Mac OS X for testing/debugging. The MEGA defines (and other similar ones) you see in the code are for Arduino variants running under the Arduino IDE (or patched extensions of it like Teensy). There are also defines for a TINY build that compiles using the Cross-Pack AVR toolchain, but that build is somewhat out of date.

-br

[matthijskooijman]

Thanks for your replies!

TINY_BUILD is an artifact from a project to build Bitlash on the ATTiny85. There was not room for baud rate selection.

Hmm, does using a constant baudrate reduce code size? I could imagine this only happens when the baudrate is inlined, but then why doesn't this work for inlining from the main sketch? In any case, I'll just leave this in for now, I was just being curious.

.c vs. .cpp: I'm not particularly enthusiastic about using C++ and standard OO techniques in Bitlash because of the code size differential. So Bitlash is mostly old-school vanilla C. This may be why some parts of the code seem to make you itch a little. But it fits (or did) in 16k. Anyway, I believe bitlash.cpp was required to be a .cpp so the Arduino IDE would recognize the library.

The fact that it uses a .cpp extension also means the compiler actually compiles as C++. However, that in itself should not increase code size (and even doing some OO coding doesn't per se, as long as you keep thinking about what your write, of course).

Build environments: Outside the Arduino environment, I compile Bitlash on Ubuntu linux and Mac OS X for testing/debugging. The MEGA defines (and other similar ones) you see in the code are for Arduino variants running under the Arduino IDE (or patched extensions of it like Teensy). There are also defines for a TINY build that compiles using the Cross-Pack AVR toolchain, but that build is somewhat out of date.

Ok, then I'll initially focus on just the Arduino and Linux builds (for lack of OSX here).

[matthijskooijman]

I accidentally commented in the wrong issue, here's the discussion so far:

Matthijs:

Here's my work so far: https://github.com/Pinoccio/library-bitlash/commits/printstream

In addition to using the Print and Stream classes, also some (seemingly) old stuff is removed and I touched some other things I came across. The second to last commit (Pinoccio@e91dd2a) is the most important one.

Right now, there is still a considerable size increase, with the bitlashdemo example on the Mega2560, the program size goes from 17,922 to 18,592 and the memory size from 5,839 to 5,870. I'm still looking as to where this increase comes from exactly to see if some of it might be avoided.

I have tested this on Arduino 1.5.5+ (git master), haven't tried 1.0.x yet (but I didn't want to keep you waiting for that :-D).
In addition to using the Print and Stream classes, also some (seemingly) old stuff is removed and I touched some other things I came across. The second to last commit (Pinoccio@e91dd2a) is the most important one.

Right now, there is still a considerable size increase, with the bitlashdemo example on the Mega2560, the program size goes from 17,922 to 18,592 and the memory size from 5,839 to 5,870. I'm still looking as to where this increase comes from exactly to see if some of it might be avoided.

I have tested this on Arduino 1.5.5+ (git master), haven't tried 1.0.x yet (but I didn't want to keep you waiting for that :-D).

Bill:

I took a quick look. You’re certainly making fast work.
FYI, the Teensy and Avropendous defines will need to stay.

[matthijskooijman]

For avropendous, do you have any reference about how to compile things for that target? I couldn't find any Arduino core for it at all?

Wrt the TEENSY defines, they don't actually seem to be used anywhere?

Perhaps the way to support Teensy was to define AVROPENDOUS as well as TEENSY or something? In any case, the Teensy should make its (usb?) serial port available as as Stream object just like regular Arduinos, so re-adding support should be trivial I think (if it defines SERIAL_PORT_MONITOR, then it might even be supported out of the box).

[matthijskooijman]

Wrt to the extra binary size, I already reclaimed 290 bytes which were due to two debug prints :-) Furthermore, I made some changes to reduce the overhead slightly, but there is still some 300 bytes (IIRC) left. This is mostly due to the fact that the Stream* and Print* used are two bytes and calling functions on them needs to load pointers from the vtable, making all operations on them a few bytes bigger than before. I suppose this is not normally a problem, but for the small targets, it is.

I'm not working on making the initBitlash(Stream*) function optional (e.g., by defining DEFAULT_CONSOLE_ONLY, the console stream will be fixed to serial port defined by DEFAULT_CONSOLE. By using some constness and modifying the type of the blconsole variable, this allows the compiler to completely optimize the variable (and all vtable operations) away in this case, which seems to reduce the overhead of this change even more (without requiring any changes in the actual code, only in the declaration of blconsole). I'm not looking into similar tricks to optimize away bldefault and/or blout when SERIAL_OVERRIDE and/or SOFTWARE_SERIAL_TX are not set, which looks promising.

[matthijskooijman]

I've pushed more commits to https://github.com/Pinoccio/library-bitlash/commits/printstream
The commits marked with "SQUASH", I'm planning to later merge into some previous commit, but for now I'm keeping them separate to make review easier for you.

With the new commits, the overhead (on a Uno) is reduced to 254 bytes of flash on normal build, 98 bytes when DEFAULT_CONSOLE_ONLY is defined. For builds with SERIAL_OVERRIDE and SOFTWARE_SERIAL_TX disabled (and optionally also TINY_BUILD enabled), the binary size even shrinks by 32 or 36 bytes :-)

Here's the results for testing on a uno. Note that I compiled the master branch with 0ea04a8 and 0b419cc included, to fix compilation.

master
    Sketch uses 17,324 bytes (53%) of program storage space. Maximum is 32,256 bytes.
    Global variables use 998 bytes (48%) of dynamic memory, leaving 1,050 bytes for local variables. Maximum is 2,048 bytes.

printstream
    Sketch uses 17,578 bytes (54%) of program storage space. Maximum is 32,256 bytes.
    Global variables use 1,028 bytes (50%) of dynamic memory, leaving 1,020 bytes for local variables. Maximum is 2,048 bytes.

printstream, DEFAULT_CONSOLE_ONLY
    Sketch uses 17,422 bytes (54%) of program storage space. Maximum is 32,256 bytes.
    Global variables use 1,026 bytes (50%) of dynamic memory, leaving 1,022 bytes for local variables. Maximum is 2,048 bytes.


master, TINY_BUILD, !SERIAL_OVERRIDE, !SOFTWARE_SERIAL_TX
    Sketch uses 10,230 bytes (31%) of program storage space. Maximum is 32,256 bytes.
    Global variables use 636 bytes (31%) of dynamic memory, leaving 1,412 bytes for local variables. Maximum is 2,048 bytes.

printstream, TINY_BUILD, !SERIAL_OVERRIDE, !SOFTWARE_SERIAL_TX, DEFAULT_CONSOLE_ONLY
    Sketch uses 10,198 bytes (31%) of program storage space. Maximum is 32,256 bytes.
    Global variables use 635 bytes (31%) of dynamic memory, leaving 1,413 bytes for local variables. Maximum is 2,048 bytes.


master, !SERIAL_OVERRIDE, !SOFTWARE_SERIAL_TX
    Sketch uses 16,762 bytes (51%) of program storage space. Maximum is 32,256 bytes.
    Global variables use 932 bytes (45%) of dynamic memory, leaving 1,116 bytes for local variables. Maximum is 2,048 bytes.


printstream, !SERIAL_OVERRIDE, !SOFTWARE_SERIAL_TX, DEFAULT_CONSOLE_ONLY
    Sketch uses 16,726 bytes (51%) of program storage space. Maximum is 32,256 bytes.
    Global variables use 931 bytes (45%) of dynamic memory, leaving 1,117 bytes for local variables. Maximum is 2,048 bytes.

In the default builds the memory usage increased by 30 bytes. I'll have a look what causes that, but it doesn't look unacceptable to me. For the small builds, the memory usage actual shrinks by 1 byte :-)

[matthijskooijman]

I just noticed that the memory figures in the above test were way too
high, turned out I locally patched my Arduino core to have a 1024-byte
buffer instead of 64 bytes :-) I fixed that and updated the numbers in
my above post.

Furthermore, notice that the above numbers are with the Arduino core
from git and a locally compiled gcc 4.8. I did another test with an
official Arduino 1.5.5 download and the included gcc 4.3, which shows
the below figures. It seems the overhead introduced is a bit bigger, but
in the TINY_BUILD, the compiler is already smart enough to kill all
that overhead.

Again compiled for a Uno:

master
    Sketch uses 17.304 bytes (53%) of program storage space. Maximum is 32.256 bytes.
    Global variables use 1.003 bytes (48%) of dynamic memory, leaving 1.045 bytes for local variables. Maximum is 2.048 bytes.

printstream
    Sketch uses 17.614 bytes (54%) of program storage space. Maximum is 32.256 bytes.
    Global variables use 1.033 bytes (50%) of dynamic memory, leaving 1.015 bytes for local variables. Maximum is 2.048 bytes.



printstream, DEFAULT_CONSOLE_ONLY
    Sketch uses 17.422 bytes (54%) of program storage space. Maximum is 32.256 bytes.
    Global variables use 1.031 bytes (50%) of dynamic memory, leaving 1.017 bytes for local variables. Maximum is 2.048 bytes.

master, TINY_BUILD, !SERIAL_OVERRIDE, !SOFTWARE_SERIAL_TX
    Sketch uses 10.352 bytes (32%) of program storage space. Maximum is 32.256 bytes.
    Global variables use 641 bytes (31%) of dynamic memory, leaving 1.407 bytes for local variables. Maximum is 2.048 bytes.

printstream, TINY_BUILD, !SERIAL_OVERRIDE, !SOFTWARE_SERIAL_TX, DEFAULT_CONSOLE_ONLY
    Sketch uses 10.328 bytes (32%) of program storage space. Maximum is 32.256 bytes.
    Global variables use 640 bytes (31%) of dynamic memory, leaving 1.408 bytes for local variables. Maximum is 2.048 bytes.

[matthijskooijman]

Also tried on 1.0.5 and (with one extra change), things also compile there. The binary sizes are abit bigger, but the difference between master and printstream are the same as for 1.5.5.

[billroy]

I’ve lost track of the Avro stuff, but there were some users, so why not leave the defines there?

Teensy includes an installer and downloader that makes modifications to the Arduino environment. You can find out more here: http://www.pjrc.com/teensy/index.html — anyway, that is why you don’t see any references in the code. They aren’t there until you install the Teensy stuff.

-br

On Jan 30, 2014, at 1:08 AM, Matthijs Kooijman notifications@github.com wrote:

For avropendous, do you have any reference about how to compile things for that target? I couldn't find any Arduino core for it at all?

Wrt the TEENSY defines, they don't actually seem to be used anywhere?

Perhaps the way to support Teensy was to define AVROPENDOUS as well as TEENSY or something? In any case, the Teensy should make its (usb?) serial port available as as Stream object just like regular Arduinos, so re-adding support should be trivial I think (if it defines SERIAL_PORT_MONITOR, then it might even be supported out of the box).

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

[matthijskooijman]

The reason I deleted the stuff was mostly that the "serialRead" macro and friends were removed and I'm not sure what the relevant replacement would be. However, assuming that the AVROPENDOUS build actually uses some Arduino-like core and makes a "Serial" object available, I guess I could just remove the serialAvailable etc. macros and leave the other stuff in.

As for the Teensy defines, I'm not sure I understand what you mean. I know that Paul supplies Teensyduino as a modified Arduino environment. I was planning to give it a whirl to do some compiletesting with the updated bitlash library soon (though I just found out that it only has a binary-blob installer which I don't really like). However, even this installer heavily modifies my Arduino environment, I suppose it will not touch the bitlash library code itself, right? And the source files inside the Arduino / Teensy core should never actually see bitlash.h, so any #define TEENSY in bitlash.h can't be used by the Teensy core code?

It looks like the TEENSY stuff was introduced in this commit (even though the commit message doesn't betray this): 2bc8314

It looks to me like before that commit, having AVR_AT90USB162 implied having an AVROPENDOUS board. However, since the TEENSY uses the same chip, this commit forces users to choose between AVROPENDOUS_BUILD and TEENSY, with TEENSY as the default. The empty #ifdef block (2bc83149#diff-6a3da1044bed8984108a96fd058ca54fR285) looks like it was intended to get TEENSY-specific defines later, but that never happened.

With that in mind, perhaps I should indeed just leave those around. Ideally, we'd autodetect the board type used, but it seems you'll need IDE version 1.5.x for that, which adds -DARDUINO_UNO (etc.) to the commandline (depending on a value from boards.txt). So, I'll drop that "remove AVROPENDOUS" commit :-)

[matthijskooijman]

I just pushed a few more commits to my printstream branch. I still have one commit pending here locally to make the UNIX_BUILD working again, but I'm out of time until monday, so I'll push that later. If you have specific comments, already let me hear them (or if you'd rather wait until I get everything squashed together into proper form, that's also ok).

[billroy]

My understanding is that Teensyduino modifies the #defines presented to sketches when they are compiled in the Arduino IDE for the Teensy build target.

Code for the Teensy and other builds cannot be disturbed as a result of this code package, because there are existing users who rely on them.

-br

On Jan 31, 2014, at 9:42 AM, Matthijs Kooijman notifications@github.com wrote:

The reason I deleted the stuff was mostly that the "serialRead" macro and friends were removed and I'm not sure what the relevant replacement would be. However, assuming that the AVROPENDOUS build actually uses some Arduino-like core and makes a "Serial" object available, I guess I could just remove the serialAvailable etc. macros and leave the other stuff in.

As for the Teensy defines, I'm not sure I understand what you mean. I know that Paul supplies Teensyduino as a modified Arduino environment. I was planning to give it a whirl to do some compiletesting with the updated bitlash library soon (though I just found out that it only has a binary-blob installer which I don't really like). However, even this installer heavily modifies my Arduino environment, I suppose it will not touch the bitlash library code itself, right? And the source files inside the Arduino / Teensy core should never actually see bitlash.h, so any #define TEENSY in bitlash.h can't be used by the Teensy core code?

It looks like the TEENSY stuff was introduced in this commit (even though the commit message doesn't betray this): 2bc8314

It looks to me like before that commit, having AVR_AT90USB162 implied having an AVROPENDOUS board. However, since the TEENSY uses the same chip, this commit forces users to choose between AVROPENDOUS_BUILD and TEENSY, with TEENSY as the default. The empty #ifdef block (2bc8314#diff-6a3da1044bed8984108a96fd058ca54fR285) looks like it was intended to get TEENSY-specific defines later, but that never happened.

With that in mind, perhaps I should indeed just leave those around. Ideally, we'd autodetect the board type used, but it seems you'll need IDE version 1.5.x for that, which adds -DARDUINO_UNO (etc.) to the commandline (depending on a value from boards.txt). So, I'll drop that "remove AVROPENDOUS" commit :-)

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

[matthijskooijman]

I have installed TeensyDuino and (after some further tweaks) confirmed that the code now still compiles for the Teensy 2.0, Teensy++ 2.0, Teensy 3.0 and Teensy 3.1. I don't have the actual hardware to verify it also runs correctly, though.

The AVROPENDOUS stuff is also back in place, so expect that should work as before as well.

Furthermore, I've added a commit to fix UNIX_BUILD support with the new Stream stuff. However, instead of adding the Stream and Print definitions to bitlash, it seemed more sensible to put all of the Arduino-on-Unix stuff into a separate library. This was something I was already planning to do for a while, so this seemed like a good occasion. The result is the ArduinoUnix library: https://github.com/matthijskooijman/ArduinoUnix

Using it allows the bitlash-unix code to be cleaned up a bit as well, since a lot of the stub implementations are now no longer needed.

I think the implementation should be completed for now, so I'll start merging commits to clean up the commit history and then submit a pullrequest for the code.

[matthijskooijman]

I just created pullrequest #31 from the cleaned up code. The original history is still available here: https://github.com/Pinoccio/library-bitlash/tree/printstream-history

[matthijskooijman]

And I've ordered a Teensy 2.0 and 3.1 so I can actually test if that still works (and for other testing as well, don't worry) ;-p