GUI.Menu causes crash about pebblejs

[FabianBeiner]

Aha! Turns out the last our was wasted… ;-)

I agree. As soon as I want to open a UI.Menu, the app crashes. But only on the watch, not in the emulator. In there, everything is working fine.

I can even reproduce this with the Pebble.js "Hello World!" application.

[FabianBeiner]

As soon as I update handle_menu_props_packet (simply_menu.c) and comment out the following, everything (except colors, I guess 😏) works again:

  //menu_layer_set_highlight_colors(simply->menu->menu_layer.menu_layer,
  //                                gcolor8_get(packet->highlight_background_color),
  //                                gcolor8_get(packet->highlight_text_color));
  //menu_layer_set_normal_colors(simply->menu->menu_layer.menu_layer,
  //                             gcolor8_get(packet->background_color),
  //                             gcolor8_get(packet->text_color));

[fletchto99]

@FabianBeiner @idoodler Can you see if this issue persists in my fork? https://github.com/fletchto99/pebblejs

This issue should have been fixed in my pending pull request: #65

This happened because I don't have a Pebble Time, and the emulator did not reproduce the same issue as the watch. So when I created the initial pull request I did not experience this bug.

I believe this issue is caused by a race condition between the phone and the watchapp where the menu tries to color it's rows before the menu its self is initialized. The reason I was unable to replicate the issue in the emulator is that the emulator operates locally on my computer, so the connection between the "phone" and "watch" (which are all in one on the emulator) is essentially transparent. Hence the menu layer would have been created in time for the colors to be set by the emulator.

On the pebble time there is a delay when communicating to the phone via bluetooth. So the set row color function would be called before the initialization of the menu which would cause a null pointer exception and crash the app.

@Meiguro Perhaps PR #65 should be priortized? As this breaks menus for all PT watches but not emulators?

[FabianBeiner]

@fletchto99 Yes, works for me then. Red menu on my wrist, lovely. 😄 Thank you!

[Meiguro]

Really sorry about this. I've looked into the issue and the problem has to do with the virtual window stack. In my attempt to make "show" and "hide" semantics simple, menu doubly "shows" itself when it shouldn't, since there is a brief period where the JS side thinks the menu is shown when it hasn't. It checks if it's at the top of the stack, but the check is moot since the window stack calls "show" after the WindowStack already modifies its stack. The fix in #65 is sufficient before I try refactoring how Menu interacts with WindowStack.

The emulator seems to be more lax with null pointers, which is another issue that'll have to be looked into, and why this was overlooked.

Thanks again @fletchto99 for fixing this!

[idoodler]

@Meiguro I just updated to your latest commit and it did not fix my issue. With APP_LOG(APP_LOG_LEVEL_WARNING, "Made it!"); I checked if handle_menu_props_packet is executed and it was NOT. It crashes before this function.
The app crashes after window.show(), I can see this, because I have a console.log() after it.

Do you have another idea?

[fletchto99]

@Meiguro That would make more sense than my race condition idea. Though I wasn't fully sure what was causing it, other than at the point the packet was being handled that the menu layer was still null. It's odd that the emulator would allow these null pointers, thanks for looking into that one!

@idoodler I'll look into this later tonight. Does it still crash with the code you provided in the initial question? I might not be able to do much though because I've only got access to the emulator. My PT from the timeline challenge should be arriving soon and then I can test it out on the watch to ensure the issue is resolved.

[Meiguro]

@idoodler Hmm, I wasn't able to reproduce the issue on master with a Pebble Time and your code snippet. Did you make sure to build before installing? I know I forget to do this sometimes. Try also deleting the app from the Apps/Timeline tab before installing. Sometimes when an app fails to install, it loads the old version anyway. I also don't recommend setting your own window id since that's an internally managed number, but that is also not causing a crash for me.

Let me know if you're still having issues. We can try different things, such as having you try a prebuilt pbw.

[idoodler]

@Meiguro I deleted my app from my PebbleTime, deleted the build folder with my IDE (Webstorm), then I build, installed the app and I also logged everything with this command: pebble build && pebble install --phone [phone] && pebble logs --phone [phone].

Also I merged your changes manually with my repo.
There were changes in simpy_menu.c from line 437 to 447:

static void handle_menu_props_packet(Simply *simply, Packet *data) {
  MenuPropsPacket *packet = (MenuPropsPacket*) data;
  simply_menu_set_num_sections(simply->menu, packet->num_sections);
  window_set_background_color(simply->menu->window.window, gcolor8_get_or(packet->background_color, GColorWhite));
  if (!simply->menu->menu_layer.menu_layer) {
    return;
  }
  menu_layer_set_highlight_colors(simply->menu->menu_layer.menu_layer,
                                  gcolor8_get_or(packet->highlight_background_color, GColorBlack),
                                  gcolor8_get_or(packet->highlight_text_color, GColorWhite));
  menu_layer_set_normal_colors(simply->menu->menu_layer.menu_layer,
                               gcolor8_get_or(packet->background_color, GColorWhite),
                               gcolor8_get_or(packet->text_color, GColorBlack));
}

There were also smaller changes in simply_menu.c line 18, 98 and 222.
Also there were changes in /gui/menu.js from line 10-12 (Removing some changes)

I also may need to tell you that I build my own ViewManager, here are the relevant parts:

/**
 * Hides a given window if hideCurrentWindow is true. The current window is been hidden after the window have been shown
 * @param window
 * @param hideCurrentWindow
 */
VM.showWindow = function showWindow(window, hideCurrentWindow) {
    Utility.DEBUG && console.log('[+]');
    windowStack.push(window);
    window.show();
    if (hideCurrentWindow === true) {
        VM.hideWindow(windowStack[windowStack.length - 2]);
    }
};

/**
 * Hides a given window
 * @param window
 */
VM.hideWindow = function hideWindow(window) {
    for (var i = 0; i < windowStack.length; i++) {
        var tempWind = windowStack[i];
        if (tempWind === window && windowStack.length !== 1) {
            Utility.DEBUG && console.log('[-]');
            windowStack.splice(i, 1);
            window.hide();
        }
    }
};

Also I am not setting the window ID, this property has been set by pebble.js

[Meiguro]

Thanks for the detailed information! I decided to investigate what 0x806bdd2 is, and it turns out it's menu_layer_set_highlight_colors, which makes sense since that's before the merge. What line in C does your app crash after the merges? (Edit: Pasting the new crash log would be very helpful)

From the looks of your description, it doesn't appear to be much different from pure pebble.js. Having a view manager that calls show and hide is safe since that still uses pebble.js' underlying window stack.

I would try to see if the crash still occurs without changes to the C side. If you have new C side features, I would create a minimal example that crashes that doesn't use the new C features and see if it still crashes on a pure pebble.js C platform. If it still occurs, you can paste the minimal example here. While JSON dump in the first post is minimal, it unfortunately doesn't cause a crash for me, but I'm still wary if there is an issue.

If it only crashes with the C side in your repo, however, I'm still happy to look into that, though you'd have to upload your repository and pbw. If you would like to do this privately, you can contact me via the email address available in the commit history.

[idoodler]

@Meiguro Sorry, I can't upload you the pbw file or the repository, because it is under NDA. I am developing this for the company I am employed. I also have not done any C changes on my own, I just merged your changes.

I also don't get a crash report when my app crashes. It just quits, this is very odd to me...

[Meiguro]

Is it launching directly into the menu? You can use pebble build && pebble install --phone=10.7.11.66 --logs to initiate and keep the logs open before it loads the app, rather than using the logs command.

[idoodler]

Here is the "screen timeline" from top to bottom:

Displayed Window	Condition
Pebble.js launching screen	No problem
Splashscreen	No Problem
UI.Menu	App crashes without an errormessage

Here are the last executed lines:

...
Utility.DEBUG && console.log("Pushing info...");
listItems.fullscreen = true;
var list = new UI.Menu(listItems);
VM.showWindow(list, true);
Utility.DEBUG && console.log('Working');
list.on('select', function(e) {
...

Then there is the select handling function starting with last line of the code above. After this block There is no other code that can be executed. The whole code from above is encapsulated in a try/catch, but it don't catches anything.

My guess now is, that the .show() function is called and then C code is executed asynchronously, so the Utility.DEBUG && console.log('Working');can still be executed. And then it crashes somewhere.

And here is my application log:

'build' finished successfully (0.210s)
[INFO    ] Enabling application logging...
[INFO    ] I ocess_manager.c:368 Heap Usage for App <Weather La: Total Size <60628B> Used <17208B> Still allocated <600B
[INFO    ] Installation successful
[INFO    ] JS: stopping app: 1F0B0701-CC8F-47EC-86E7-7181397F9A25 Weather Land
[INFO    ] Displaying logs ... Ctrl-C to interrupt.
[INFO    ] JS: starting app: 133215F0-CE20-4C05-997B-3C9BE5A64E5B MyApp
... <--- some JSON parsing logs...
[INFO    ] JS: MyApp: Pushing info...
[INFO    ] JS: MyApp: [+]
[INFO    ] JS: MyApp: (+) [menu 3] : [card 2],[menu 3]
[INFO    ] JS: MyApp: Working
[INFO    ] JS: stopping app: 133215F0-CE20-4C05-997B-3C9BE5A64E5B MyApp
[INFO    ] JS: starting app: 1F0B0701-CC8F-47EC-86E7-7181397F9A25 Weather Land

[idoodler]

@Meiguro I successfully pinned down the error! It has something to do with displaying the Splashscreen icon property. The value is set to images/MyAppForPebble.png, here is the appinfo.json part:

      {
        "type": "png",
        "name": "IMAGE_APP_ICON",
        "file": "images/MyAppForPebble.png",
        "menuIcon": true
      }

Everything should be fine here, or am I missing something?

[fletchto99]

@idoodler I'm pretty sure the identifier should be IMAGE_MENU_ICON

[idoodler]

@Meiguro No, thats not the case. I use the path as the identifier in my whole project. I can also use another image and it also crashes. But I am able to use other images in my Project, so I guess its just with UI.Card(). Are you able to display an icon?

[Meiguro]

Hmm, that's odd. Yes, displaying an icon should be fine, that's what the demo does, and it does so in Card and Menu with the menu icon. Thanks for figuring out to this point! If you can find out what's the difference with the demo and your usage, we may be able to figure out the exact issue. So your Card can't display any image as the icon, but other windows in the project can display the image? Is your Card displaying the image, and crashing on exit?

He may be referring to the name in appinfo.json, but the SDK knows that it's the menu icon with the flag "menuIcon", so your entry is fine. I do tend to use "IMAGE_MENU_ICON" in all my projects however.

Commands meant to be sent to the Pebble are scheduled to be sent to the watch in the next tick, so any command affecting state on the watch work asynchronously. Usually when an app stops without error, it exited itself, such as by having no more windows. You can add a log line to simply_deinit in src/simply/simply.c or the end of src/main.c to see if this is the case; if it doesn't get printed, then it likely silently crashed. Of course, it appears to be working without an image, but it's possible adding an image is causing the command order to pop all windows first.

[idoodler]

@Meiguro simply_deinit() is not been called when my app crashes. And I can not use this specific image anywhere! The size is 28x28, its a 32-bit color PNG with a white background and a black icon on top, it is 792 Byte big. The compositing is set. The card has a title, subtitle, body and of course an icon, also I set isSplash: true, backgroundColor: "white" and fullscreen: true.

Have there been some changes to the image transition from Pebble.js to C? Did the image requirements change?

[Meiguro]

The images are not transferred from pebble.js to C unless you provide a http url. They are compiled into the app and converted by the native Pebble SDK. You'll know that they're compiled in if they load instantly without any visible flashes, much like the icon in the demo.

Does that mean you are able to use other images, just not that specific one? The Pebble SDK actually has changed its PNG conversion, so you may want to try it in a native Pebble app. For example, you can see if that specific image crashes if you try it in the "feature-image" example here https://github.com/pebble-examples/feature-image. If it crashes only in pebble.js, I would love to have the image or similar (maybe you could visually modify it yet still retain its crashing properties). If it crashes in the feature-image example app, I would actually still like the image or similar, but you would also want to forward it to devsupport (at) getpebble.com.

Related to the new PNG conversion, if you're using Photoshop to create the image, try using the "Save for web" feature instead of saving in the format of PNG; it seems to prefer that. If you have ImageMagick, you can use mogrify -type palette -colorspace sRGB -colors 64 -depth 2 image.png to convert it to a Pebble SDK compatible format.

[Meiguro]

@idoodler If you're still unable to get the image to display, let me know with a gist of an example app with the problem in a new issue and I'll be happy to assist. Closing this issue since the title issue, GUI.Menu crashing because of colors, shouldn't occur anymore.