-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Fix BusDisplay transaction handling #10571
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
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.
Thanks for the PR! I'm not sure this is the way we want to do it though. (More inline)
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.
I'm worried all of these deletes are going to break something due to changing chip select issues. I wonder if we should split out the bus locking from chip select.
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.
Makes sense, I was unaware that some display drivers are fussy about when chip selects are toggled. I can change it back into individual bus transactions for now, and spin in any cmd sequences. This would eliminate the possibility of regressions.
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.
That sounds like a good plan. Thanks!
Fixes #10569
I've tested on a m5stack_cores3 with no apparent regressions on display handling. This strangely does not resolve #10536 so some other issue must also be present, however I can see on an oscilloscope that the SPI bus is now being shared correctly between busdisplay and sdcardio. One thing to note is running spi = board.SPI(); spi.try_lock() will now prevent the screen from being updated until the lock is released - this is a limitation of the the hardware.
I have also applied the same/similar fixes to EPaperDisplay - however I do not have hardware to verify this.