-
-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
sw spi bus: Added support for configured width and encoding #6633
base: master
Are you sure you want to change the base?
Conversation
92fafa8
to
2ab93d7
Compare
Enables specifying any width (5 to 64 bits) for the SW SPI bus and defining encoding big/little-endian and LSB/MSB bit-first. Signed-off-by: Alexander Bazarov <[email protected]>
42c2894
to
abeb372
Compare
The code is black magic to me, but I can confirm this makes the display work on the A10T I converted to Klipper. At the very least it makes getting these junky and bug-ridden printers into a stable configuration that much easier. |
Thank you for your contribution to Klipper. Unfortunately, a reviewer has not assigned themselves to this GitHub Pull Request. All Pull Requests are reviewed before merging, and a reviewer will need to volunteer. Further information is available at: https://www.klipper3d.org/CONTRIBUTING.html There are some steps that you can take now:
Unfortunately, if a reviewer does not assign themselves to this GitHub Pull Request then it will be automatically closed. If this happens, then it is a good idea to move further discussion to the Klipper Discourse server. Reviewers can reach out on that forum to let you know if they are interested and when they are available. Best regards, PS: I'm just an automated script, not a human being. |
Self review:
|
Thanks. As high-level feedback, I am leery of making such an intricate change to a common component in support of very rare hardware. The concern is that a change to this common code could introduce a regression (either directly or via a performance degradation). Which devices did you have in mind for this support? Is there any other way to drive these devices without requiring a full 9-bit (or other) byte order? For example, arranging to always send commands in multiples of nine 8-bit bytes. I have not done a full code review, but I did notice that the C code on this PR introduces an arbitrary integer division (along with a modulo). Most micro-controllers do not directly support division (nor modulo) and using these operators can result in slow and bloated code being introduced by the compiler. Cheers, |
Thanks for the feedback.
Hopefully, the 8 commands solution will work, so I will be able to update #6639 without this PR (and I hope adding display driver doesn't fall in "no go zone":). |
The proposed sending 8 commands as 9 bytes actually works. I'm not fixing this, as it is not intendent to be merged now.
|
Adds option to work with any device/sensor unrelated to HW SPI bus width.
Implemented to work with
YHCB2004
display onGeeetech A10/M/T
printers that require 9-bit SPI commands.Test results
5 bit command (less than byte)
12 bit command (non integer number of bytes)
32 bit command (standard for many sensors)
64 bit command (artifitially set max)
transmitting 20 bits
0x80f05
testing all 4 options of encoding of
0x80f05
big/little-endian
lsb/msb bit first