Skip to content

Bug fixes related to text #16

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

Merged
merged 3 commits into from
Sep 8, 2020
Merged
Changes from 1 commit
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
71 changes: 39 additions & 32 deletions adafruit_matrixportal/matrixportal.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ def add_text(
"""
if text_font:
if text_font is terminalio.FONT:
self._text_font = text_font
self._text_font = terminalio.FONT
else:
self._text_font = bitmap_font.load_font(text_font)
if not text_wrap:
Expand Down Expand Up @@ -218,7 +218,7 @@ def preload_font(self, glyphs=None):
if not glyphs:
glyphs = b"0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ-!,. \"'?!"
print("Preloading font glyphs:", glyphs)
if self._text_font and self._text_font is not terminalio.FONT:
if self._text_font is not terminalio.FONT:
self._text_font.load_glyphs(glyphs)

def set_text_color(self, color, index=0):
Expand All @@ -243,36 +243,43 @@ def set_text(self, val, index=0):
# Make sure at least a single label exists
if not self._text:
self.add_text()
if self._text_font:
string = str(val)
if self._text_maxlen[index]:
string = string[: self._text_maxlen[index]]
if self._text[index]:
# print("Replacing text area with :", string)
# self._text[index].text = string
# return
try:
text_index = self.splash.index(self._text[index])
except AttributeError:
for i in range(len(self.splash)):
if self.splash[i] == self._text[index]:
text_index = i
break

self._text[index] = Label(self._text_font, text=string)
self._text[index].color = self._text_color[index]
self._text[index].x = self._text_position[index][0]
self._text[index].y = self._text_position[index][1]
self.splash[text_index] = self._text[index]
return

if self._text_position[index]: # if we want it placed somewhere...
print("Making text area with string:", string)
self._text[index] = Label(self._text_font, text=string)
self._text[index].color = self._text_color[index]
self._text[index].x = self._text_position[index][0]
self._text[index].y = self._text_position[index][1]
self.splash.append(self._text[index])
string = str(val)
if not string:
max_glyphs = 50
else:
max_glyphs = len(string)
if self._text_maxlen[index]:
string = string[: self._text_maxlen[index]]
print("text index", self._text[index])
if self._text[index] is not None:
print("Replacing text area with :", string)
self._text[index].text = string
# return
# try:
text_index = self.splash.index(self._text[index])
# except AttributeError:
# for i in range(len(self.splash)):
# if self.splash[i] == self._text[index]:
# text_index = i
# break
Copy link
Member

Choose a reason for hiding this comment

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

Why leave this in?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I commented out for testing purposes, but I think it makes more sense to remove it than uncomment it.


self._text[index] = Label(
self._text_font, text=string, max_glyphs=max_glyphs
)
self._text[index].color = self._text_color[index]
self._text[index].x = self._text_position[index][0]
self._text[index].y = self._text_position[index][1]
self.splash[text_index] = self._text[index]

elif self._text_position[index]: # if we want it placed somewhere...
print("Making text area with string:", string)
self._text[index] = Label(
self._text_font, text=string, max_glyphs=max_glyphs
)
self._text[index].color = self._text_color[index]
self._text[index].x = self._text_position[index][0]
self._text[index].y = self._text_position[index][1]
Copy link
Member

Choose a reason for hiding this comment

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

This looks identical to above. Could it be factored out?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Possibly. I can take a look.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, took a look. It looks like the lines before and after this section are different. So while we could improve code reuse a tiny bit, I think the potential to introduce bugs from the if/else conditions changing midway during operations and reduced readability make it not worth refactoring.

self.splash.append(self._text[index])

def get_local_time(self, location=None):
"""Accessor function for get_local_time()"""
Expand Down