(Teletype) Firmware build troubles with 2.2 (Fixed!)

zebra · 2017-10-30T19:48:07.522Z

thanks sam, i knew i was forgetting / misrepresenting some important stuff. that is a high-quality brain dump.

important clarification for anyone following along:

NVRAM isn’t used for text or rodata, it’s defined as a dedicated flash region for arbitrary usage. the linker script defines its size with __flash_nvram_size__ (default 256K, half of total flash) and places it at the end of the flash section. (this could always be changed if necessary.) nothing will go in nvram unless we specify it with an attribute or abuse the flash controller. on teletype i think it’s only used for scene storage.
the summary line (berkeley-style size report) includes both flash and allocaed INTRAM. so it is actually not that useful if we just want to know how much space remains for code and rodata.

here’s example output from the buld script:

SIZE    teletype.elf
teletype.elf  :
section                size         addr
.reset               0x200c   0x80000000
.rela.got               0x0   0x8000200c
.init                  0x1a   0x8000200c
.text               0x18718   0x80002028
.exception            0x200   0x8001a800
.fini                  0x18   0x8001aa00
.rodata              0x5550   0x8001aa18
.dalign                 0x4          0x4
.ctors                  0x8          0x8
.dtors                  0x8         0x10
.jcr                    0x4         0x18
.got                    0x0         0x1c
.data                0x1000         0x1c
.bss                 0x3dec       0x1044
.heap               0x111d0       0x4e30
[debug, comment omitted here ]
.stack               0x2000      0x16000
.flash_nvram        0x2ed82   0x80040000
.debug_ranges        0x5b38          0x0
Total             0x2b493f9

   text	   data	    bss	    dec	    hex	filename
0x1fea6	 0x1014	0x45d42	 420860	  66bfc	teletype.elf

(these reports are produced by calling avr32-size -Ax and -Bx respectively on teletype.elf.)

i’d expect that the free space left to us for instructions and rodata can be found by taking the ending address of the last flash section (.rodata, so 0x8001aa18 + 0x5550 = 0x8000x1ff68) and subtracting it from the starting address of nvram (0x80040000), which gives 0x20098 or ~128K. my last estimate was off because it included .bss and .data, which roughly adds up.

does that calculation seem right now? if so, it would probably be useful to have the makefile perform it somehow.

oh, and an outstanding question i have is whether the linker will complain if we add too much stuff to .text, .data and/or .rodata. i’m 99.9% sure that it will but don’t have the patience to test it right now.

sam · 2017-10-30T21:39:47.686Z

sliderule:

I’ve been encountering problems building the firmware that I think might be related to code space.

While trying to track down why one commit wouldn’t run on the hardware, I started to add debug statements to the code to see what was happening. As I added debug statements, less of the code would actually run, to the point where it didn’t enter main() at all.

Have you tried removing some bits of unrelated code, and seeing if things start working again?

sliderule · 2017-10-30T22:44:17.508Z

I solved the problem by creating diffs, rebasing backwards, and applying the diffs. There might have been something wrong in the files that were checked out. I was having issues with op_enum.h. I always make clean and run the enum py script.

Figure that it was something wrong with my git cache.

sliderule · 2017-11-03T16:20:37.064Z

Alright, I’ve hit the same snag again. The codebase that I once built and released as Alpha 3 now does not operate on the module when I rebuild it.

Steps to reproduce:

git clone --recursive https://github.com/burnsauce/teletype
cd teletype
git checkout v2.2.0-alpha.3
cd module
make clean && make
./update_firmware.command

Behaviour:

Firmware uploads, but teletype never starts

Notes:

I’ve tried this with 2 different toolchains, same result
I once built this firmware, as evidenced by the release of Alpha 3

sam · 2017-11-03T16:36:22.734Z

Do you have the FTDI serial up and running? If so do you get any output at boot?

sliderule · 2017-11-03T17:22:47.170Z

Naked, I get the boot message “// teletype” and the “clearing flash” message.

So I started adding debug statements to see what’s happening. I eventually track it down to handler_ScreenRefresh()

Here’s where I get stuck. The bottom of screen_refresh_live():

char s[8];
itoa(screen_dirty, s, 10);
print_dbg("\r\nlive mode dirty: ");
print_dbg(s);
return screen_dirty;

