-
Notifications
You must be signed in to change notification settings - Fork 13
Changing the spi baudrate does not work. #8
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
Comments
This needs to be updated to use SPIDevice. It is currently using the SPI bus directly. |
I will be working on updating this to use SPIDevice this week. |
After looking into this a bit more I think I misunderstood initially what needed to be done. I was thinking the library needed updating to use SPIDevice. But the library takes whatever object you pass in the constructor and uses it. My understanding now is that it's the example code that would need to get updated to use SPIDevice. Looking on that learn guide page a bit further down I see this section which is showing the use SPIDevice instead of directly handling the bus. Perhaps this section added sometime after the issue was created, I'm not sure if there is a way to view the change history. @caternuson does that section of learn guide handle you had in mind? Or is there something more still needed? |
That's the general approach. You would still pass in a SPI bus instance. But you would then use that to create a SPIDevice instance. All subsequent code would use the the SPIDevice and not the SPI instance. It's similar to how I2C / I2CDevice is used in I2C drivers. Might help to look at one of those to also see how a context manager is used. |
@FoamyGuy Are you still working on this? |
@kattni To the best of my understanding there is nothing further that needs done. My understanding of what was needed was updating the example code in the Learn Guide to use the SPIDevice object instead of "manually" setting up the SPI connection. There is a section that illustrates the use of SPIDevice here: https://learn.adafruit.com/circuitpython-basics-i2c-and-spi/spi-devices#spidevice-library-4-21 My best guess is that at the time this issue was created that section of the learn guide did not exist yet. Or perhaps there was some confusion because the learn guide covers the "manual" way of setting up SPI first before then saying that the SPIDevice is easier and the best practice? Maybe the order of those concepts should be swapped within the guide? Show the best practice easy way first. Then afterward dig into the more manual way to show underneath the hood some? If I am mistaken and there is something more that still needs doing I am happy to help out with it but I'll need a point in the right direction. |
@caternuson Can you weigh in? |
Well, huh. OK. This board is a bit special. See here: I think this issue is still unresolved though. As stated in first post - the baudrate is hardwired and not using the current bus setting. This same issue looks like it exists in the TLC5947 library: |
@FoamyGuy It looks like there are changes that need to be made. If possible, work with @caternuson to see how it can be resolved. |
Okay I can start by trying to use the existing SPI bauderate instead of the hard-coded value and test to make sure that is still working. I can work on it this week. |
Fixed by #14 |
Using Adafruit_Blinka on Jetson Xavier and
Following: https://learn.adafruit.com/circuitpython-basics-i2c-and-spi/spi-devices instruction, the SPI baudrate stay at 10Mhz instead of the new specified value.
sample test code will print 10000000 instead of 500000
The problem comes from the _write(self) function (line 177):
self._spi.configure(baudrate=10000000, polarity=0, phase=0)
It's not using the previously set baudrate. The line could be instead:
self._spi.configure(baudrate=self._spi._spi.baudrate, polarity=0, phase=0)
but that feels incomplete as polarity and phase are not kept.
Should the values be kept in https://github.com/adafruit/Adafruit_Blinka/blob/master/src/busio.py SPI class and calling configure without parameter will reuse the previously set? I have no idea of the impact on other project and will let you decide the best course of action.
The text was updated successfully, but these errors were encountered: