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

New option for Waveshare 2.13 (D) display #44

Merged
merged 4 commits into from
Sep 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion config.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@ def create_config(config_file=None):
logger.warning("Papirus display library not installed.")
sys.exit("Exiting...")


# Display - Waveshare 2.13 is 250 * 122 pixels
if "waveshare" in conf["atm"]["display"]:
try:
Expand All @@ -135,6 +134,15 @@ def create_config(config_file=None):
logger.warning("Waveshare display library not installed.")
sys.exit("Exiting...")

# Display - Waveshare 2.13 (D) is 212 * 104 pixels
if "waveshare2in13d" in conf["atm"]["display"]:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These options are mutually exclusive. I'm not sure how to install the library (maybe i did that wrong) but why not using elif constructs? This fixed my issue.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct. I took the normal if so as not to touch the papirus block. If the correct display is choose in the config.ini, there are no problems either.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can make the change with a separate PR

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure i understand the approach. Why don't you want to touch the papirus block? This is a configuration procedure and there is some kind of fallback-mode via "waveshare" (as it's not 100% specified). However, that works with elif as well and these values are mutually exclusive. Using "if", not elif, makes it error-prone: I had issues with the normal if-block. Maybe i missed the point somewhere: Where is the description of the installation of the waveshare-library? Maybe i did that wrong.
But that should not prevent doing elifs?!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you look at the existing PRs, you will see the touch of the papirus block will results in additional merge conflicts. I would therefore prefer to have a new PR with the elif. Then we can choose the time when we approve the adjustment. I have no problems using the normal if.

try:
from waveshare_epd import epd2in13d
WAVESHARE = epd2in13d.EPD()
except ImportError:
logger.warning("Waveshare display library not installed.")
sys.exit("Exiting...")

# API URL for coingecko
COINGECKO_URL_BASE = "https://api.coingecko.com/api/v3/"

Expand Down
Loading