Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Improve device icon serving #25299

Merged
merged 13 commits into from
Dec 28, 2024
Merged

fix: Improve device icon serving #25299

merged 13 commits into from
Dec 28, 2024

Conversation

Koenkk
Copy link
Owner

@Koenkk Koenkk commented Dec 22, 2024

Currently device icons are stores as base64 string inside the device options. This significantly increases the size of the configuration.yaml, especially when done for all devices (which is what the localise device images on the frontend does). This can even lead to Zigbee2MQTT failing to start (e.g. #25259). With this PR device icons are now saved in the device_icons directory, the configuration.yaml only contains the reference to the image, e.g.:

devices:
    '0x847127fffe8d96bc':
        friendly_name: '0x847127fffe8d96bc'
        icon: device_icons/08a9016bbc0657cf5f581ae9c19c31a5.png

Note that images must be in the device_icons directory!

Fixes: #25259, nurikk/zigbee2mqtt-frontend#2100, nurikk/zigbee2mqtt-frontend#1477

TODO:

@Koenkk Koenkk marked this pull request as draft December 22, 2024 21:19
@Koenkk Koenkk requested a review from Nerivec December 24, 2024 13:56
@Koenkk Koenkk marked this pull request as ready for review December 24, 2024 14:23
@Nerivec
Copy link
Collaborator

Nerivec commented Dec 24, 2024

Did you give this a try on Windows? Since it's dealing with file system.

@Koenkk
Copy link
Owner Author

Koenkk commented Dec 24, 2024

I don't have a Windows machine to test on but the tests found an issue which I fixed in e46c2b6

@Koenkk
Copy link
Owner Author

Koenkk commented Dec 24, 2024

Added a migration to automatically convert base64 icons to pngs and prohibited base64 in the configuration.yaml.

@Nerivec
Copy link
Collaborator

Nerivec commented Dec 27, 2024

Tested it on Windows, image showed up without problem.

"icon": {
"type": "string",
"title": "Icon",
"description": "The user-defined device icon for the frontend. It can be a full URL link to an image (e.g. https://SOME.SITE/MODEL123.jpg) (you cannot use a path to a local file) or base64 encoded data URL (e.g. image/svg+xml;base64,PHN2ZyB3aW....R0aD)"
},

This description needs updating.

@Koenkk
Copy link
Owner Author

Koenkk commented Dec 28, 2024

@Nerivec good point, I forgot that URLs are should also be supported!

Koenkk added a commit to zigbee2mqtt/hassio-zigbee2mqtt that referenced this pull request Dec 28, 2024
@Koenkk Koenkk merged commit 329b8c9 into dev Dec 28, 2024
21 checks passed
@Koenkk Koenkk deleted the fix/device-icons branch December 28, 2024 14:07
Koenkk added a commit that referenced this pull request Dec 29, 2024
@Koenkk
Copy link
Owner Author

Koenkk commented Dec 29, 2024

thanks for the feedback, created #25364

Koenkk added a commit that referenced this pull request Dec 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants