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

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!

sliderule · 2017-11-03T19:19:49.060Z

Changed my code to reflect that change. Thanks for finding this bug for me! Working with a lot of code at once makes my eyes glaze over sometimes.