bme280-rs icon indicating copy to clipboard operation
bme280-rs copied to clipboard

SPI communication doesn't work

Open wiktorwieclaw opened this issue 1 year ago • 6 comments

The following code is not right, as it would require the data to be prepended with a dummy byte

impl<SPI, CS> SPIInterface<SPI, CS>
where
    SPI: Transfer<u8>,
    CS: OutputPin,
{
    fn read_any_register(
        &mut self,
        register: u8,
        data: &mut [u8],
    ) -> Result<(), Error<SPIError<SPI::Error, CS::Error>>> {
        self.cs
            .set_low()
            .map_err(|e| Error::Bus(SPIError::Pin(e)))?;
        self.spi
            .transfer(data, &[register])
            .map_err(|e| Error::Bus(SPIError::SPI(e)))?;
        self.cs
            .set_high()
            .map_err(|e| Error::Bus(SPIError::Pin(e)))?;
        Ok(())
    }
}

Spitting the transfer would do the job:

bus.write(&[register])?;
bus.read(data)

wiktorwieclaw avatar Jan 08 '23 15:01 wiktorwieclaw

Fixed with https://github.com/VersBinarii/bme280-rs/pull/23

VersBinarii avatar Jan 16 '23 18:01 VersBinarii

@VersBinarii I believe that #23 doens't fix the issue. The actual fix is this:

From:

self.spi
    .transfer(data, &[register])
    .map_err(|e| Error::Bus(SPIError::SPI(e)))?;

To:

self.device
    .transaction(|bus| {
        bus.write(&[register])?;
        bus.read(data)
     })

Please reopen the issue, I'll prepare a new, updated PR that resolves conflicts.

wiktorwieclaw avatar Jan 16 '23 18:01 wiktorwieclaw

Have you tested it though? I dont have the hardware on my desk to confirm it. I'll get some by the end of the week.

VersBinarii avatar Jan 16 '23 18:01 VersBinarii

@VersBinarii Yup. I'll test it again on Wednesday just to be sure.

For now I could try to explain why the current approach doesn't work.

Look at impl of transfer from stm32f4xx_hal:

fn transfer(&mut self, buff: &mut [W], data: &[W]) -> Result<(), Self::Error> {
    assert_eq!(data.len(), buff.len());

    for (d, b) in data.iter().cloned().zip(buff.iter_mut()) {
         nb::block!(<Self as FullDuplex<W>>::write(self, d))?;
         *b = nb::block!(<Self as FullDuplex<W>>::read(self))?;
     }

     Ok(())
}

Let's say we want to read 3 bytes from address 0xb7. For each byte we want to read, we need to provide a dummy byte to write. So, something like this wouldn't work:

let write = [0xb7];
let mut read = [0x00; 3];
spi.transfer(&write, &mut read)

If we really want to use transfer method, we shall do this:

let write = [0xb7, 0x00, 0x00, 0x00];
let mut read = [0x00, 0x00, 0x00, 0x00];
spi.transfer(&write, &mut read);

let response = &read[1..];

HAL_SPI_TransmitReceive from the C HAL works exactly same.

Note that I didn't use transfer in my PR to avoid slicing and copying the response from the read buffer.

wiktorwieclaw avatar Jan 16 '23 19:01 wiktorwieclaw

I see, you're correct. Feel free to make a PR then if you wish 😁 If not i'll look at fixing this later this week.

VersBinarii avatar Jan 17 '23 13:01 VersBinarii

FYI, I connected everything today, but I'll finish the PR tomorrow :P

wiktorwieclaw avatar Jan 20 '23 21:01 wiktorwieclaw