Skip to content
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

Proposal: deprecate xfer2 #99

Open
Gadgetoid opened this issue May 29, 2020 · 5 comments
Open

Proposal: deprecate xfer2 #99

Gadgetoid opened this issue May 29, 2020 · 5 comments

Comments

@Gadgetoid
Copy link
Contributor

This library has suffered somewhat from function creep caused by useful features being added in conjunction with a fear of breaking the existing API.

I think it's worth reviewing what functionality an SPI library should constitute- taking into account #83 and the possibility of reading/writing bytearrays. But before we dive into what could be a time consuming nightmare it migh tbe worth reviewing the current functionality and attempting to whittle the codebase down to a maintainable core.

Right now as near as I can tell from code-review xfer and xfer2 are functionality identical unless the library is compiled with -DSPIDEV_SINGLE. I'm unaware of how this library is packaged for RPi (I have asked who I believe to be the relevant individual) but this flag is not referenced or set anywhere in the packaging visible in this repository. The library is shipped on PyPi without this flag set. The lack of any functional difference between xfer and xfer2 was noticed in #35 and #25 and has remained unchanged since.

I don't believe there's any reason to have a first-class method that handles re-asserting chip-select between "blocks" (in this case, we mean bytes).

  • Nobody has asked for it (xfer does not de-assert CS between bytes #35 notwithstanding)
  • nobody has discovered or mentioned -DSPIDEV_SINGLE
  • anyone who desires this behaviour can simply invoke xfer byte-by-byte from Python (or ask for it)

I propose we quietly roll xfer and xfer2 into a single function and drop the -DSPIDEV_SINGLE code path altogether. Along with dropping some of the Python version checks this constitutes a significant reduction in code that needs maintained and testing in future.

Right now there are also inexplicable minor differences (allowing/disallowing threads) between xfer and xfer2 that should be ironed out, so this would be a good opportunity to reconcile those.

@Gadgetoid
Copy link
Contributor Author

Have confirmation that -DSPIDEV_SINGLE is never set- at least in the Raspberry Pi distribution.

Are there any other binary distributions that might set this flag and suffer from that code being removed?

I supose at the very least if the bugfixes are rolled out a release before this code is killed, then it will always be possible for anyone who needs this functionality to rely on a previous release (everyone pins their dependencies right, right!?).

@Elektrozeugs
Copy link

Hi,
I tried to use xfer because I have a chip that needs a CE spike between any 8bit transaction. With my Logic analyzer I can see that this is not working with the function. Is there any easy way to get the "old" xfer function as described on the main page?
(PS: sending xfer byte-by-byte is no option for me, because I need a fast transaction without delay)
Best wishes

@semininja
Copy link

semininja commented Jan 18, 2021 via email

@Elektrozeugs
Copy link

I am sending RGB data to a display, so I need a fast transmission.
Here is my test code

import spidev
import time
spi = spidev.SpiDev()
spi.open(0, 0)

spi.xfer([0xFF, 0xFF, 0xFF],1000000, 0, 8)
spi.xfer([0xFF],1000000, 0, 8)
spi.xfer([0xFF],1000000, 0, 8)
spi.xfer([0xFF],1000000, 0, 8)

spi.close()

SPItest

As you can see the first trasmission is a lot faster.
You can also see a bug that the CE is dropped twice which is mentioned here:
https://www.raspberrypi.org/forums/viewtopic.php?t=73323
but sadly it wasnt solved

@AI0867
Copy link

AI0867 commented Jul 8, 2024

I realize this issue has been silent for a while, but to me, your usecase suggests the addition of a new xfer4, which takes a list of lists of values, running each list in a separate transaction.

Your test behaviour (which I know isn't the behaviour you want) could then be reproduced, (but faster, and without the spurious CE0 transitions) with:

spi.xfer4([
    [0xFF, 0xFF, 0xFF],
    [0xFF],
    [0xFF],
], 1000000, 0, 8)

As for deprecating xfer or xfer2, given that xfer2 is the function that actually does what its documentation says it does, I would suggest keeping that one, so that people who use xfer, and might expect the documented behaviour, get a heads up and get to choose which semantics they want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants