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

Update SimpleHalSpiDevice transaction to execute all the operations together in a transaction #152

Merged
merged 2 commits into from
Aug 20, 2024

Conversation

whatisbyandby
Copy link
Contributor

@whatisbyandby whatisbyandby commented Aug 17, 2024

This should resolve Issue #151

  • Updated the SimpleHalSpiDevice to take a concrete Spi instance, instead of a generic hal bus
  • Updated the fn transaction to map the hal spi operations to segments, so they can be executed with transfer_segments in a single transaction

…spi operations to segments, so they can all be executed together as a single transaction
Updated the SimpleHalSpiDevice transaction implementation to map the …
@whatisbyandby whatisbyandby marked this pull request as draft August 17, 2024 05:07
@whatisbyandby
Copy link
Contributor Author

I finished the code, and it seems to be working as expected when I interact with the RFM69 chip I have connected to the Raspberry Pi. I wanted to submit the code, so you could provide feedback, but before it's merged I'd like to ensure everything looks good on the oscilloscope. I should have time to do that tomorrow.

Cheers

@golemparts
Copy link
Owner

Thanks for the PR! From a quick glance, the code changes look good. I won't have time to run any tests on this myself, so I'm looking forward to your results before merging.

@whatisbyandby
Copy link
Contributor Author

whatisbyandby commented Aug 17, 2024

I think things are looking good.

Here is the code snippet I used for testing

use embedded_hal::spi::{Operation, SpiDevice};
use rppal::spi::{Bus, Mode, Segment, SimpleHalSpiDevice, SlaveSelect, Spi};
use std::time::Instant;

fn main() {
    let spi = Spi::new(Bus::Spi0, SlaveSelect::Ss0, 1_000_000, Mode::Mode1).unwrap();

    let mut read_buff: [u8; 1] = [0x00];
    let write_buff:[u8; 1] = [0x01];

    let segments = [Segment::with_write(&write_buff), Segment::with_read(&mut read_buff)];

    spi.transfer_segments(&segments).unwrap();


    // Verify the spi_device.transactions executes all operations in the same transaction
    let mut spi_device = SimpleHalSpiDevice::new(spi);
    let mut operations = [Operation::Write(&write_buff), Operation::Read(&mut read_buff)];

    let now = Instant::now();
    spi_device.transaction(&mut operations).unwrap();
    let elapsed = now.elapsed();



    // Verify the delay is actually adding a delay
    let mut read_buff_with_delay: [u8; 1] = [0x00];

    let mut operations_with_delay = [Operation::Write(&write_buff), Operation::DelayNs(65_000), Operation::Read(&mut read_buff_with_delay), Operation::DelayNs(65_000)];

    let now_with_delay = Instant::now();
    spi_device.transaction(&mut operations_with_delay).unwrap();
    let elapsed_with_delay = now_with_delay.elapsed();

    println!("No Delay: {:?}", elapsed);
    println!("With Delay: {:?}", elapsed_with_delay);

    let difference = elapsed_with_delay.checked_sub(elapsed).unwrap();
    println!("Difference: {:?}", difference);
    
}

and the results

image
SDS00001
SDS00002

So now the operations seems to be all executed while the CS pin is held low, and the delay seems to be getting applied. The delay doesn't seem to be super precise, and it tends to add more delay that is specified, but I'm not sure how large of a concern that is.

@whatisbyandby whatisbyandby marked this pull request as ready for review August 17, 2024 16:01
@golemparts golemparts merged commit 74a3c67 into golemparts:master Aug 20, 2024
4 checks passed
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