The serial console displays that dirty is 255. Back in handler_ScreenRefresh():

    switch (mode) {
        case M_PATTERN: screen_dirty = screen_refresh_pattern(); break;
        case M_PRESET_W: screen_dirty = screen_refresh_preset_w(); break;
        case M_PRESET_R: screen_dirty = screen_refresh_preset_r(); break;
        case M_HELP: screen_dirty = screen_refresh_help(); break;
        case M_LIVE: screen_dirty = screen_refresh_live(); break;
        case M_EDIT: screen_dirty = screen_refresh_edit(); break;
        case M_SCREENSAVER: screen_dirty = screen_refresh_screensaver(); break;
    }

    print_dbg("\r\nscreen_dirty: ");
    itoa(screen_dirty, s, 10);
    print_dbg(s);

I never see this debug line appear. Additionally, no further events are called.

scanner_darkly · 2017-11-03T17:30:33.094Z

you can use print_dbg_ulong to avoid having to use itoa.
what happens if you comment out screen_refresh_live?

sliderule · 2017-11-03T17:36:43.720Z

If I comment out screen_refresh_live, everything works in that I can change to other modes and teletype operates correctly. Obviously live mode is never displayed.

No change in behaviour on the ulong change.

So… compiler bug? Linker bug? Stack overflow?

sam · 2017-11-03T17:42:32.149Z

The variable display is the newest bit of the live screen refresh, try commenting that out?

If that does fix the issue, I’d suggest going through the pointer arithmetic in that bit of code line by line.

sam · 2017-11-03T17:44:09.821Z

Looking at the code, is vars_prev uninitialised? (edit: doesn’t matter)

scanner_darkly · 2017-11-03T17:44:20.314Z

yeah, didn’t mean to suggest changing to print_dbg_ulong would fix the issue, just that it’s easier to use.

really weird, i’m doing much heavier updates in screen_refresh_live for the grid visualizer and haven’t seen any issues.

sliderule · 2017-11-03T17:49:27.285Z

sam:

Looking at the code, is vars_prev uninitialised?

vars_prev is lazily initialized (D_LIST will be true when the variable display is enabled for the first time).

sam:

The variable display is the newest bit of the live screen refresh, try commenting that out?

show_vars will be false on first run, so none of that code is relevant.

Commenting it out doesn’t work.

sam · 2017-11-03T17:53:17.547Z

This bit has changed recently too:

        else if (show_welcome_message) {
            strcpy(s, TELETYPE_VERSION ": ");
            strcat(s, git_version);
            show_welcome_message = false;
        }

Try changing it to:

        else if (show_welcome_message) {
            strcpy(s, TELETYPE_VERSION ": ");
            // strcat(s, git_version);
            show_welcome_message = false;
        }

edit: I reckon it is this, it’s trying to include your tag in the version string, and it’s ending up too long.

sliderule · 2017-11-03T18:08:05.726Z

No luck, same behaviour.

I’m trying to look through the disassembly but it seems that the source lines indicated in between the assembly code don’t correlate. Is that normal?

sliderule · 2017-11-03T18:20:30.359Z

I found it.

Defined int i in one place and size_t i in the same function.

Would have thought that this would not have been a problem given my understanding of variable scope, but I guess I’ve run into this before.

sliderule · 2017-11-03T18:28:17.706Z

I guess that wasn’t it. After checking out master and applying that change, I get the same behaviour again.

I’ll start commenting again and see what I find.

sliderule · 2017-11-03T18:40:56.301Z

sam:

Try changing it to:

    else if (show_welcome_message) {
        strcpy(s, TELETYPE_VERSION ": ");
        // strcat(s, git_version);
        show_welcome_message = false;
    }

Yup, it was this, after all. Don’t know why the build didn’t work the first time I commented it out.

The git version now includes the tag, I see. I’ll fix up this bug (bigger buffer, strncat), but we should prevent the gitversion from including the tag or else we won’t be able to see the build number.

sam · 2017-11-03T18:44:18.268Z

It includes the tag so that it picks up the version automatically too. Most tags are quite short, e.g. v2.1.1.

Just changing to strncat should be enough.

sliderule · 2017-11-03T18:45:57.253Z

But then there will be duplicate information in tagged builds, e.g.:

TELETYPE 2.2.0: v2.2.0-DEADBEE

If we’re including TELETYPE_VERSION, why would we need tags?

sam · 2017-11-03T18:53:07.089Z

Ah sorry. I’m getting confused, I thought we’d already merged this PR:

github.com/monome/teletype

programatically grab version for docs and live mode

by jlmitch5 on 12:53AM - 14 Oct 17 UTC

1 commits changed 7 files with 28 additions and 12 deletions.

But we haven’t yet. I might get round to it next week…

What’s really funny is that I suspected those lines because I thought that they had changed recently, but they haven’t for 7 months!