Allow app title/icon color to be changed thru a new appmanifest.json file#41
Allow app title/icon color to be changed thru a new appmanifest.json file#41Miner365247 wants to merge 6 commits intopimoroni:mainfrom
Conversation
I am not sure about whether this code runs on hardware, as I have not received my tufty 2350 yet. Also, if the JSON is not valid, the menu will (most likely) crash.
…), also added support for custom icon colors along with faded ones.
Miner365247
changed the title
| if len(word) <= 1: | ||
| return word | ||
| return word[0].upper() + word[1:] | ||
| if is_dir(f"{root}/{path}"): |
There was a problem hiding this comment.
This won't work. path is undefined.
| name=None | ||
| custom_color=None | ||
| custom_fade_color=None | ||
| if name==None: |
There was a problem hiding this comment.
Better use
| if name==None: | |
| if name is None: |
There was a problem hiding this comment.
Also, I'm not really versed in Python's scoping rules, but if variables are block-scoped, name would also be undefined here.
| name = " ".join([capitalize(word) for word in path.split("_")]) | ||
|
|
||
| if is_dir(f"{root}/{path}"): | ||
| if file_exists(f"{root}/{path}/icon.png"): |
There was a problem hiding this comment.
Technically,
if file_exists(f"{root}/{path}/icon.png"):is good enough, you don't need to check if the parent exists and is a directory.
If the full check succeeds, the parent dir must logically exist - and otherwise, you don't really care if the parent exists and is a directory or not, since you're not doing anything with it.
|
|
||
| if is_dir(f"{root}/{path}"): | ||
| if file_exists(f"{root}/{path}/icon.png"): | ||
| App(self.apps, name, path, image.load(f"{root}/{path}/icon.png"), custom_color=custom_color, custom_fade_color=custom_fade_color) |
There was a problem hiding this comment.
You're already passing in the custom_color and custom_fade_color arguments in the same ordering as in App's constructor declaration, not sure you're going anything by specifying the argument name explicitly?
| data=json.load(f) | ||
| except Exception: | ||
| data = {} | ||
| name=data.get("title", None) |
There was a problem hiding this comment.
I hope that this works. If an exception is terminating the whole with block, then all the data extraction here will be skipped.
Python is certainly not my strong suite, so take this with a grain of salt.
|
I’ll try to fix these issues and refactor when I have time. Thanks for pointing these problems out. |
| with open(f"{root}/{path}/appmanifest.json") as f: | ||
| try: | ||
| for path in sorted(os.listdir(root)): | ||
| if file_exists(f"{root}/{path}/appmanifest.json"): |
There was a problem hiding this comment.
If you set/initialize name = None etc. here before checking for appmanifest.json, you won't need to duplicate it later on - not even if an exception occurs.
Example:
name=None
custom_color=None
custom_fade_color=None
if file_exists(f"{root}/{path}/appmanifest.json"):
try:
with open(f"{root}/{path}/appmanifest.json") as f:
data=json.load(f)
name=data.get("title", None)
normal_color=data.get("color",None) # Should be a 4 integer list being passed into normal_color
fade_color=data.get("fade_color",None) # Should be a 4 integer list being passed into fade_color
custom_color=tuple(normal_color) if normal_color else None
custom_fade_color=tuple(fade_color) if fade_color else None
except Exception:
pass
if name is None:Note that I also had to drop a level of indentation to the try block - Python will probably error out if you use two levels there.
|
Don't worry, in general! You'll have plenty of time to play around with the device once you get it. :) |
…e centalized in one area.
| name=None | ||
| custom_color=None | ||
| custom_fade_color=None | ||
| pass |
There was a problem hiding this comment.
One level of indentation too much here, use only 4 spaces instead of 8.
2 participants
Haven't received my Tufty yet, so I am unable to test, and feel free to mention any problems, but in theory, what this code should do is allow for an option to use an appmanifest.json file in the app's folder to use a title not auto-determined by the code, custom color for app icons, along with faded ones as well. Should any part of the appmanifest.json not exist, the code should fall back to defaults (auto-title determination, along with default colors).
I have attached a sample of what an appmanifest.json could look like:
ex-appmanifest.json
Some of this code could be ported to other badges, but I'm not sure.