Skip to content

Commit

Permalink
Merge #351
Browse files Browse the repository at this point in the history
351: SPI: split into device/bus, allows sharing buses with automatic CS r=therealprof a=Dirbaio

Because why not?

This separates the concepts of "SPI bus" and "SPI device". I got the idea of having traits for SPI devices from #299. It has an excellent breakdown of the issues of the status quo, so go read that first.

The difference from #299 is that proposes "repurposing" the existing traits to represent devices, while this PR proposes specifying the existing traits represent buses, and introduces a new trait for representing devices on top of that.

- HALs impl the bus traits, just like now. No extra boilerplate.
- Crates like `shared-bus` add the "bus mutexing" logic, to impl `SpiDevice` on top of `SpiBus` (+ `OutputPin` for CS). There's a wide variety of possible impls, all can satisfy the `SpiDevice` requirements:
   - Exclusive single-device, that just owns the entire bus
   - Single-thread, on top of `RefCell`
   - Multithreaded std, using std `Mutex`
   - Multithreaded embedded, using some `AtomicBool` taken flag.
   - Async embedded, using an async mutex (this'd require an async version of SpiDevice).
- Drivers take `T: SpiDevice` (or `SpiDeviceRead`, `SpiDeviceWrite`), then they start a `.transaction()`, then use it to read/write/transfer data. The transaction ends automatically when returning. Example:

```rust
fn read_register(&mut self, addr: u8) -> Result<u8, Error> {
    let mut buf = [0; 1];
    self.spi.transaction(|bus| {
        bus.write(&[addr])?;
        bus.read(&mut buf)?;
    })?;
    Ok(buf[0])
}
```

## No Transactional needed

Previous proposals boiled down to: for each method call (read/write/transfer), it automatically assets CS before, deasserts it after. This means two `write`s are really two transactions, which might not be what you want. To counter this, `Transactional` was added, so that you can do multiple operations in a single CS assert/deassert.

With this proposal, you have an explicit "Transaction" object, so you can easily do multiple reads/writes on it in a single transaction. This means `Transactional` is no longer needed.

The result is more flexible than `Transactional`, even. Take an imaginary device that responds to "read" operations with a status code, and the data only arrives on success. With this proposal you can easily read data then take conditional action.  With `Transactional` there's no way, best you can do is read the data unconditionally, then discard it on error.

It allows for smaller memory usage as well. With `Transactional` you had to allocate buffer space for all operations upfront, with this you can do them as you go.

```rust
fn read_data(&mut self, addr: u8, dest: &mut [u8]) -> Result<(), Error> {
    self.spi.transaction(|bus| {
        bus.write(&[addr])?;

        // Read status code, 0x00 indicates success
        let mut buf = [0; 1];
        bus.read(&mut buf)?;
        if buf[0] != 0x00 {
            return Err(Error::ErrorCode(buf[0]));
        }

        // If the operation is successful, actually read the data, still in the same transaction
        bus.read(dest)
    })
}
```


Co-authored-by: Dario Nieuwenhuis <dirbaio@dirbaio.net>
  • Loading branch information
bors[bot] and Dirbaio committed Feb 23, 2022
2 parents ddf4375 + b50b065 commit 6fda0f2
Show file tree
Hide file tree
Showing 8 changed files with 343 additions and 69 deletions.
4 changes: 2 additions & 2 deletions .github/bors.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ status = [
"ci-linux (stable, x86_64-unknown-linux-gnu)",
"ci-linux (stable, thumbv6m-none-eabi)",
"ci-linux (stable, thumbv7m-none-eabi)",
"ci-linux (1.46.0, x86_64-unknown-linux-gnu)",
"ci-linux (1.54.0, x86_64-unknown-linux-gnu)",
"ci-linux-test (stable)",
"ci-linux-test (1.46.0, x86_64-unknown-linux-gnu)",
"ci-linux-test (1.54.0, x86_64-unknown-linux-gnu)",
"fmt",
]
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ jobs:

include:
# Test MSRV
- rust: 1.46.0
- rust: 1.54.0
TARGET: x86_64-unknown-linux-gnu

# Test nightly but don't fail
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
rust: [stable]

include:
- rust: 1.46.0
- rust: 1.54.0
TARGET: x86_64-unknown-linux-gnu

# Test nightly but don't fail
Expand Down
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

## [Unreleased]

### Changed
- The Minimum Supported Rust Version (MSRV) is now 1.54.0
- `spi`: unify all traits into `SpiReadBus`, `SpiWriteBus` and `SpiBus` (read-write).
- `spi`: Add `SpiDevice` trait to represent a single device in a (possibly shared) bus, with managed chip-select (CS) pin.

## [v1.0.0-alpha.7] - 2022-02-09

*** This is (also) an alpha release with breaking changes (sorry) ***
Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[![crates.io](https://img.shields.io/crates/d/embedded-hal.svg)](https://crates.io/crates/embedded-hal)
[![crates.io](https://img.shields.io/crates/v/embedded-hal.svg)](https://crates.io/crates/embedded-hal)
[![Documentation](https://docs.rs/embedded-hal/badge.svg)](https://docs.rs/embedded-hal)
![Minimum Supported Rust Version](https://img.shields.io/badge/rustc-1.46+-blue.svg)
![Minimum Supported Rust Version](https://img.shields.io/badge/rustc-1.54+-blue.svg)

# `embedded-hal`

Expand Down Expand Up @@ -98,7 +98,7 @@ Because of this we only aim to support the latest `-alpha`.

## Minimum Supported Rust Version (MSRV)

This crate is guaranteed to compile on stable Rust 1.46 and up. It *might*
This crate is guaranteed to compile on stable Rust 1.54 and up. It *might*
compile with older versions but that may change in any new patch release.

## License
Expand Down
Loading

0 comments on commit 6fda0f2

Please sign in to comment.