MrYsLab / telemetrix

A user-extensible replacement for StandardFirmata. All without the complexity of Firmata!
GNU Affero General Public License v3.0
68 stars 26 forks source link

SPI read is discarding the first byte #37

Closed GregDuckworthRenishaw closed 1 year ago

GregDuckworthRenishaw commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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

Thanks

Greg

MrYsLab commented 1 year ago

@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 commented 1 year ago

@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.

GregDuckworthRenishaw commented 1 year ago

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