If all error conditions are handled the same, there's not much
point to use distinct code for each of them.
Also, handle collisions like the other error conditions.
This saves a nice 52 bytes of program memory.
Program: 24404 bytes
Data: 1543 bytes
EEPROM: 32 bytes
Now we shouldn't experience wait cycles in i2c_write() during
typical display writes any longer. It should also distribute CPU
load of display writes a lot better.
Previously writing a line of text to the display would take
almost as long as it took to actually send it to the display,
because the I2C queue could hold only one transmission, which
effectively meant only one character. This could hold the main
loop for several milliseconds.
Now we queue characters, send them one by one, and return to the
main loop in between.
This costs 160 bytes program memory. Only 18 bytes RAM, because
the I2C queue was reduced accordingly. Now:
Program: 24456 bytes
Data: 1543 bytes
EEPROM: 32 bytes
This is
- clearing 'i2c_should_end', so i2c_write() doesn't hang and
- draining the buffer on errors.
This way we loose the remaining transmission, which is typically
half a character, but we no longer stall the entire firmware main
loop.
Actually, such error conditions are surprisingly frequent, at
least on the test hardware. Now they result in some flickering
of the displayed numbers.
This isn't pretty at all, but it shows the principle.
Unfortunately it also exploits a bug in the I2C sending mechanism,
I2C sending hangs a few seconds after reset.
Point of this change is to allow using these functions for
writing to the display, too, without duplicating all the code.
To reduce confusion, functions were renamed (they're no longer
'serial', after all:
serwrite_xxx() -> write_xxx()
sersendf_P() -> sendf_P()
To avoid changing all the existing code, a couple of macros
with the old names are provided. They might even be handy as
convenience macros.
Nicely, this addition costs no additional RAM. Not surprising, it
costs quite some binary size, 278 bytes. Sizes now:
Program: 24058 bytes 168% 79% 38% 19%
Data: 1525 bytes 149% 75% 38% 19%
EEPROM: 32 bytes 4% 2% 2% 1%
Regarding USB Serial: code was adjusted without testing on
hardware.
You see where the journey is going? This is the equivalent to the
function to write a character to the serial line, so we can
basically swap these two in other functions.
Monospaced fonts require a bit less space, right now it saves a
nice 110 bytes binary size.
Actually, the distinction between monospaced and proportional
fonts was in font.h already, so not supporting them was a bug.
But so far we have no monospaced font, so nobody noticed.
Note by Traumflug: this code was written by Ruslan Popov a lot
earlier already. I picked it to i2c_test.c to get "something"
visible running. This is the commit finally adjusted to the new
display infrastructure.
While this could also be done properly now, with a couple of
additional fonts, this would require quite some time. However,
it's more important to have at least one display working now,
rather than having a ton of sophisticated eye-candy, which isn't
even user-visible, because the upper layers are still missing.
What 'make size' previously reported was misleading, because
it didn't count the .data section as Flash usage. However, this
section is actually written to Flash.
The .data section holds the data needed for inititalising
variables. As such it counts to both, Flash and RAM usage.
Nice verification: reported 'Program' size now matches upload
size reported by avrdude exactly.
There's now the tool 'avr-size', which makes reading such stuff
much easier:
avr-size -C build/teacup.elf
Example output:
AVR Memory Usage
----------------
Device: Unknown
Program: 23704 bytes
(.text + .data + .bootloader)
Data: 1543 bytes
(.data + .bss + .noinit)
EEPROM: 32 bytes
(.eeprom)
So far for choices based on boolean sets, only, because choices
for values typically need no more readable names.
This elegantly removes the somewhat ugly check for '(', too.
Having a choice with a defined set of options is nice, but it
also requires these options to be #defined somewhere _before_
entering config.h. To keep class-like encapsulation, we'd need
two header files for each code unit, one for the options, another
one for the usual header.
That said, we use other examples of such options, e.g. CPU, F_CPU
or KINEMATICS. For CPU and F_CPU it works fine, because their
options are numbers or other values known by the compiler. For
KINEMATICS it kind of works, because this #define is used in only
one place ... and there it's suboptimal already, because no option-
set.
Anyways, I was unsure about this change and if it turns out to be
a poor decision later, it can be reverted.
Most work by Ruslan Popov, collected from various commits and
made compatible with regression tests by Traumflug.
Display test code is now enabled by #defining DISPLAY_BUS to
i2c_twi.
As we usually talk to one device only, there's not much point in
carrying around the address all the time. Surprise, this saves
only 16 bytes binary size despite of heavy usage.
So far calling code had to wait long enough between individual
transmissions to make sure they end up in distinc ones. Now
calling code can stuff as fast as it wants, i2c_write() takes
care of the distinction.
This removes the need to write in full blocks, so data can be
sent from loops and/or program memory.
This capability allows to clear the screen without too much
effort, see i2c_test.c.
Still two weaknesses left:
- Transmission end is currently detected by ringbuffer becoming
empty, so delays are needed to make sure a transmission is
completed before the next one is sent to the buffer.
- Error handling is still only half existent. Any error on the
bus will stop I2C entirely. A recovery strategy is required.
Sizes show, taking into account the additional screen clearing
code, no significant change:
FLASH : 23216 bytes
RAM : 2052 bytes
EEPROM : 32 bytes
Naively restarting I2C immediately is certainly not the solution
and just leads to an interrupt flood. As I2C is currently meant
to drive displays, where successful data transmission isn't
crucial, we now simply stop transmission on errors.
This saves another 80 bytes binary size:
FLASH : 23094 bytes
RAM : 2051 bytes
EEPROM : 32 bytes
This commit squashes in some fixes found after debugging on the
topic branch. Test code by Traumflug, collected from Ruslan's
code on the topic branch.
Before, same as now without I2C:
FLASH : 22408 bytes
RAM : 1393 bytes
EEPROM : 32 bytes
Now with I2C:
FLASH : 23224 bytes
RAM : 2057 bytes
EEPROM : 32 bytes
This totals to some 800 bytes with a whole lot of test code, so
implementation is pretty small :-)
Markus Hitter
Ruslan Popov