Update character_lcd.py #37

profbrady · 2019-04-06T15:58:16Z

Update so self.message respects the cursor_position() settings. If '\n' is in a message string, the new line will be use the same column setting as the first line.

Update so `self.message` respects the `cursor_position()` settings. If `'\n'` is in a message string, the new line will be use the same column setting as the first line.

profbrady · 2019-04-09T16:17:51Z

Build failed??? What does that mean? Was this caused by my edits? I am hopeful that I can have my students use this change in an upcoming project. Forgive my stupidity, I am new to Github nor am I formally trained in programming.

dhalbert · 2019-04-09T16:23:45Z

Hi - to check the errors, click on "Details" above, then on "The build" in "The build failed". Look at the build log. On this build, looks like there's a documentation error reported on line 755.

deshipu · 2019-04-09T16:24:48Z

Warning, treated as error:
../README.rst:119:undefined label: bundle_installation (if the link has no caption the label must precede a section header)
The command "cd docs && sphinx-build -E -W -b html . _build/html && cd .." exited with 2.

profbrady · 2019-04-09T16:35:08Z

I am not sure what all of that means. I did not change anything documentation-wise (I don’t think so at least). Maybe I should have??

…

On Apr 9, 2019, at 12:24 PM, Radomir Dopieralski ***@***.***> wrote: Warning, treated as error: ../README.rst:119:undefined label: bundle_installation (if the link has no caption the label must precede a section header) The command "cd docs && sphinx-build -E -W -b html . _build/html && cd .." exited with 2. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#37 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/APSHU6pl6YSaNa_GSjDhZHD8QmuRy_6uks5vfL7RgaJpZM4cgRch>.

kattni · 2019-04-09T16:55:07Z

The failure is unrelated to anything you did. The README needs to be edited to fix this issue. I can walk you through what to change, @profbrady

kattni · 2019-04-09T16:58:02Z

@profbrady Please remove lines 116-125 from README.rst and push the changes to this PR.

Installation
============

This library is **NOT** built into CircuitPython to make it easy to update. To
install it either follow the directions below or :ref:`install the library bundle <bundle_installation>`.

To install:

#. Download and unzip the `latest release zip <https://github.com/adafruit/Adafruit_CircuitPython_CharLCD/releases>`_.
#. Copy the unzipped ``adafruit_character_lcd`` to the ``lib`` directory on the ``CIRCUITPY`` or ``MICROPYTHON`` drive.

profbrady · 2019-04-09T18:45:12Z

I think I did it. I edited the file and committed the changers to my branch. Do I need to do more?

…

On Apr 9, 2019, at 12:58 PM, Kattni ***@***.***> wrote: @profbrady <https://github.com/profbrady> Please remove lines 116-125 from README.rst and push the changes to this PR. Installation ============ This library is **NOT** built into CircuitPython to make it easy to update. To install it either follow the directions below or :ref:`install the library bundle <bundle_installation>`. To install: #. Download and unzip the `latest release zip <https://github.com/adafruit/Adafruit_CircuitPython_CharLCD/releases>`_. #. Copy the unzipped ``adafruit_character_lcd`` to the ``lib`` directory on the ``CIRCUITPY`` or ``MICROPYTHON`` drive. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#37 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/APSHU4W2rVFM59F037AcwRRVPFIsukR8ks5vfMabgaJpZM4cgRch>.

kattni · 2019-04-09T18:51:05Z

@profbrady You need to push the branch to GitHub.

profbrady · 2019-04-09T19:08:18Z

This is where my ignorance shows. I don't know how to push the branch.

kattni · 2019-04-09T19:15:45Z

@profbrady Ok, let's take a couple of steps back. Where did you make the changes to the README? Tell me the exact process you took to make those changes so I can help you figure out the next steps.

As well, you can reach me as @kattni on the Adafruit Discord. If live chat would work better for you, we can figure it out there. I'm happy to continue here if this is what works for you.

profbrady · 2019-04-09T19:19:26Z

I went into my branch; profbrady: prof brady-patch-1 and clicked on the edit icon for the readme file. I removed the lines and clicked on Commit Changes with the commit directly to my branch toggled on.

makermelissa · 2019-04-09T19:22:53Z

It appears the readme file was updated on your master branch, but your PR is based off of the profbrady-patch-1 branch.

profbrady · 2019-04-09T19:25:18Z

I think I edited the correct one now.

kattni · 2019-04-09T19:27:30Z

@profbrady You did! Well done! Your build is in behind a rather long build, so it might take a bit for Travis to run. Please be patient and we'll see if my suggestions fix the issue. :)

profbrady · 2019-04-09T19:28:02Z

