MycroftAI / skill-homescreen

Apache License 2.0
7 stars 7 forks source link

Homescreen wallpaper filetype support expansion and/or more graceful error detail #17

Open jrwarwick opened 2 years ago

jrwarwick commented 2 years ago

Describe the bug Homescreen skill does not support PNG files for wallpaper, and moreover doesn't tell user that is what the problem is specifically enough. It just says "wallpaper error"

To Reproduce Steps to reproduce the behavior:

  1. Go to home.mycroft.ai
  2. Click on Skills > Homescreen
  3. Select option "Customer (using URL below)"
  4. Populate Custom Image URL box with valid url to PNG image file

Expected behavior Mycroft homescreen wallpaper replaced with selected image (as would be the case, were the file a jpeg file). Whether or not PNG support is an option, I would also expect Mycroft to verbally give the detail already present in the exception message in the log, i.e., "Wallpaper error, the custom wallpaper is not an image file [that I recognize!]" and perhaps go ahead and say "Please try converting to JPEG format, I like JPEG format!".

Log files log excerpt:

File "/opt/mycroft/skills/homescreen.mycroftai/skill/wallpaper.py", line 108, in _add_custom self._validate_custom() File "/opt/mycroft/skills/homescreen.mycroftai/skill/wallpaper.py", line 140, in _validate_custom raise WallpaperError("Custom wallpaper is not an image file") homescreen_mycroftai.skill.wallpaper.WallpaperError: Custom wallpaper is not an image file

Environment (please complete the following information):

krisgesling commented 2 years ago

Yeah right, thanks for the write up - surprised this hasn't come up before.

I imagine that supporting png will be very possible - something we'll need to do some testing on and work out the best place to test for a valid image. Whatever we define "valid" as for this purpose.