STM32: DisplayIO support, common hal additions #2265
Conversation
|
ok this is dependant on PWM so we need to get that merged in first :) |
oh right boo git CI. Here's a bin for the build |
|
oled does work, but TFT ILI341 did not, i do see the right pins toggling so a little mysterious. will require more research! |
|
no |
|
@ladyada is this the tft featherwing? The 2.4" one? |
|
yes, the 2.4" is ILI9341, the 3.5" ix HX8357D |
|
Got it. I can take a closer look as soon as the A2 Saleae frees up again (should I request one of those?) |
|
yes order a https://www.adafruit.com/product/2512 |
|
hiya is there something for me to re-review? |
|
@ladyada no sorry I just noticed you weren't being tracked as a reviewer. Were you still digging into the TFT issue? If you'd like me to pick up on that instead, could you let me know what issues you were seeing in a little more detail so I can hone in on it? |
|
i have not done any digging into the TFT issue, you can spend 3-4 hours trying to debug it. if you have not figured it out by then, post here |
fix weird non-conflict
tannewt
left a comment
tannewt
left a comment
There was a problem hiding this comment.
Thanks for adding the shared pin level reset. Makes total sense to not expose the struct members outside of the port.
A couple small suggestions but good otherwise.
| void common_hal_never_reset_pin(const mcu_pin_obj_t* pin); | ||
| void common_hal_reset_pin(const mcu_pin_obj_t* pin); |
There was a problem hiding this comment.
I know this hasn't been said anywhere but I only use the common_hal_ prefix for shared API that maps to Python API. For cross-port functions, they should just be added here without a prefix like assert_pin and assert_pin_free already.
There was a problem hiding this comment.
That makes total sense, but now I'm not really sure what to name these. I was reluctant to redo the original functions so I wouldn't risk breaking functionality in other sections of the ports and shared bindings, so I mimicked what was done with common_hal_mcu_pin_is_free, which is a common-hal alias on top of a port-level function.
It seems like there's a distinction between functions and structures that ONLY exist at the port level, which are essentially helpers, and stuff that's exposed in shared bindings, even if it isn't a 1:1 map to python. Do you think a new prefix would be appropriate for functions like this? port_hal or some such?
|
@ladyada regarding TFT featherwing: logic analyzer comparison to reveals that there's no obvious pin level issues with the SPI connection, such as something being disabled or otherwise not communicating properly. The two feathers both have active SPI but send very different data on different intervals. |
|
@ladyada I've confirmed SPI is still working fine with the NOR flash chips and my other screens and devices, so the issue with this may be coupled to the python component. I'll do my best to try and figure it out but anyone you can think of with more python experience would be a welcome testing addition. |
|
please get all other issues/PRs for STM resolved, and we'll visit this after |
Should this PR still get merged, for other displays? Or should it wait on the TFT issue? |
|
it should wait becuse there is some underlying issue |
|
Closing due to re-forking to fix merge issues. |
3 participants
This PR adds support for DisplayIO to the STM32 port, using a stub for ParallelBus for now. Tested with an SSD1306 on the Feather_STM32F405_Express. PWMOut is a prerequisite to this branch.
Additionally, this PR suggests two new definitions to be added to the common hal in shared-bindings,
common_hal_reset_pin, andcommon_hal_never_reset_pin. These avoid exposing the internal structure of the pin object, which varies between ports. They do not replace the port level versions and should not affect other parts of circuitpython other than DisplayIO.