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

Support sending strings with multi-byte-wide SPI #3529

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

stuartpb
Copy link

The iteration in the loop in the prior version of spi.send assumes that the value of databits is the width of char when sending any string value.

  • This PR is for the dev branch rather than for the release branch.
  • This PR is compliant with the other contributing guidelines as well (if not, please describe why).
  • I haven't thoroughly tested my contribution - it's a simple enough change that, if it compiles and passes existing tests, it works (it only fixes behavior that was previously broken).
  • The code changes are reflected in the documentation at docs/*. (This only fixes the function to more closely match the expected documented behavior.)

The iteration in the loop in the prior version of spi.send assumes that the value of databits is the width of  `char` when sending any string value.
@stuartpb
Copy link
Author

There should probably be further tests with this to ensure that multi-char values from strings get read and sent properly - there could be endianness issues with the string layout.

Alternately, trying to send a string value with a databits value of anything other than 8 could be an error, or ignore the databits value and always use a width of 8 for strings, transmitted character-by-character (this override should probably be documented in that case).

@jmattsson
Copy link
Member

I don't see how platform_spi_send( id, spi_databits[id], pdata[ i ] ); would work with >8 bits? The pdata[i] would only pass 8 bits of data through to the function, given that pdata is of const char * type. Am I missing something?

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 this pull request may close these issues.

2 participants