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

Bitbang SPI phase=1 lowest bit is wrong #20

Closed
gingle opened this issue Sep 6, 2021 · 11 comments · Fixed by #31
Closed

Bitbang SPI phase=1 lowest bit is wrong #20

gingle opened this issue Sep 6, 2021 · 11 comments · Fixed by #31

Comments

@gingle
Copy link

gingle commented Sep 6, 2021

CircuitPython version

Adafruit CircuitPython 6.1.0 on 2021-01-21; Adafruit QT Py M0 with samd21e18 also
Adafruit CircuitPython 7.0.0-beta.0-161-g65753a1c2 on 2021-09-01; with samd51j19

Code/REPL

# 09/03/2021
# looses both least significant bits
# tested on QT Py and Metro_M4_Express

import board
import digitalio
import adafruit_bitbangio as bitbangio


nEN = digitalio.DigitalInOut(board.A1)    
nEN.direction = digitalio.Direction.OUTPUT

spi = bitbangio.SPI(board.SCK, MOSI=board.MOSI, MISO=board.MISO)

def TestSPI():  
    select = nEN
    select.value = True
    while not spi.try_lock():
        pass
    spi.configure(baudrate=100000, polarity=0, phase=1, bits=8 )
    bytes_read = bytearray(4)
    select.value = False
    bytes_read = bytearray([0x33])  # changes 0x33 to 0x32 
    spi.write(bytes_read)
    bytes_read = bytearray([0x0b])  # changes 0x0b to 0x0a
    spi.write(bytes_read)
    select.value = True
    spi.unlock()

Behavior

image

Description

No response

Additional information

This works correctly:
from adafruit_bus_device.spi_device import SPIDevice

@jepler jepler changed the title Bitbang SPI looses least significant bit Bitbang SPI loses least significant bit Sep 6, 2021
@dhalbert
Copy link
Contributor

dhalbert commented Sep 7, 2021

Thanks for the report. A couple of questions:

  1. Does your device require phase=1? Is it misbehaving?
  2. Did you set the Saleae Logic SPI analyzer to CPHA=1? Screenshot below is from Logic 2.3.36:
    spi-phase-1

The operative code is here, and looks OK to me on first glance:
https://github.com/adafruit/circuitpython/blob/d29a12cd81c12b76ce94317074c6396755ccde6c/shared-module/bitbangio/SPI.c#L152-L167

@dhalbert dhalbert changed the title Bitbang SPI loses least significant bit Bitbang SPI phase=1 lowest bit is wrong Sep 7, 2021
@gingle
Copy link
Author

gingle commented Sep 7, 2021 via email

@dhalbert
Copy link
Contributor

dhalbert commented Sep 7, 2021

Thanks, your inline image did not come through. Could you post directly in GitHub?
Which DDS chip is it?

[I edited your reply to remove the repetition of my post.]

@gingle
Copy link
Author

gingle commented Sep 7, 2021 via email

@dhalbert
Copy link
Contributor

dhalbert commented Sep 8, 2021

I just realized you are using the https://github.com/adafruit/Adafruit_CircuitPython_BitbangIO Python-based library (import adafruit_bitbangio as bitbangio), not the built-in bitbangiomodule. Do you see the same problem when using native import bitbangio?

@dhalbert
Copy link
Contributor

dhalbert commented Sep 8, 2021

Could you send any replies to ***@***.******@***.***>? I am doing most of my work at home and have my files on my home computer.

We use GitHub so everyone can see the replies. When you get an email from GitHub, don't reply via mail, but instead click the "View it on GitHub" link in the email:

view-it-on-github

That will bring you directly here so you can reply.

@dhalbert dhalbert transferred this issue from adafruit/circuitpython Sep 9, 2021
@dhalbert
Copy link
Contributor

dhalbert commented Sep 9, 2021

@gingle
Copy link
Author

gingle commented Sep 9, 2021

Dan, you are correct. I verified the built in version of bitbangio as working correctly. the Adafruit version has the LSbit problem.

@Linkeor
Copy link
Contributor

Linkeor commented Dec 8, 2023

Bonjour,
I don't think I'm in the wrong place. I had to test a max31856 and a tft st7735R with bitbangio, on a pico/u2if. I can read but not write in the registers, or at least the values that come out of the register are not at all those written and of course my temperatures are wrong and the screen displays nothing.
After exploring and testing a lot of things I modified def write( in adafruit_bitbangio.py and everything now works (for the max31856 in phase 1 as for the st7735 in phase 0)
This happens on lines 401 and 407 where
401
self._sclk.value = self._polarity
becomes
self._sclk.value = not self._polarity
and 407
self._sclk.value = not self._polarity
becomes
self._sclk.value = self._polarity

I don't know if this is the best way to do things and if it could have side effects, I'm not an experienced developer. But for now this is a modification that works for me.
I hope this can help.

PS sorry, for English via google translate, I read well but I speak English "like a Frenchman" ;-)

Linkeor added a commit to Linkeor/Adafruit_CircuitPython_BitbangIO that referenced this issue Dec 11, 2023
in connection with adafruit#20
tested on max31856 (phase 1) and st7735R (phase 0) on a raspberry pico/u2if.
Also solves the problem of a max31856 on raspberry pi (impossible to set the thresholds)
@chipgarner
Copy link

Please accept @Linkeor 's pull request as above! The MAX31956 does not work using BitbangIO without this fix. Some kiln-controller users (https://github.com/jbruce12000/kiln-controller) cannot upgrade their software without re-soldering as software pins will not work with this library on the '56.

I have tested @Linkeor 's changes on an RPi 3+ with both the MAX31855 and '56. They work in all the expected cases, hardware pins / software SPI via BitbangIO, hardware pins / hardware SPI, software pins / BitbangIO.

This change does not effect the MAX31855 but it seems to be needed for any board that uses MOSI.

@kbsriram
Copy link
Contributor

kbsriram commented Feb 26, 2024

While writing some tests, I came across what might be a timing bug introduced by #30

This is the test code:

    with adafruit_bitbangio.SPI(clock=clock, MOSI=copi, MISO=cipo) as spi:
        spi.try_lock()
        spi.configure(baudrate=100, polarity=0, phase=0, bits=8)
        echo.enable(True)
        spi.write([0b10101011])

Prior to the change, as noted in the initial description here, there was a general offset problem where bits were pushed out one clock transition too late. However, each bit was still pushed on the data line at the right clock phase.

Here is a sample trace before the fix. The first bit is set on the first falling clock, but the first rising clock has already been read by that time. So the peripheral effectively sees a right-rotated version of the data in this case. However, each bit is still pushed during the falling clock transition, it doesn't interfere with the read.

mode_0_prev

However, after the fix the bits are set on rising clock transitions, when they are also read.

mode_0_after

Possibly many peripherals are still able to decode it as there's just enough delay between the data line being set and the clock being immediately transitioned in python land; but I'm not sure if this is ideal.

(Happy to provide a fix if this seems worth correcting, as I noticed this while testing the protocol; rather than seeing issues in the wild.)

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

Successfully merging a pull request may close this issue.

5 participants