Adding hd44780 driver - Githubissues

dhrone commented 3 years ago

Requesting a draft PR as I need some guidance on documentation. Currently the luma.lcd docs only supply information on SPI style interfaces which are included on the install.rst. There is no hardware section to describe different interfaces types like luma.oled has. The hd44780 driver supports the new parallel and pcf8574 interfaces so I can either place the information for interfacing with the hd44780 on a dedicated page for it, or modify the luma.lcd docs to be more consistent with the luma.oled ones by adding a hardware.rst and modifying the install.rst. That's the path I would suggest but it would mean largely copying hardware.rst from luma.oled to luma.lcd. Either way, I plan to add a page to the document dedicated to the hd44780 to explain how the character and font functions work for it. Let me know what direction you'd like taken.

thijstriemstra commented 3 years ago

modify the luma.lcd docs to be more consistent with the luma.oled ones by adding a hardware.rst and modifying the install.rst.

+1

dhrone commented 3 years ago

Ok. I'll get to modifying the docs accordingly. QQ: Did a release go out for luma.core? I'm still seeing pypy showing 1.16.0 for it.

rm-hull commented 3 years ago

Not yet, I’ll try and get release 1.16.1 out this evening

dhrone commented 3 years ago

Thanks @rm-hull and sorry for the trouble. I got tripped up testing across the projects.

rm-hull commented 3 years ago

@dhrone luma.core 1.16.1 was released

dhrone commented 3 years ago

I suggested an organization around luma.core.interface above. However, I acknowledge the change may not be worth the impact. The simplest change seems to be to create a parallel module within luma.core.interface.

luma.core.interface.serial

SPI
I2C
ftdi_spi
ftdi_i2c
PCF8574
MCP23008 (TBD)
MCP23017 (TBD)
bitbang_spi
bitbang_serial (TBD - for ht1621 style interfaces)

luma.core.interface.parallel

bitbang_6800
bitbang_8080 (TBD)

dhrone commented 3 years ago

I have an unrelated question. During testing for the ws0010, I wound up writing a C++ version of luma.core.parallel. It leverages the new chardev interface introduced in Linux kernel 4.8 and the libgpiod library (v1.4.4). Performance tests seem to show around a 20-25% advantage over the python version at least on an RPi 3b. The libgpiod is under active development and 1.5.2 is the current version but I am using the older one as 1.5 required Linux Kernel 5.5+ which is not currently released for most RPi focused distros. I chose to use the chardev interface as it should be stable across any SBCs that are running the 4.8 or better Linux kernel (e.g. not RPi specific).

So my question is, do you want to introduce libgpiod as the way that the GPIO is controlled? I do have the C++ library interfaced to python using ctypes but I unfortunately have no experience adding the compiling of the c code into a python (setup.py) build. I would need your assistance if you decided to use it.

rm-hull commented 3 years ago

In regards the C++ version, if it can be installed painlessly by end-users (and by painlessly, I mean doesn't introduce lots of "It doesnt work for me"-type issues being created) then I don't see why not... however the only sticky thing might be how to test it via the travis CI runner. We have currently used the python unittest.mocks to patch the RPI.GPIO library (if my memory serves correctly) and this seemed to work reasonably well. If using a C++ module and relying on libgpiod means it is baking in a hard dependency that makes it difficult or impossible to test, this might be a deal-breaker.

Although having said all that... when you say it is 25% faster, have you got any hard numbers? If it can already support a frame-rate of say 200FPS for example on 16x2 character device, and the C++ lib boosts it to 250 FPS then I would say its not worth it (because the frame rate is already very high), but if it is say 4 FPS and it goes up to 5 FPS then it may be worth it

dhrone commented 3 years ago

Hold up on publishing this PR. @satiromarra discovered a GPIO cleanup bug in the backlight code. I'm chasing down what the correct clean-up approach should be.

dhrone commented 3 years ago

Getting back to the libgpiod question. It really depends on the display. For the two I am testing with (winstar weg010016a, hd44780 16x2), it really wouldn't be worth the effort. While I can physically get the data to the displays faster with the library, the amount of time it takes the displays to paint their screens is the limiting factor. For the hd44780s anything above about 5 fps looks horrible. For the weg010016a its about 30 fps. Above that you get tearing and other undesirable display artifacts. If you had a larger display, say 256x128, it might be worth it if the device doesn't support SPI.

thijstriemstra commented 3 years ago

In regards the C++ version, if it can be installed painlessly by end-users (and by painlessly, I mean doesn't introduce lots of "It doesnt work for me"-type issues being created) then I don't see why not...

There's some pi-specific registery enabled by default in raspbian that has pre-compiled native bindings for rpi. I hope it'll automatically do it for luma.* but not sure how it works (maybe preference based on popularity?)

thijstriemstra commented 3 years ago

I suggested an organization around luma.core.interface above.

That's unfortunate if it cannot be avoided, but it happens, and I would suggest a luma.core 2.0 for this.

dhrone commented 3 years ago

For the organization of luma.core.interface, if we use the approach I laid out in my last message, the only immediate change would be the movement and renaming of the current luma.core.interface.serial.parallel class to luma.core.interface.parallel.bitbang_6800. To prevent that from being breaking for the recent release of luma.oled we could cross import bitbang_6800 back to luma.core.interface.serial.parallel. I'd recommend we leave the discussion around the new interfaces I suggested for a later release. Thoughts?

dhrone commented 3 years ago

@rm-hull Is there anything else you'd like to see before a merge and release?

rm-hull commented 3 years ago

Yep I'm happy for it to be merged if @thijstriemstra is good for approval

thijstriemstra commented 3 years ago

To prevent that from being breaking for the recent release of luma.oled

It wasn't released yet.

s there anything else you'd like to see before a merge and release?

Maybe make the change to core first and then change/release luma.lcd?

dhrone commented 3 years ago

Quick update. Items in flight but unsubmitted:

luma.lcd and luma.oled documentations edited to make them consistent
new parallel module added to luma.core.interface
moved parallel class from luma.core.interface.serial, renamed to bitbang_6800 and placed in luma.core.interface.parallel

To do:

Update luma.core documentation to reflect new module/class.
Update tests to ensure bitbang_6800 functions correctly
Ensure changes still pass all functional, quality, and documentation checks
Issue PRs for all three projects

I'll try to get the to-do's done this weekend.

Is there anything I'm missing?

dhrone commented 3 years ago

Merging hd44780 branch in main

thijstriemstra commented 3 years ago

@rm-hull can you also add HD44780 to the github description (and any others if missing)? And publish a new release?

rm-hull / luma.lcd

Adding hd44780 driver #114