Thanks for all of the help and patience

makermelissa · 2019-04-09T19:28:35Z

I'll go ahead and test this out tonight.

kattni · 2019-04-09T19:28:35Z

Thank you for sticking with it and for your contribution!

makermelissa · 2019-04-10T01:42:05Z

Hi @profbrady, I went ahead and tested this with the charlcd_mono_simpletest.py example and with these changes, it appears to be losing a bunch of the text. Could you try running this example and see what your results are? Thanks

makermelissa

This seems to be losing a bunch of the text, especially on the first line.

profbrady · 2019-04-10T02:04:56Z

I'll have to test it in the morning. I left my LCD in my office at school. I am using a standard LCD, not SPI or I2C. It is a 6 digital out variety that is about 4 years old. I ran the following code on it and it worked fine.

# Circuit 15 LCD Display
# Use 5V, not 3.3V, to power the LCD display
# Requires Mr. Brady's character_lcd.py library file
# Use the Serial button in Mu to see printed output

import time, math, digitalio
from adafruit_character_lcd import character_lcd
from board import *

#   Character LCD Config:
#   modify this if you have a different sized charlcd
lcd_columns = 16
lcd_rows = 2

#   Metro m0 Pin Config:
lcd_rs = digitalio.DigitalInOut(D12)
lcd_rs.switch_to_output()

lcd_en = digitalio.DigitalInOut(D11)
lcd_en.switch_to_output()

lcd_d7 = digitalio.DigitalInOut(D2)
lcd_d7.switch_to_output()

lcd_d6 = digitalio.DigitalInOut(D3)
lcd_d6.switch_to_output()

lcd_d5 = digitalio.DigitalInOut(D4)
lcd_d5.switch_to_output()

lcd_d4 = digitalio.DigitalInOut(D5)
lcd_d4.switch_to_output()

#lcd_backlight = digitalio.DigitalInOut(D13)
#lcd_backlight.switch_to_output()

#   Init the lcd class
lcd = character_lcd.Character_LCD_Mono(lcd_rs, lcd_en, lcd_d4, lcd_d5, lcd_d6, lcd_d7, lcd_columns, lcd_rows)

#   Print a 2x line message
lcd.cursor_position(1,0)
lcd.display = False
lcd.message = 'Hello,\nWorld!'
time.sleep(0.2)
for i in range(6):
    lcd.move_left()
    time.sleep(0.2)
lcd.display = True
for i in range(15):
    lcd.move_right()
    time.sleep(0.2)
for i in range(9):
    lcd.move_left()
    time.sleep(0.2)
start_time = int(time.monotonic())

while True:
    lcd.cursor_position(12,0)
    current_time = int(time.monotonic() - start_time)
    time_display = '{:03d}\nsec'.format(current_time)
    lcd.message = time_display
    time.sleep(0.1)

makermelissa · 2019-04-10T02:24:08Z

Yeah, I used a standard LCD as well. Your code works with your update, but with reverse text like in the example, it appears to break.

profbrady · 2019-04-10T02:34:53Z

I found that I did have an I2C RGB at home. If I ran the sample code for that as is I had the same problem you did after a direction change. However, if I added lcd.cursor_position(0,0) after a direction change and before the next message command everything worked great. Maybe issuing a direction change will need to automatically reset self.row and self.column back to 0, otherwise just manually set the position after a direction change.

profbrady · 2019-04-10T03:01:38Z

A question I have is do we want to remember the position of the cursor after displaying a message so that another message starts at the end of the previous? Or do we want to have the next message start where the previous message started; essentially where the last cursor_position() was set? Either is possible, I have just tested both.

I still haven't come up with a method to set the cursor_postion() to anything but the rightmost column when changing the text direction to right-to-left.

makermelissa · 2019-04-10T07:05:54Z

One idea may be to have a settable option that makes it behave as it did before if it's set and have it perform the way your code needs it if it's set with it being off by default so it doesn't break existing code.

profbrady · 2019-04-10T11:34:34Z

That’s a possibility. It’s actually pretty easy to do since currently the message is always displayed at 0,0. When I get a few moments today I’ll test my idea.

…

On Apr 10, 2019, at 3:05 AM, Melissa LeBlanc-Williams ***@***.***> wrote: One idea may be to have a settable option that makes it behave as it did before if it's set and have it perform the way your code needs it if it's set with it being off by default so it doesn't break existing code. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

profbrady · 2019-04-10T13:47:34Z

I just pushed a change to character_lcd.py and charled_mono_simpletest.py to implement the changes mentioned above. The demo now demonstrates the use of cursor_position().

makermelissa · 2019-04-11T03:56:29Z

Hi @profbrady, one of the reasons I wanted to avoid changing the simpletests if possible is because we would need to make changes to them all and test all of them. I think the simplest approach would be to undo the changes to the simpletest, add a property to the library that is called something like column_align with a default value of False, and if the value is set true, it runs the code with your additions. That way it won't break the examples and you'll have the feature that works with your code.

profbrady · 2019-04-11T11:14:56Z

I will happily return the simpletest example to its previous form. I was only attempting to demonstrate that the existing parts of the test continue to work as they previous did while also demonstrating the use of `cursor_position`. I was not trying to break anything and believe that the functionality is the same as it was if `cursor_position` is never set. A bit of history. Last year I had my class use this library as part of an assignment. At that time we were still using Circuitpython 2.x. The `cursor_position` was actually called `set_cursor` way back then. When it wasn’t used, the first printed message started at (0,0) and subsequent messages started where the first left off, even if that meant they were completely off the display. The `set_cursor` method worked as expected. Now, in CircuitPython 3.x, `cursor_position` does not work at all and all messages are displayed starting at (0,0). I believe my code essentially returns the functionality to the way the library worked a year ago.

…

On Apr 10, 2019, at 11:56 PM, Melissa LeBlanc-Williams ***@***.***> wrote: Hi @profbrady <https://github.com/profbrady>, one of the reasons I wanted to avoid changing the simpletests if possible is because we would need to make changes to them all and test all of them. I think the simplest approach would be to undo the changes to the simpletest, add a property to the library that is called something like column_align with a default value of False, and if the value is set true, it runs the code with your additions. That way it won't break the examples and you'll have the feature that works with your code. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#37 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/APSHU2U92USWKxjHvESa6TKVUsVbicApks5vfrJugaJpZM4cgRch>.

profbrady · 2019-04-12T01:34:38Z

I was able to take some time today to implement the property idea. The property column_align defaults to False. When False the text after a \n will start at the beginning of the line. When True it will align to the text on the previous line. This works regardless if cursor_position() is set or not and regardless of whether the text direction is left-to-right or right-to-left. The cursor_position() setting only affects the first message after it is set. After that the message start position reverts to (0, 0) since it is reset when the message is displayed. The cursor_position() setting works for both text directions and is determined from the left for left-to-right text and from the right for right-to-left text.

The current simple demo works as before, so existing code should not break. Pushing the change in a minute.

makermelissa · 2019-04-12T01:43:58Z

Ok, thanks @profbrady. I'll take a look at this shortly.

makermelissa

You're very close here. I found a few more things to change. Also it looks like Travis is unhappy about some whitespace on lines 205, 251, and 398. I went ahead and tested with the original simpletest file and that seemed to work fine, so I think that's the only changes to make. Thanks for sticking with me here.

makermelissa · 2019-04-12T02:28:26Z

adafruit_character_lcd/character_lcd.py

+
+    @column_align.setter
+    def column_align(self, enable):
+        self._column_align = enable


Let's add some type checking here, so something like:

if isinstance(enable, bool): self._column_align = enable else: raise ValueError('The column_align value must be either True or False')

makermelissa · 2019-04-12T02:29:20Z

examples/charlcd_mono_simpletest.py

@@ -53,7 +54,7 @@
 scroll_msg = '<-- Scroll'
 lcd.message = scroll_msg
 # Scroll message to the left
-for i in range(len(scroll_msg)):
+for i in range(len(scroll_msg)+5):
    time.sleep(0.5)


Looks like there are still some changes to the simpletest, so just remove the +5

makermelissa · 2019-04-12T02:29:53Z

examples/charlcd_mono_simpletest.py

@@ -33,6 +33,7 @@
 lcd.message = "Hello\nCircuitPython"
 # Wait 5s
 time.sleep(5)
+lcd.clear()


Let's remove the lcd.clear() just to keep the examples consistent.

profbrady · 2019-04-12T02:46:44Z

@makermelissa thanks for the assistance on this. I just pushed the changes. I hope this fixes it. It is a bit exciting knowing that I am contributing to the community. I did a very small fix to simpleio about a year ago, but this was a bit more work.

makermelissa

Looks like there's just a couple small changes that need to be made.

makermelissa · 2019-04-12T03:12:44Z

adafruit_character_lcd/character_lcd.py

