Skip to content
This repository has been archived by the owner on Apr 16, 2021. It is now read-only.

SPI.transfer16 not working as expected #85

Closed
rmlearney-digicatapult opened this issue Jun 19, 2020 · 3 comments
Closed

SPI.transfer16 not working as expected #85

rmlearney-digicatapult opened this issue Jun 19, 2020 · 3 comments

Comments

@rmlearney-digicatapult
Copy link
Contributor

rmlearney-digicatapult commented Jun 19, 2020

One expects SPI.transfer16() to be functionally equivalent to 2x sequential calls to SPI.transfer().

However the following functions produce very different results on my Nano 33 BLE communicating with an ADS1118, despite identical appearance. In particular, the function using SPI.transfer16() does not work but the SPI.transfer() one does.

int16_t ADS1118::getData(void){
  uint8_t buffer[2];
  int16_t retVal;

  SPI.beginTransaction(SPISettings(4000000, MSBFIRST, SPI_MODE1));
  buffer[0] = SPI.transfer(((_config >> 8) & 0xFF)); // MSB
  buffer[1] = SPI.transfer((_config & 0xFF)); // LSB
  SPI.transfer(0x00); // Have to do 32-bit write if !CS held low
  SPI.transfer(0x00);
  SPI.endTransaction();

  retVal = ((buffer[0] << 8) | buffer[1]);
  return retVal;
}
int16_t ADS1118::getData16(void){
  int16_t retVal;

  SPI.beginTransaction(SPISettings(4000000, MSBFIRST, SPI_MODE1));
  retVal = SPI.transfer16(_config);
  SPI.transfer16(0x0000); // Have to do 32-bit write if !CS held low
  SPI.endTransaction();

  return retVal;
}

This unexpected difference led to about 4 weeks of hardware debugging, and I advise anyone using SPI.transfer16() on the Nano BLE 33 to rewrite your drivers to use 2x 8-bit transfers via SPI.transfer() until this is solved.

Would really appreciate a review of MBed's SPI system to explain why these are not functionally equivalent.

facchinm added a commit that referenced this issue Jun 22, 2020
Borrow implementation for samd core
Should fix #85
@facchinm
Copy link
Member

Indeed, the current SPI transfer16 implementation does not consider MSBFIRST or LSBFIRST defines, using the processor's builtin byte arrangement. I just pushed 92833a2 to fix it, let me know if it works so it can be merged safely.

@rmlearney-digicatapult
Copy link
Contributor Author

@facchinm that looks like it fixes it, thank you!

@rmlearney-digicatapult
Copy link
Contributor Author

rmlearney-digicatapult commented Jun 26, 2020

@facchinm I've been trying to think how this works in terms of endian-ness and could you maybe clarify for me?

You make a 16-bit word union from 2x 8 bits that goes <15 ... 8> <7 ... 0>

So if you send MSBFIRST it sends out bits in the following order:

<15 ... 8> <7 ... 0>

But if you send LSBFIRST it just swaps the order of the bytes, not the bits, and sends:

<7 ... 0> <15 ... 8>

So how does this actually solve the problem to make sure the SPI bus is transmitting the whole byte MSB (bit 16) or LSB (bit 0) first?

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

No branches or pull requests

2 participants