search

sibbl / hass-lovelace-kindle-screensaver

This tool generates a png from a Home Assistant Lovelace view, which can be displayed on a Kindle device which has the Online Screensaver plugin installed.
MIT License
311 stars 71 forks source link

Allow switching output image format #116

Closed nbarrientos closed 3 months ago

nbarrientos commented 3 months ago

Totally untested -- never ran. I've never written any NodeJS either :grin:

This patchset assumes that LaTrappe's patch works.

It's interesting that LaTrappe's patch only changes the type attribute when creating the screenshot and a jpeg image is directly passed to convertImageToKindleCompatiblePngAsync which seems to happily deal with it and output jpeg, too? If that's the case then that function should be renamed as part of this patch.

The change is not backwards compatible as OUTPUT_PATH does not contain the extension anymore. Another option is to avoid using an extra configuration variable (IMAGE_FORMAT) and extract the format from the path by looking at the extension. Your call.

Closes #109.

sibbl commented 3 months ago

I'm really happy to see this PR so quickly after your "+1" comment this morning! 👍

Some first thoughts from a quick glance:

  1. from my point of view, the breaking change to remove the extension from the OUTPUT_PATH env var is okay. I guess most people are not using it anyway and it's rather something internal.
  2. as far as I see, you append the extension when saving the file, but not when reading it (see the fs.readFile call)
  3. please also add the new IMAGE_FORMAT variable to the run.sh and config.yaml to also let Home Assistant Add-On use it

I'd be happy to see these changes before accepting the PR and releasing a new version. Thanks a lot for your work and a happy weekend already!

nbarrientos commented 3 months ago

Thanks for your comments, I think I've incorporated all your suggestions (history rewritten).

Please test yourself if you have time before merging, I haven't run anything myself :laughing:

Have a great week-end you too!

nbarrientos commented 3 months ago

It's never late to learn some NodeJS and NPM ;). It seems to work for me. This is file on the generated image (after downloading it from the webserver):

JPEG image data, JFIF standard 1.01, resolution (DPI), density 72x72, segment length 16, baseline, precision 8, 600x800, components 1
sibbl commented 3 months ago

Thanks for the quick changes and also setting it up locally - it's always good to have a 2nd confirmation. I just updated the version and changelog: 1.0.9 is currently building and should be available soon! Great job 👍

nbarrientos commented 3 months ago

Thanks for reviewing. I've deployed the latest Docker image IRL and it seems to work fine (JPEGs are generated).