Skip to content

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

Closed
FrancoisGuimond opened this issue Nov 6, 2019 · 11 comments
Closed

Changing the spi baudrate does not work. #8

FrancoisGuimond opened this issue Nov 6, 2019 · 11 comments
Assignees
Labels
bug Something isn't working

Comments

@FrancoisGuimond
Copy link

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

spi = busio.SPI(board.SCK, MOSI=board.MOSI)
while not spi.try_lock():
    pass
spi.configure(baudrate=500000)
spi.unlock()

leds = adafruit_tlc59711.TLC59711(spi)
leds[0] = (0, 0, 0)
print(f'*** SPI Clk: {spi._spi.baudrate} ******')

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.

@caternuson
Copy link

This needs to be updated to use SPIDevice. It is currently using the SPI bus directly.

@FoamyGuy FoamyGuy self-assigned this Mar 2, 2020
@FoamyGuy
Copy link
Contributor

FoamyGuy commented Mar 2, 2020

I will be working on updating this to use SPIDevice this week.

@FoamyGuy
Copy link
Contributor

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?

@caternuson
Copy link

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.

@kattni
Copy link
Contributor

kattni commented May 4, 2020

@FoamyGuy Are you still working on this?

@FoamyGuy
Copy link
Contributor

FoamyGuy commented May 4, 2020

@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.

@kattni
Copy link
Contributor

kattni commented May 4, 2020

@caternuson Can you weigh in?

@caternuson
Copy link

Well, huh. OK. This board is a bit special. See here:
adafruit/Adafruit_CircuitPython_TLC5947#9 (comment)
So, yah, maybe nevermind about what I said about SPIDevice.

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:
https://github.com/adafruit/Adafruit_CircuitPython_TLC5947/blob/master/adafruit_tlc5947.py#L161

@kattni kattni added the bug Something isn't working label May 4, 2020
@kattni
Copy link
Contributor

kattni commented May 4, 2020

@FoamyGuy It looks like there are changes that need to be made. If possible, work with @caternuson to see how it can be resolved.

@FoamyGuy
Copy link
Contributor

FoamyGuy commented May 4, 2020

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.

@kattni
Copy link
Contributor

kattni commented Aug 17, 2020

Fixed by #14

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants