Broken CPython support
clach04 opened this issue · 4 comments
Thanks for making this library available! From reading through the docs/code looks like the intent is support both CPython and MicroPython?
I just tried this on my SBC with CPython (GPIO I²C port expander (using a PCF8574) to a Hitachi HD44780 controller) and got errors in the sleep code. It looks like this is micropython specific?
I have a quick hack so I'm up and running, but not sure what direction to take here for final fix:
(py38) pi@rock64lcd:~/py/python_lcd$ git diff
diff --git a/lcd/lcd_api.py b/lcd/lcd_api.py
index 61945d5..928c75c 100644
--- a/lcd/lcd_api.py
+++ b/lcd/lcd_api.py
@@ -205,4 +205,5 @@ class LcdApi:
def hal_sleep_us(self, usecs):
"""Sleep for some time (given in microseconds)."""
- time.sleep_us(usecs)
+ #time.sleep_us(usecs) # appears to be a micropython specific function
+ time.sleep(usecs / 1000000)
My instinct is to support CPython time.sleep
first and only use time.sleep_us
if micropython has no support for it. I'm unclear on the micropython support though :(
We could monkey patch in a sleep_us
but it feels a bit dirty.
Thoughts before I hack in a monkey patch and open a PR?
Error details:
Traceback (most recent call last):
File "my_demo.py", line 40, in <module>
lcd.custom_char(0, happy_face)
File "/home/pi/py/python_lcd_clach04/lcd/lcd_api.py", line 170, in custom_char
self.hal_sleep_us(40)
File "/home/pi/py/python_lcd_clach04/lcd/lcd_api.py", line 208, in hal_sleep_us
time.sleep_us(usecs)
AttributeError: module 'time' has no attribute 'sleep_us'
The intent is that lcd_api.py should be independent of the platform. So it shouldn't have a sleep or a sleep_us in it. sleep_us is used so that you don't need to have floating point enabled (not all micropython port provide support for floating point operations).
Unfortunately, the sleep stuff came in fairly late in the development, so I added a "default" implementation in lcd_api.py so as not to break the existing implementations.
Specific hal layers can override this if desired. circuitpython, for example, also doesn't support sleep_us so it provides a hal_sleep_us function like this:
python_lcd/lcd/circuitpython_i2c_lcd.py
Lines 84 to 86 in 4376476
It looks like i2c_lcd.py (which is intended for CPython use) should provide a similar hal_sleep_us function.
I think that adding some comments to the hal_sleep_us function implemented in lcd_api.py would also help.
Thanks for taking the time to explain this, really helpful. I posted a PR that resolves the issue and should help with other platforms. It may benefit from further comments/hints.