Gallery app

[yannickulrich]

Gallery application

With this application you can add image and text files to the watches FS and view them on the watch (#1237). You can have multiple files on the watch by copying them into the directory /gallery. This would allow for example to have QR codes (or other barcodes such as Aztec codes which are often used for airline tickets) on the watch by creating them in the companion app (related #181). Further, you can have text files on the watch that will just be shown.

Usage

You first need to make the directory /gallery. Using eg. itctl

$ itctl fs mkdir /gallery                          # ensure folder exists

Files with the suffix .bin are treated as images and passed to lvgl for rendering. Files with suffix .txt will be shown as text files.

Image files need to have the correct size (max 240x240) and be converted using lv_img_conv and can then be uploaded using eg. itctl

$ lv_img_conv code.png --output-file code.bin \
        --color-format CF_TRUE_COLOR \
        --output-format bin \
        --binary-format ARGB8565_RBSWAP            # convert image
$ itctl fs write code.bin /gallery/code.bin        # upload image

You can navigate between the files by swiping left and right. Long pressing the screen hides the file name and navigation bar.

Screenshots

Status

The app works well in the simulator and I have started testing it on a physical device. However, only test this on a sealed watch if you know what you are doing!

I'd appreciate any input but especially concerning memory leaks.

[velllu]

What about simple markdown support? Just colors, no need for bigger text for titles. Would this be possible?

[yannickulrich]

Colours should be possible, yes. LVGL supports re-colouring. Everything else would be quite difficult I think

[velllu]

That would be great, maybe even with links?
So you can click on a link (like this: [Link to another page](second_file.md))
and it takes you to second_file.md

Also a button to show/hide formatting?

[minacode]

Now this is an app I want to have! Thank you @yannickulrich 🙂
I like the minimalistic style of it.

[velllu]

Same, will this be in the next version?

[yannickulrich]

I'll make the changes to implement colour using the syntax used in LVGL

Write a #ff0000 red# word

Implementing links seems quite complicated as it's not directly supported by LVGL as far as I can tell. We could somehow place buttons at the correct positions or implement the touch callback. Either would be quite involved and we would have to essentially re-build the text rendering system. I don't think I understand LVGL well enough to do this but maybe someone else could give it a go..?
As it stands, this PR isn't 100% bug free and my physical watch has reset a couple of times (potentially related to copy files to the watch while the app is running). Obviously before this can be merged, it needs to be investigated. However, the bug doesn't happen in the simulator and since I have a sealed device, I can't debug this particularly well. Again, I'd appreciate any sort of input!

[Douile]

With this on watch, experience crashes when changing brightness occasionally, not sure how it causes as also have a sealed watch. App works well though and hasn't crashed while I'm using it. I feel like its todo with more stuff being in the file system making it unstable but more testing needed to confirm that.

[minacode]

I think the best way is to get this merged in a simple first version and then build additional PRs on top of it. I don't want this to be another PR dying over two years because of feature creep discussions.

[velllu]

I'm willing to help finding bugs, how do you use your fork in InfiniSim? I replaced the InfiniTime folder with your fork but there's no gallery icon in the menu

[minacode]

The "Configure and Build" section of InfiniSims readme explains the argument you have to set. You can just link to another directory and don't have to replace something.

[velllu]

@minacode I did build it but it still doesn't show

[minacode]

With -DInfiniTime_DIR=path-to-your-infinitime? I did not build it myself so I cannot verify that is does build correctly.

[velllu]

With -DInfiniTime_DIR=path-to-your-infinitime?

I think I did

[minacode]

I found a few things from scrolling through your changes. This is not a full review.

[yannickulrich]

@minacode, I've made the requested changes, thanks!

Regarding colouring, we can do this trivially by

diff --git a/src/displayapp/screens/FileView.cpp b/src/displayapp/screens/FileView.cpp
index 5026c50e..7d0e3fed 100644
--- a/src/displayapp/screens/FileView.cpp
+++ b/src/displayapp/screens/FileView.cpp
@@ -97,6 +97,7 @@ TextView::TextView(uint8_t screenID, uint8_t nScreens, DisplayApp* app, const ch
   lv_obj_t* label = lv_label_create(lv_scr_act(), nullptr);
   lv_label_set_long_mode(label, LV_LABEL_LONG_BREAK);
   lv_obj_set_width(label, LV_HOR_RES);
+  lv_label_set_recolor(label, true);
   lv_obj_align(label, lv_scr_act(), LV_ALIGN_CENTER, 0, 0);

   lfs_info info = {0};

However, now we can't have a # in the text because LVGL is trying to parse it. We could try to engineer around this, but in the interest of getting this merged quickly (#1384 (comment)) it might be a good idea to postpone this?

[minacode]

I guess one would be more annoyed by a missing # than missing colors.

[minacode]

This looks like it belongs to some util file and have a more generic name. But maybe it is fine for now.

[yannickulrich]

I agree with this but I don't think that we currently have util file. Of course I could create one but I wouldn't want to disrupt any plans the maintainers might have. I would argue that this PR should be as minimal as possible, even if that means that code needs to be moved around later on.

[minacode]

Ok, so I reviewed it. Sorry for the waiting. I am relatively new to C++ and have not done a lot with LVGL so I cannot really say something about the little details in your implementation. I seems fine to me.
But on a more conceptual level I think a separate widget for the horizontal scroll bar, or an extension of the existing vertical scrollbar, would be really nice. I am sure other PRs in the future would profit from this.

[Outplayed8713]

I added this app to my pinetime and it's really cool! Some quick feedback though:

• The animation for swiping between images takes a long time on the pinetime when compared to Infinisim
• Adding some text files will inexplicably crash the device causing it to restart. I don't know what causes the crash, but certain files will crash every time while others will not crash the device. Simply adding or removing a word can make the device crash/ no longer crash. This may also be a bug with ITD or or Infinitime.
• I would personally prefer if the filenames were hidden by default and I could long press to see them. This is just my personal preference though.

Thank you for your work!

[velllu]

@JackRaymondCyber how did you managed to try the app?

Also, will this be included in the next release?

[Outplayed8713]

@JackRaymondCyber how did you managed to try the app?

@FVellu

I merged the changes with the base repo and then followed the build instructions. I've just made my own fork with some extra apps if you would like to try it: https://github.com/JackRaymondCyber/InfiniTimeExtra

[yannickulrich]

Addressing @JackRaymondCyber's comments (thanks!)

• The animation for swiping between images takes a long time on the pinetime when compared to Infinisim

This could potentially be improved by caching the images in LVGL though am not sure.

• Adding some text files will inexplicably crash the device causing it to restart. I don't know what causes the crash, but certain files will crash every time while others will not crash the device. Simply adding or removing a word can make the device crash/ no longer crash. This may also be a bug with ITD or or Infinitime.

Yes, unfortunately I've also experienced this. No idea why though 😕. Since my device is sealed, I can't debug this efficiently. #560 should be able to help with this but I couldn't get this work either. If someone has an unsealed device and would be willing to poke around a bit, I'd appreciate it greatly!

• I would personally prefer if the filenames were hidden by default and I could long press to see them. This is just my personal preference though.

I don't have any strong opinions either. You could get this behaviour with the following patch

diff --git a/src/displayapp/screens/FileView.cpp b/src/displayapp/screens/FileView.cpp
index ec1e9d8e..734baa9c 100644
--- a/src/displayapp/screens/FileView.cpp
+++ b/src/displayapp/screens/FileView.cpp
@@ -62,7 +62,6 @@ ImageView::ImageView(uint8_t screenID, uint8_t nScreens, DisplayApp* app, const
   lv_img_set_src(image, path);
   lv_obj_align(image, lv_scr_act(), LV_ALIGN_CENTER, 0, 0);

-  ShowInfo();
 }

 TextView::TextView(uint8_t screenID, uint8_t nScreens, DisplayApp* app, const char* path, Pinetime::Controllers::FS& fs)
@@ -98,5 +97,4 @@ TextView::TextView(uint8_t screenID, uint8_t nScreens, DisplayApp* app, const ch

   fs.FileClose(&fp);

-  ShowInfo();
 }

[velllu]

Will this be merged any time soon?

[JF002]

The animation for swiping between images takes a long time on the pinetime when compared to Infinisim

This is probably caused by the relatively slow SPI bus that is shared with the external memory and the LCD. There's not much we can do here, except trying to keep the files as small as possible.

Will this be merged any time soon?

It seems that there's at least one crash issue with this app that will need to be diagnosed and fixed before we merge the feature. Then, it'll depend on the availability of us maintainer to review and eventually merge the branch.

[yannickulrich]

In case anyone's interested, I have rebased this again onto main in case some of the maintainers are interested into getting this merged (esp. since #1894 removes the danger of running out of space). If anyone has comments, please let me know 😊

[mark9064]

Tentatively adding this to the milestone :) will try and review when I have time