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

SPI read is discarding the first byte #37

Closed
GregDuckworthRenishaw opened this issue Jul 26, 2022 · 5 comments
Closed

SPI read is discarding the first byte #37

GregDuckworthRenishaw opened this issue Jul 26, 2022 · 5 comments
Labels
enhancement New feature or request

Comments

@GregDuckworthRenishaw
Copy link

Hi,

I am using an SPI device which does not implement SPI correctly and is causing trouble as a result; a MAX31855 Thermocouple Amplifier.

The interface they use is more basic than SPI typically uses insofar as the IC does not have a concept of internal addresses. They expect you to drive the chip select (CS) line low, then clock 32 bits of data out of the device, then drive the CS line high so it can read the temperature again. The IC doesn't even have an SDI pin as it never expects any data to be input.

So the difficulty is that your library (amazing btw, I am very much enjoying it) writes the address to the SPI device first, discarding the byte of data that is sent back during the initial SPI.transfer() call, then gets 4 bytes of data as it should.

A solution:

I have modified the Telemetrix4Arduino ino file to expect an extra command byte. The Arduino checks this byte to see if it should store the first byte and then acts accordingly.
I also had to modify telemetrix.py to allow this parameter to be passed through (I made the parameter default to False to preserve the functionality of existing code)

Updated Arduino Code
`
// read a number of bytes from the SPI device
void read_blocking_spi() {
#ifdef SPI_ENABLED
// command_buffer[0] == number of bytes to read
// command_buffer[1] == read register
// command_buffer[2] == Return initial transfer result too

// spi_report_message[0] = length of message including this element
// spi_report_message[1] = SPI_REPORT
// spi_report_message[2] = register used for the read
// spi_report_message[3] = number of bytes returned
// spi_report_message[4..] = data read

spi_report_message[1] = SPI_REPORT;
spi_report_message[2] = command_buffer[1]; // register
spi_report_message[3] = command_buffer[0]; // number of bytes read

if(command_buffer[2]){
// initial transfer data has been requested
// configure the report message
// calculate the packet length
spi_report_message[0] = command_buffer[0] + 4; // packet length

// write the register out. OR it with 0x80 to indicate a read
spi_report_message[4] = SPI.transfer(command_buffer[1] | 0x80);

// now read the specified number of bytes and place
// them in the report buffer
for (int i = 0; i < command_buffer[0] ; i++) {
  spi_report_message[i + 5] = SPI.transfer(0x00);
}
Serial.write(spi_report_message, command_buffer[0] + 5);

}
else{
// configure the report message
// calculate the packet length
spi_report_message[0] = command_buffer[0] + 3; // packet length

// write the register out. OR it with 0x80 to indicate a read
SPI.transfer(command_buffer[1] | 0x80);

// now read the specified number of bytes and place
// them in the report buffer
for (int i = 0; i < command_buffer[0] ; i++) {
  spi_report_message[i + 4] = SPI.transfer(0x00);
}
Serial.write(spi_report_message, command_buffer[0] + 4);

}

// configure the report message
// calculate the packet length
#endif
}
`

The Python was updated to this
`
def spi_read_blocking(self, register_selection, number_of_bytes_to_read,
call_back=None, return_all_data=False):
"""
Read the specified number of bytes from the specified SPI port and
call the callback function with the reported data.

    :param register_selection: Register to be selected for read.

    :param number_of_bytes_to_read: Number of bytes to read

    :param call_back: Required callback function to report spi data as a
               result of read command


    callback returns a data list:
    [SPI_READ_REPORT, count of data bytes read, data bytes, time-stamp]

    SPI_READ_REPORT = 13

    """
    if return_all_data:
        return_all_data = 1
    else:
        return_all_data = 0

    if not self.spi_enabled:
        if self.shutdown_on_exception:
            self.shutdown()
        raise RuntimeError(f'spi_read_blocking: SPI interface is not enabled.')

    if not call_back:
        if self.shutdown_on_exception:
            self.shutdown()
        raise RuntimeError('spi_read_blocking: A Callback must be specified')

    self.spi_callback = call_back

    command = [PrivateConstants.SPI_READ_BLOCKING, number_of_bytes_to_read,
               register_selection, return_all_data]

    self._send_command(command)

`

Keep up the great work.

@MrYsLab MrYsLab added the enhancement New feature or request label Jul 26, 2022
@MrYsLab
Copy link
Owner

MrYsLab commented Jul 26, 2022

@GregDuckworthRenishaw Thanks for the code. I will try it out with a "standard" SPI chip, and if all goes well will incorporate your suggestion in the next release. I have nothing planned for Telemetrix, so it may likely be a while before I get to this.

@GregDuckworthRenishaw
Copy link
Author

No Problem, I will be using my patched version for a while anyway.

Thanks

Greg

@MrYsLab
Copy link
Owner

MrYsLab commented Feb 4, 2023

@GregDuckworthRenishaw
Are you still interested in my implementing this feature? If so, would you be willing to test it since I don't have a device to do so? Thanks.

@MrYsLab
Copy link
Owner

MrYsLab commented Feb 7, 2023

@GregDuckworthRenishaw
I will close this issue since I cannot test any changes I make. If you wish me to reopen this issue, please leave a comment here.

@MrYsLab MrYsLab closed this as not planned Won't fix, can't repro, duplicate, stale Feb 7, 2023
@GregDuckworthRenishaw
Copy link
Author

I looked into altering the code, it was fairly straightforward (I used a flag in the command whether to put the first byte into the buffer), but because of the AGPL license I have had to stop working with Telemetrix altogether. No worries on closing the issue.
Greg

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

No branches or pull requests

2 participants