Circuitpython port #26
Conversation
lcd/lcd_api.py
Outdated
| time.sleep(0.00004) | ||
| for i in range(8): | ||
| self.hal_write_data(charmap[i]) | ||
| time.sleep(0.00004) |
There was a problem hiding this comment.
So lcd_api.py is a common file, so you'll need to factor the sleeping into the hal layer.
Also, the original files all have linux line endings. Please convert before committing, otherwise you wind up changing every line.
There was a problem hiding this comment.
I changed the EOLs (I think). I don't understand what you mean by factoring the sleeping into the hal layer.
Sorry for the bumbling, I'm new to this.
There was a problem hiding this comment.
You'll notice that inside lcd_api.py it has lots of calls to self.hal_xxx (self.hal_write_command for example).
hal_write_command is implemented in the the file that's platform specific (in your case circuitpython_i2c_lcd.py).
In order to maintain backwards compatibility, we'd probably need to provide a default function which calls time.sleep_us that the hal layer can override.
So have LcdApi implement a hal_sleep_us function which calls sleep_us (so we don't break existing code) and then provide a hal_sleep_us function in the circuitpython code which calls time.sleep
The call to sleep_us managed to sneak by me and crept in when custom_char support was added. If I had noticed it, I would have had the original author change it then.
There was a problem hiding this comment.
It would also be good to squash the commits together. In this case it will make viewing the differences easier.
|
Thanks for the clarification. Squashed and implemented. |
lcd/circuitpython_i2c_lcd.py
Outdated
| @@ -80,3 +80,8 @@ def hal_write_data(self, data): | |||
| byte = (MASK_RS | (self.backlight << SHIFT_BACKLIGHT) | ((data & 0x0f) << SHIFT_DATA)) | |||
| self.i2c.writeto(self.i2c_addr, bytearray([byte | MASK_E])) | |||
| self.i2c.writeto(self.i2c_addr, bytearray([byte])) | |||
|
|
|||
| def hal_sleep_us(self, data): | |||
There was a problem hiding this comment.
I would rename data to be usecs. Otherwise this looks great.
lcd/lcd_api.py
Outdated
| @@ -193,3 +193,7 @@ def hal_write_data(self, data): | |||
| function. | |||
| """ | |||
| raise NotImplementedError | |||
|
|
|||
| def hal_sleep_us(self, data): | |||
There was a problem hiding this comment.
Same thing here. Rename data to be usecs
Changed to more descriptive variable name.
lcd/circuitpython_i2c_lcd_test.py
Outdated
| def test_main(): | ||
| """Test function for verifying basic functionality.""" | ||
| print("Running test_main") | ||
| i2c = busio.I2C(board.SCL, board.SDA) |
There was a problem hiding this comment.
This file should use spaces rather than tabs for indentation (4 spaces is consistent with the remaining files)
Some minor changes were needed to make it work on Circuitpython (I tested on an Adafruit Trinket M0). Mostly it was about replacing time.sleep_ms and time.sleep_us with time.sleep plus a few more odds and ends.