mrworf / photoframe

Software to pull random photos from Google Photos and show them, like a photo frame
GNU General Public License v3.0
215 stars 38 forks source link

Changing keywords can result in incorrect image count. #182

Closed dadr closed 3 years ago

dadr commented 3 years ago

Found a bug where keywords added or deleted (in this case from USB service) were leaving behind the number of images in the state.json file - even when the keyword itself was removed. This got the slideshow stuck, because it looks for additional pictures until all have been shown. I changed the logic in services/base.py:getImagesTotal() so that it only added images for keywords listed rather than taking all the entries in _NUM_IMAGES even when they were not listed as a keyword. Oddly though, after several iterations with adding and removing keywords, the problems seem to have gone away. I'm unsure whether this is because strings got updated along the way or because of something else. Another consideration might be to add something in addition to the warning messages in modules/servicemanager.py:servicePrepareNextItem. If no additional pictures can be found, the frame just shows a black screen with error message, but it might be better to assume the source has changed, and to rescan the keywords.

dadr commented 3 years ago

OK, this "bug" is also present in the current production system. I had noticed that the "do nothing" option for resizing photos was not working like I thought it should, and went to a freshly installed and updated system to see how it worked there. In the process of adding and deleting keywords I managed to get that system into the same state I describe above. I "think" this happens when you add a keyword and then delete it before the scan for pictures has completed. I've attached some log snippets and the contents of /root/photoframe_config/services/*/state.json. This also seems to say that it might not be all that necessary to update the config data types as part of the upgrade. I'll post the fix described above as a PR. Do you think it worthwhile to add some error case code to the servicePrepareNextItem as described above?

Photoframe log:

Mar  8 11:17:51 photoframe photoframe[523]: 2021-03-08 11:17:51,176 - WARNING - prepareNextItem for USB-Photos came back with an error: No (new) images could be found
Mar  8 11:19:08 photoframe photoframe[523]: 2021-03-08 11:19:08,908 - WARNING - prepareNextItem for USB-Photos came back with an error: No (new) images could be found
Mar  8 11:19:51 photoframe photoframe[523]: 2021-03-08 11:19:51,307 - WARNING - prepareNextItem for USB-Photos came back with an error: No (new) images could be found

/root/photoframe_config/services/*/state.json:

{"_NUM_IMAGES": {"art": 258, "python3": 8}, "_KEYWORDS": ["python3"], "_OAUTH_CONFIG": null, 
"_NEXT_SCAN": {"test": 1615153074.86118, "_PHOTOFRAME_": 1612643803.609333, "art": 1615154792.32103, 
"python3": 1615223516.384467}, "_OAUTH_CONTEXT": null, "_CONFIG": null, "_EXTRAS": null, "_INDEX_IMAGE": 0, 
"_INDEX_KEYWORD": 0}
dadr commented 3 years ago

This is addressed in the python3 branch (v2)