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

Print output to system journal, auto-rotate images and allow randomization of photo's #7

Merged
merged 12 commits into from
May 20, 2020

Conversation

ErwinP
Copy link
Contributor

@ErwinP ErwinP commented May 3, 2019

You should know that I have no background in programming, let alone writing code in Python. Therefore, the suggested implementations might be written in a "dirty" way. It does work (it's running on 10" screen at my grandmothers house), but feel free to re-write this in a better way.

Secondly: my grandmother was very pleased with this gift - therefore: THANK YOU!

 - It only shuffles the list one time, instead of every time.

 - New photo's will still be shown right after arrival, and
   the shuffling will only take place after all new photo's
   have been shown
 - It only shuffles the list one time, instead of every time.

 - New photo's will still be shown right after arrival, and
   the shuffling will only take place after all new photo's
   have been shown
In the Telegram API v12 (which is, at time of writing, the latest available for the Raspberry Pi 3), context
based callbacks got upgraded:

   https://github.com/python-telegram-bot/python-telegram-bot/wiki/Transition-guide-to-Version-12.0

Furthermore, the metafile writer got updated to write the parameters as strings instead of bytes (i.e. "w" instead of "wb").
As explained in the tutorial, config.py.in should be renamed to config.py. If a
user failes to do this, the following error is raised:

   from config import *
   ModuleNotFoundError: No module named 'config'

The deceptive nature of this errors description makes it quite difficult to troubleshoot
at first sight - the module is there, it's the file we're lacking.
@bmcage
Copy link
Contributor

bmcage commented May 19, 2020

@ErwinP As people run into errors with current python, I would merge this. Then see if I would do some things differently when I upgrade my box and move to python3 (don't fix what's not broken).
Is this PR finalized? The code looks good to me, for somebody with no prior knowledge, congrats!

The main thing I'm worried about is changes to the documentation, so the .md files. I believe only RANDOMIZE_PHOTOS in config.in could be stated also in the documentation, correct?

@ErwinP
Copy link
Contributor Author

ErwinP commented May 19, 2020

@ErwinP As people run into errors with current python, I would merge this. Then see if I would do some things differently when I upgrade my box and move to python3 (don't fix what's not broken).
Is this PR finalized? The code looks good to me, for somebody with no prior knowledge, congrats!

Thank you :)
The code until commit f81bc6f was being used as such on the familiekiosk of my grandmother, without problems. However, at that time, no buttons were present, we didn't send video's or audio, and there was no reply button. Meaning I don't know if it affects that. In the mean time, I added a next and previous button, and that still worked (it determines the random order once, and than you're able to navigate through it).

Code untill 845443a has been tested by janvangent. He ended up cloning my fork and using that, rather than applying all changes manually. @janvangent perhaps you can comment furhter?

The main thing I'm worried about is changes to the documentation, so the .md files. I believe only RANDOMIZE_PHOTOS in config.in could be stated also in the documentation, correct?

That is correct.

PS: excuse my previous comment abouth the python transition not being present in this PR; I didn't know these got updated dynamically.

@bmcage bmcage merged commit d8d905d into TeamScheire:master May 20, 2020
@bmcage
Copy link
Contributor

bmcage commented May 20, 2020

Merged. Thanks. Keep the PR coming if you do further improvements!

@janvangent
Copy link

@ErwinP As people run into errors with current python, I would merge this. Then see if I would do some things differently when I upgrade my box and move to python3 (don't fix what's not broken).
Is this PR finalized? The code looks good to me, for somebody with no prior knowledge, congrats!

Thank you :)
The code until commit f81bc6f was being used as such on the familiekiosk of my grandmother, without problems. However, at that time, no buttons were present, we didn't send video's or audio, and there was no reply button. Meaning I don't know if it affects that. In the mean time, I added a next and previous button, and that still worked (it determines the random order once, and than you're able to navigate through it).

Code untill 845443a has been tested by janvangent. He ended up cloning my fork and using that, rather than applying all changes manually. @janvangent perhaps you can comment furhter?

The main thing I'm worried about is changes to the documentation, so the .md files. I believe only RANDOMIZE_PHOTOS in config.in could be stated also in the documentation, correct?

That is correct.

PS: excuse my previous comment abouth the python transition not being present in this PR; I didn't know these got updated dynamically.

Correct, with your changes everything works 100%! Great work

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.

None yet

3 participants