+    @property
+    def column_align(self):
+        """If True, message text after '\n' starts directly below start of first
+        character in message. If False, text after '\n' starts at column zero.


Been looking into this since you submitted changes. It looks like it's failing in travis because the \n's need to be escaped. Instead of '\n' use '\\n' (two slashes. I had to use three for formatting the comment)

makermelissa · 2019-04-12T03:13:10Z

adafruit_character_lcd/character_lcd.py

@@ -169,6 +169,11 @@ def __init__(self, rs, en, d4, d5, d6, d7, columns, lines
        self._message = None
        self._enable = None
        self._direction = None
+        # track row and column used in cursor_position
+        # itialize to 0,0


I figured you mean initialize here

makermelissa · 2019-04-12T03:32:38Z

Ok, I just gave it a final test and it worked great. Thanks again for this.

Updating https://github.com/adafruit/Adafruit_CircuitPython_CharLCD to 3.2.0 from 3.1.2: > Merge pull request adafruit/Adafruit_CircuitPython_CharLCD#37 from profbrady/profbrady-patch-1

Update character_lcd.py

c7d02b5

Update so `self.message` respects the `cursor_position()` settings. If `'\n'` is in a message string, the new line will be use the same column setting as the first line.

Update README.rst

d6338ab

makermelissa self-requested a review April 10, 2019 01:40

makermelissa requested changes Apr 10, 2019

View reviewed changes

profbrady added 2 commits April 10, 2019 09:38

Update character_lcd.py

a6eb4ef

Update charlcd_mono_simpletest.py

2516129

profbrady added 3 commits April 10, 2019 10:18

Update charlcd_mono_simpletest.py

2c1a7a4

Update character_lcd.py

5bc8ec5

Update character_lcd.py

7ef2cc6

Update charlcd_mono_simpletest.py

3d4340c

Update character_lcd.py

4ffe6de

makermelissa requested changes Apr 12, 2019

View reviewed changes

profbrady added 2 commits April 11, 2019 22:36

Update charlcd_mono_simpletest.py

eb8612e

Update character_lcd.py

ff1df52

makermelissa reviewed Apr 12, 2019

View reviewed changes

Update character_lcd.py

f9d662e

makermelissa approved these changes Apr 12, 2019

View reviewed changes

makermelissa merged commit d0fc756 into adafruit:master Apr 12, 2019

profbrady deleted the profbrady-patch-1 branch April 12, 2019 03:35

kattni mentioned this pull request Apr 12, 2019

Feature request - Begin message at cursor position #21

Closed

Update character_lcd.py #37

Update character_lcd.py #37

Uh oh!

Conversation

profbrady commented Apr 6, 2019

Uh oh!

profbrady commented Apr 9, 2019

Uh oh!

dhalbert commented Apr 9, 2019

Uh oh!

deshipu commented Apr 9, 2019

Uh oh!

profbrady commented Apr 9, 2019 via email

Uh oh!

kattni commented Apr 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kattni commented Apr 9, 2019

Uh oh!

profbrady commented Apr 9, 2019 via email

Uh oh!

kattni commented Apr 9, 2019

Uh oh!

profbrady commented Apr 9, 2019

Uh oh!

kattni commented Apr 9, 2019

Uh oh!

profbrady commented Apr 9, 2019

Uh oh!

makermelissa commented Apr 9, 2019

Uh oh!

profbrady commented Apr 9, 2019

Uh oh!

kattni commented Apr 9, 2019

Uh oh!

profbrady commented Apr 9, 2019

Uh oh!

makermelissa commented Apr 9, 2019

Uh oh!

kattni commented Apr 9, 2019

Uh oh!

makermelissa commented Apr 10, 2019

Uh oh!

makermelissa left a comment

Choose a reason for hiding this comment

Uh oh!

profbrady commented Apr 10, 2019

Uh oh!

makermelissa commented Apr 10, 2019

Uh oh!

profbrady commented Apr 10, 2019

Uh oh!

profbrady commented Apr 10, 2019

Uh oh!

makermelissa commented Apr 10, 2019

Uh oh!

profbrady commented Apr 10, 2019 via email

Uh oh!

profbrady commented Apr 10, 2019

Uh oh!

makermelissa commented Apr 11, 2019

Uh oh!

profbrady commented Apr 11, 2019 via email

Uh oh!

profbrady commented Apr 12, 2019

Uh oh!

makermelissa commented Apr 12, 2019

Uh oh!

makermelissa left a comment

Choose a reason for hiding this comment

Uh oh!

makermelissa Apr 12, 2019

Choose a reason for hiding this comment

Uh oh!

makermelissa Apr 12, 2019

Choose a reason for hiding this comment

Uh oh!

makermelissa Apr 12, 2019

Choose a reason for hiding this comment

Uh oh!

kattni commented Apr 9, 2019 •

edited

Loading

makermelissa Apr 12, 2019 •

edited

Loading