-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New quirk for sh1107 #3450
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
New quirk for sh1107 #3450
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems fine if it is working, though from one of your comments it sounds like it's not working yet. I'd also want to find out from @tannewt if the "column_and_page_addressing" flag should go in the "core" object or stay where it's been placed.
Thanks Jeff. To clarify, Scott asked me to post a draft PR so he could review and help me get past this problem with the bindings. I don't know what I missed in trying to pass the new parameter from the driver back to the display core. -mark. |
@mdroberts1243 I don't see anything wrong with your code here or in your test library. That error can happen if the name of a kwarg doesn't match one in the parse table. Maybe a typo? Does it go away if you comment out the new kwarg? If so, maybe you don't have your code loaded on the device. You can do |
SH1107 DisplayIO Support UpdateI thought I would use this draft PR to keep posting progress on trying to get SH1107 supported under DisplayIO. TL;DR: The Good
The Bad
The Ugly
Where to go from here?
Thanks for making it this far! |
I think you can add a second quirk (or just call this the sh quirk and have it do two things.) The second thing you could do is change the subrectangle that is updated here: https://github.com/adafruit/circuitpython/blob/main/shared-module/displayio/Display.c#L277 Basically change each subrectangle to be the smallest area you can update. |
Thanks. I might try switching to 'vertical' mode as well, then the display will have a horizontal orientation naturally without the user having to set it up. Console output will work better too in that case. Bandwidth is somewhat limited next few days, so progress might be slow... |
I have done a clean up and committed changes "Mostly-working-version with comments". I have also updated the driver and the example code: https://github.com/mdroberts1243/mdroberts1243_CircuitPython_DisplayIO_SH1107 (don't know how to contribute that!) Currently everything looks like it is working except label text (bitmap_label will work fine). An easy way to see the problem with text is to run the example code and then enter the REPL. You will see that the terminalio<?> output works fine but when you enter code into the REPL the text you enter will wrap back to the beginning of the line at about the 1/2 way point on the screen. See the circuitpython discord for a picture. PS Forgot to mention I changed the name of the quirk to "SH1107_addressing" and changed to the vertical mode of the display driver, so now each row is one pixel deep and 128 pixels wide in 16 bytes of data. This makes the display naturally horizontal. PPS One thing that worries me is that you need to be on byte boundary in x with your subrectangle since each byte will paint 8 pixels horizontally. I wasn't sure that was handled correctly but I tried different odd starting points and things seem to work o.k. |
I fixed the masking error I had in setting the page on the controller and now all the basic operations work. You can use it with the REPL even. How do I contribute the driver and example code linked above? Not sure how to create a "Pull Request" for something new. Let me know if you need anything else. -mark. |
For the driver we can either fork your copy into the adafruit org or you can transfer it. We'll maintain it since we sell a product with the display. Would you like us to fork your repo? |
Also, congrats! You found all of the boards with displays initialized in board_init. You'll need to add extra |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really nice work @mdroberts1243, thank you! Just a couple suggestions to make the code clearer. You will also need to fix up the display construct calls for boards with built in displays. Thanks!
Yes, please fork and maintain! I'm willing to transfer it as well, but don't know how that is accomplished. |
O.k. That was a bit tedious! Also, there is an issue in build-arm (electronut_labs_blip) where it complains about too few arguments on the set region to update... I suppose they've made their own copy of that for some reason? I probably need to change that too... |
I have made all the requested changes and updates. But a few boards are still failing the checks but due to messages related to the number of arguments passed to displayio_display_core_set_region_to_update. The errors seem to be the same. Boards include: electronut_labs_blip, pewpew_m4, pybadge, pybadge_airlift, pygamer, pygamer_advance, ugame10. Many of these didn't show this problem until after the original common_hal function changes were made. @tannewt I need help understanding why these errors are being thrown and what is causing them... as far as I know the files they are related to are common to all the boards! Every board should see the same number of arguments into displayio_display_core_set_region_to_update Also, in testing the new logic I decided to move a label around the screen by a pixel or two in x and y and see if there were any issues with strange clipping etc. This heavily loaded the I2C bus and I found that after a few seconds the program/system would lock up, freeze the display and only be recovered by a hard reset. My conditions:
Is there a known issue with I2C? Or this combination of stuff? Alpha CP? STM too new? I also have seen the software not release the I2C properly (requires reset), which might be related to the above problem too. -mark. |
A small status update:
So I'm concluding that there's an issue with the STM32F405 port and/or CircuitPython 6.0 alpha on the STM Feather. I confirmed the STM I2C bus is at 400kHz and that is supported by the chip and the display controller. I don't know what speed the GrandCentral M4 Express is running at, but it updates quite quickly. |
@mdroberts1243 I think those are the boards with the I think you'll want to pull the parameter out of the display object like the other params. |
@tannewt I made the change to _stage and rebuilt the Grand Central M4 Express just to make sure nothing was broken. I had to resolve one conflict and merged it in. But now the checks are queued and seem to be stuck. It is the checks that I want to see! -mark. |
@tannewt The checks failed. Failing in a different way then before and the various board builds didn't run at all. The only thing different was I was asked to resolve a conflict, which I did, accepting a bunch of changes to seeduino wio terminal apparently in order to add my quirk to the end of the display construct. In ports/atmel-samd/boards/seeeduino_wio_terminal/board.c Anyway, things seem to have regressed and I'm at a loss to interpret the failure. |
Note that the one build failure is a clone failure and can be ignored. |
b34e2bf
to
ef245ef
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @mdroberts1243! This looks good.
Driver repo is here: https://github.com/adafruit/Adafruit_CircuitPython_DisplayIO_SH1107 |
This change introduces a new quirk to support the FeatherWing SH1107 128x64 OLED display. A parameter, "column_and_page_addressing" when set to true in the driver will cause the displayio_display_core_set_region_to_update function to use the column lower, column higher and page set commands on the sh1107.
I wrote a new driver to test with: https://github.com/mdroberts1243/mdroberts1243_CircuitPython_DisplayIO_SH1107
Right now python is throwing an error that I'm sending too many keyword arguments to the super().init