rust-embedded/wg

Define minimal Traits for common embedded peripherals

Closed this issue · 35 comments

jamesmunns commented

Peripherals are often used in a similar style (or a few styles), with an uncountable number of details of difference between implementations.

In order to have Arduino-level usability and portability, it would be useful to define a minimal set of traits that can be used to write libraries against. When more constraints are necessary, these traits can be composed together.

For example, a baseline trait may look like this:

trait Serial {
  fn read(&self) -> Option<u8>;
  fn write(&self, u8) -> Result<(), ()>;
}

When necessary to constrain, the following pattern can be used.

struct FifoSerial {
// some fields omitted
}

impl Serial for FifoSerial {
  fn read(&mut self) -> Option<u8> {
    self.rd_fifo.pop()
  }

  fn write(&mut self, data: u8) -> Result<(), ()> {
    self.wr_fifo.push(data)
  }
}

// Marker trait used as a tag
impl Nonblocking for FifoSerial {};

fn forward<I: Serial, O: Serial+Nonblocking>(in: I, out: O) -> Result<(), ()> {
  // Take from a high priority port to a low priority, memory backed port
  if Some(b) = in.read() {
    try!(out.write(b));
  }
  Ok(())
}

I would like to define the behavior of at least the following peripherals:

  • GPIO (maybe pins, maybe ports, maybe both)
  • SPI
  • I2C
  • Serial
  • PWM (analogWrite)

Additionally, Arduino Language Reference and Arduino Libraries may be good references for a minimal amount of functionality to be useful.

thejpster commented

I have come across one situation so far, writing a Linux application in Rust, where it was necessary for my Serial object to be used in a read thread and a write thread simultaneously. To do this, I needed to split it into two objects.

I wonder if we could expand this Trait so that one half of the comms (say, read) could be broken off in to a separate object for such occasions. And maybe merged back in.

jamesmunns commented

For UART, that definitely makes sense. For other kinds of serial comms (i2c, spi) it doesn't make sense to break them up due to how they are implemented.

thejpster commented

I think I agree.

The question for SPI I think is whether you have an in buffer and a mut out buffer, or a single buffer it but reads and writes from.

I've previously argued that a single Linux style char device interface should be the goal, but the more I think about it the more I think the limited ability of the platform to work around problems with the API (with buffering, etc) perhaps means we need a more specific API for each interface.

jamesmunns commented

@thejpster I struggled with this on teensy3: https://github.com/jamesmunns/teensy3-rs/blob/master/teensy3/src/spi.rs#L121

I went with one in, one out with in-place replacement, since SPI will always be a 1:1 match.

To be honest, I would probably prefer something like this, but it is not currently possible AFAIK:

fn write(&mut self, out: [u8; N]) -> [u8; N]; // N == N
japaric commented

👍 frome as long as

In order to have Arduino-level usability and portability

This is clearly marked, in the crate name, as an Arduino compatible interface / compatibility layer.

trait Serial

This probably wants an associated type error.

self.wr_fifo.push(data)

Is wr_fifo a "software buffer" (i.e. a chunk of RAM) or the "hardware fifo" that some chips have? If the former when does the data actually get pushed into the wire?

impl Nonblocking for FifoSerial {};

If the same trait (Serial) can be blocking or nonblocking based on the implementation, then we should make it a guideline to clearly reflect this in the API documentation. As in:

impl Serial for MySerial {
    // Heads up! This actually BLOCKS
    fn write(&mut self, byte: u8) -> Result<(), !> { ... }
}

Also, having a marker trait to denote async feels "weak". The library author can forget to implement it. And feels easy to miss when skimming over the API reference (but this a rustdoc problem wrt to not clearly showing what marker traits a type implements)

Finally, having the same trait mean both blocking and nonblocking feels useful but, IMO, it also feels like an stretch: It feels like the trait doesn't rely specify what the behavior is, instead the implementation fully governs the actual behavior (blocking / async). I guess my point is that reading code like this:

use Serial;

my_serial.write(b'H');

Raises questions like: "Does this block or not?" Which can't be immediately answered. I guess you could infer it from the local context but will ultimately have to refer to the docs of MySerial to be sure.

But, hey, we are still experiment! Let's see what lies down this road 😄.

jamesmunns commented

@japaric

This is clearly marked, in the crate name, as an Arduino compatible interface / compatibility layer.

My point here wasn't to sell an "arduino compatable layer", but rather, define some lowest common denominator that is useful, so that people who dont care how things work can have things "just work", which is a hallmark of the Arduino. This would be useful for the first stage of a bringup, and allow writers of a library (for example, a radio modem), to consume these traits.

So I would expect that:

  • A library writer writes a library, e.g. TiModem, that is generic over <T: Serial>
  • The person responsible for writing the "Board Support Package" e.g. Teensy3 would be able to write one or a few drivers, perhaps BlockingSerial and BufferedSerial which both implement trait Serial.
  • The end-user decides how to compose these two items together. For example:
/// Developed by the library - e.g. in crate "ti-modem"
struct TiModem<S: Serial> {
  state: SomeMetaData,
  port: S
}

/// Developed by the board maintainer - e.g. in crate "teensy3"
struct HwBufferBackedSerial {
  // ...
}

impl Serial for HwBufferBackedSerial {
  // In here, we use the hardware native buffers available to implement the serial trait
}

/// In the application code developed by the user
fn main() {
  // setup the modem
  let modem = TiModem::new(HwBufferBackedSerial::new(some, serial, config), some, modem, configs);
  // ...
}

In my mind, there will be two tiers of traits that it would be good to standardize on:

  • Lowest common denominator traits, e.g. Serial, I2C, etc. These have the bare minimum detail associated with them. Basically, these are a good first step for "does this work"? Then if behavior needs to be refined, you move on to:
  • Higher level traits, could be marker traits, could be traits with additional exposed behavior. These are useful when it is important HOW the underlying trait is implemented, e.g. NonBlocking, HwBuffered, even things like Sync and Send for things that need to be interrupt/thread safe.

I might be arguing a bad point here, especially from an embedded perspective, but I would say that to keep libraries general and useful for many platforms, it is important NOT to care how things are implemented, but rather focus on the APIs.

The pattern tends to be:

  • Library writers specify the high level details of how a library works
  • Platform developers expose a few sane defaults that generally can be plugged into existing libraries
  • End users plug the right options to meet the library, or, if a bespoke solution is necessary, they can use traits to guide the API of what they need to develop, and are then in charge of keeping up with the details. Hopefully then, these solutions can at least be merged upstream as another option for the platform maintainers, if they are not closed source, etc.

Is wr_fifo a "software buffer"?

I would suggest, should the library "care"?

Also, having a marker trait to denote async feels "weak"

I agree. I am open to other suggestions. This was just a thought in passing. Perhaps this is something that could be addressed in the Copper book.

It feels like the trait doesn't rely specify what the behavior is, instead the implementation fully governs the actual behavior

Thats exactly what I am going for.

  • Library: "What will this do"?
  • BSP: "How could you do it"?
  • End-User: "What is the right way to do what I want it to do"?

Also as a side note, by setting a standard API to develop against, we set ourselves up much better for a hopeful future where Silicon makers like Freescale et. al will also release a rust BSP, rather than it be all community driven. Even as a community BSP developer, I often struggle with "okay, what do I write in rust? just port everything the hardware supports? That will take forever!". It would bring a lot of value to say "If you support these 15 traits, you have at least initial support for 85% of embedded focused crates out there". From there, we can move for refinement (e.g. nonblocking implementations, better hardware-accelerated peripheral usage, etc.)

japaric commented

My point here wasn't to sell an "arduino compatable layer", but rather, define
some lowest common denominator that is useful

That seems fine to me and I pretty much agree with your points about being able
to write code in a generic way.

I'm just afraid that we (the community) will settle for an Arduino-like API
(read: direct translation of the Arduino API) without exploring patterns
that are more Rustic and more familiar to the Rust community as a whole, like
futures, and that that could get us stuck in a non-optimal state (can't know
where the optimal point is without exploring) due to inertia. And I'm afraid
that may happen because (a) a direct translation is the easiest thing to do and
(b) it's also the easiest way to get "market share", as many people are already
familiar with the Arduino API.

TL;DR I only want to advise against rushing things and jumping right into the
easiest thing to do.

Is wr_fifo a "software buffer"?

I would suggest, should the library "care"?

Not if they are writing generic code but I care because I can't see how an
implementation like that would fulfill the contract. :-)

From there, we can move for refinement (e.g. nonblocking implementations,
better hardware-accelerated peripheral usage, etc.)

(emphasis mine)

This is the part I can't see how it would work. The traits in your first sketch
look incompatible with patterns like futures and coroutines. I'd like to see
some well thought-out "future proofing" work before settling on any API. And I'd
like to see, as well, non-trivial applications implemented in more than one API
before committing to one.

jamesmunns commented

direct translation of the Arduino API

advise against rushing things

I agree, I don't want to copy it wholesale. I'm still dipping my toes into futures, and think it could make sense from the embedded point of view, I guess I was just pushing to get something consistent first, that could grow.

non-trivial applications

I will do my best to rebuild some existing things I have in C and C++ in the embedded space into at least a partially fleshed out set of examples. It might be good to outline what we want here, so that multiple people can outline an apples-to-apples comparison.

I have started here: https://github.com/jamesmunns/rfcs/tree/add-serial-trait/mock-drivers, which consumes https://github.com/jamesmunns/rfcs/tree/add-serial-trait/trait-apis - though I admit it is pretty shallow. As I mentioned in the pull request discussion, I will break this out into a crate, or set of crates, and we can work from there.

jamesmunns commented

Broken out into a organization/repo here: https://github.com/knurling-rs

I will publish the crates once crates.io is feeling a little better.

@posborne and @japaric I would welcome criticism, comments, and pull requests, especially with how to "specialize" these traits.

posborne commented

For reference, here's the core traits that I have been using for a few of the libraries I have written. These are decidedly not designed for MCUs/no_std, but they might be useful for comparison as they mimic the Linux API closely (which we will definitely want to have be supported in most cases).

I agree that blocking/non-blocking is definitely going to be a real problem that will need to be addressed somehow. Most of the Linux APIs are blocking (which is much easier to work with in the simple case) -- It might be possible to work with non-blocking APIs by judiciously making using of fork(). I think that supporting something like futures could be a great way to raise the level of abstraction without incurring significant runtime cost.

thejpster commented

So, as part of novemb.rs, and after some discussion on IRC, I've put some serial traits at https://crates.io/crates/embedded-serial. They differ from other examples in that they break out read/write seperately, and try to handle blocking, non-blocking, and blocking-with-timeout scenarios. The only trait that's missing is non-blocking-but-callback-when-it-would-no-longer-block and I don't have a good feel for how that would work (especially as the callback is likely to be in interrupt context).

I also had a useful discussion with my fellow sprinters about whether UARTs (and indeed peripherals in general) should be represented by stack objects to which references are passed around, or if they should be represented by static singletons in the driver code, accessed via some locking mechanism to ensure exclusivity. There were opposing views, and I'm not sure we came to a conclusion.

japaric commented

Here are two sketchs of blocking I2C traits:

Disclaimer: I haven't tried to implement them so they likely contain errors

The goal of the design is to prevent, at the compile time, users from trying to "connect" (send START) to more than one device if they haven't finished their current "session" (haven't sent STOP).

kevinmehall commented

It's supposed to be legal to send a repeated start, even to a different slave address. This can be useful in a multi-master situation to avoid releasing the bus to another master.

japaric commented

@kevinmehall That could be implemented as a restart method (that takes a slave address as argument) on the "session type based" design then the connected state would mean "currently holding ownership of the bus" rather than "currently connected to a specific device".

japaric commented

Sketch of futures based IO traits (Read and Write). Sadly most of the methods in them can't be defined in the traits without ATC (Associated Type Constructors). However, note that it's possible to "concretely" implement the full API today (i.e. using inherent impls) but that way we can't write generic code.

istankovic commented

Sketch of a session types based approach that requires only one trait.

japaric commented

Check japaric/f3#52 for a futures based async API for (basic) Timers, Serial, I2C and SPI that has been implemented for a STM32F3 micro (that PR also contains async API to read three different motion sensors). I'd like to know if the API over there, or a blocking version of it, can be implemented as it's for different micros. Or want changes would be required to make it more universal.

Kixunil commented

This is interesting topic. My opinion is that we should define low-level traits as well as high-level traits and wrappers that implement high-level traits for stuff that implements low-level traits.

Commonly known example of such design is Read and BufRead from std::io. Read is low-level trait which you can implement and you get the possibility to create BufReader if you need buffered reading. However, if for whatever reason your low-level type already uses buffering, you can impl BufRead directly and get the benefit of higher performance.

I did something similar in my WIP PN532 and mifare crates. I sliced whole thing into several layers so if you impl BusRead and BusWrite traits for your types (i2c, spi, uart) you get ability to communicate with PN532. Since there are two possible waiting strategies (busy waiting and waiting for IRQ), middle-level WaitRead trait is implemented. Of course I need to solve bunch of other problems: the most important is support for Futures and bare-metal.

So I can imagine something like this (just general idea):

trait InterruptHandler {
    fn handle_interrupt();
}

trait HardwareSerial {
    /// Returns false if device is not ready.
    fn send_byte(&mut self, u8) -> bool;
    /// Returns None if device is not ready.
    fn recv_byte(&mut self) -> Option<u8>;
    /// None means disable Interrupt
    fn set_read_complete_isr(&mut self, isr: Option<&InterruptHandler>);
    /// None means disable Interrupt
    fn set_write_complete_isr(&mut self, isr: Option<&InterruptHandler>);
}

struct SerialRingBuffer<S: HardwareSerial> {
    buffer: [u8; 64],
    serial: S
}

trait BufferedSerial {
    type ReadError;
    type WriteError;

    read(&mut self, buf: &mut [u8]) -> Result<usize, Self::ReadError>;
    write(&mut self, buf: &[u8]) -> Result<usize, Self::WriteError>;
}

#[cfg(feature = "with_std")]
impl<T: BufferedSerial> BufRead for T {
    // ...
}

That's just general idea. I'd certainly separate reading and writing. Async needs to be expressed too. (Maybe do something like I did with WaitReadTimeout - declare trait ReadNonblock and trait WriteNonblock with fns of same name and special Error type enum NonblockingError<E> { Other(E), WouldBlock, })

japaric commented

For comparison sake, here's an implementation (not by me) of the closure based I2C design I posted before.

Kixunil commented

I see closures mostly as a way to prevent leaking values (e.g. to solve scoped thread fiasco) or avoid some lifetime issues (for example query_map() method in rusqlite crate). Does any of these apply to I2C? Or is there some other reason to use closures? I tend to avoid closures if I can, because API without them provides somewhat bigger flexibility.

japaric commented

My opinion is that we should define low-level traits as well as high-level traits and wrappers that implement high-level traits for stuff that implements low-level traits.

Yeah, I think this is the right approach: having wrapper to provide more functionality.

Async needs to be expressed too.

In my mind, the API should be nonblocking first / only. It's pretty hard to do anything non-trivial just with a blocking API. OTOH, you can easily block with a nonblocking API by e.g. busy waiting.

Do note there's a new async model in the ecosystem: the tasks and resources introduced in this blog post. The traits should support that model as well. Also check this comment where I describe an idea to make a nonblocking API compatible with futures without having the API directly depend on the futures crate.

japaric commented

re-posting my u.r-l.o comment here:

An update on the HAL front.

I have now published (on GitHub) the cortex-m-hal crate which contains a
Hardware Abstraction Layer (HAL), in the form of a bunch of traits, for the
following peripherals / functionality:

  • Input Capture
  • Pulse Width Modulation
  • Quadrature Encoder Interface
  • Serial Peripheral Interface (SPI)
  • Serial (UART) interface
  • Timer / timeouts

Along with a reference implementation in the form of the blue-pill crate,
which I believe has the most complete API build on top of a device crate
generated via svd2rust. That crate also contains a bunch of examples.

The key points of this HAL are:

It's fully non-blocking but it's not tied to a particular asynchronous
model. You can easily adapt it to operate in blocking mode, or to work with
futures, or to work with an async / await model. All this magic is done via
the nb crate so check out that crate documentation too. That nb crate even
has an await! implementation, which doesn't work right now because generators
have not landed in the language, but that macro lets you do cooperative
multitasking in a cleaner way than if you would have used the futures
crate.

It's minimal to make it easy to implement and keep it as zero cost as
possible. The main idea is have enough of an API to erase device specific
details like registers but let you build higher level abstractions
with minimal overhead. Want a blocking read with timeout semantics? Just compose
the Serial and Timer abstractions using a generic function.

The ultimate goal of this HAL is code reuse. I know people have different
opinions about how an embedded program should be structured (e.g. cooperative
tasks vs event-driven tasks vs threads) and will want to use different
higher level abstractions tailored for their needs. What I want to see is that
those abstractions get built on top of this HAL instead of seeing everyone
rewrite very similar register manipulation code to build those abstractions.

I'd love to get some feedback. I have opened a bunch of issues in the
cortex-m-hal issue tracker where you can leave comments about each
particular trait. The HAL also needs more testing across devices to make sure
it's generic enough to be implemented for devices from different vendors so let
me know if can or can't implement this HAL for some device.

  • 🎉2
whitequark commented

Why is it called cortex-m-hal? I don't see anything obviously specific to Cortex-M...

japaric commented

Why is it called cortex-m-hal? I don't see anything obviously specific to Cortex-M...

Because the name hal is already taken on crates.io :-(

Because the only existing implementation of this HAL targets a Cortex-M microcontroller and because IDK if I may add some Cortex-M specific stuff to it in the future (seems rather unlikely). It'd be OK with renaming it if someone confirms that this HAL makes sense for MSP430, AVR and/or embedded Linux.

whitequark commented

I worked with AVR and embedded Linux and sure. It would be extremely surprising, to say the least, if this interface would be more expressive (and so unimplementable) than anything on any embedded platform.

One issue though is that it can be not expressive enough. For example, it doesn't allow for 9-bit communication over serial.

thejpster commented
Kixunil commented

I agree that such thing should not be limited to specific architecture. But it seems nice anyway.

Edit: I miss 9-bit serial too.

whitequark commented

I'd also prefer the traits being split in to separate crates. If I write,
say, a reusable command line harness implementation, then I'm only
interested in the UART, and keeping that trait isolated helps with
versioning add reducing churn.

This seems like a really bad idea. Instead of a coherent, internally consistent set of abstractions, we will now have many disjoint crates and the associated versioning nightmare. I see what's currently called cortex-m-hal becoming a crate akin to collections--not covering everything there is, but giving a good foundation, and becoming an example to follow.

We already have embedded-serial,
which is used by at least one other person.

Having two users as opposed to having zero users is not a strong motivation for reliance on any crate.

thejpster commented

Instead of a coherent, internally consistent set of abstractions, we will now have many disjoint crates and the associated versioning nightmare.

My experience is limited, but I'd have said that managing versions for a series of disjointed crates was one of the things that Cargo does pretty well right now. To take one example, we already take advantage of this by having cortex-m, alloc-cortex-m and cortex-m-semihosting instead of one larger crate. At the moment, I don't see the UART trait having anything much in common with, say, the SPI trait - they do fundamentally different things - so I don't really see the argument for bundling.

My concern is that in a semantically versioned crate, the major version will need to bump every time the API is changed for any of the interfaces within it. If I come along with my library and see the HAL crate has moved two major versions, I don't actually know if that introduces an incompatibility or not, as it might have changed the API for an interface I don't even use. That might be OK if it's just my application using a monster chip-crate with all the peripherals together, but what If I try and build an application where the chip crate uses the HAL at some version X.0.0, while some third-party console library I need uses the HAL at version Y.0.0 and some other third-party I2C accelerometer driver I need uses the HAL at version Z.0.0, but it turns out they're all compatible because the breaking changes were actually in, say, the SPI trait - can Cargo deal with that? What version do I use in my application?

Maybe I'm worrying too much about API instability.

Having two users as opposed to having zero users is not a strong motivation for reliance on any crate.

Point taken.

adamgreig commented

👍 on keeping it as a single crate with a nice collection of abstract traits, and 👍 on reconsidering the name as well - perhaps embedded-hal or embedded-peripherals?

@thejpster I would kind of hope this could stabilise relatively quickly and then not require much churn -- short of inventing entirely new types of things, once we've covered UART, SPI, I2C, ADC, DAC, TIM, GPIO, and maybe CAN (and USB for the sadists), I expect the vast majority of use cases will be covered. Adding new peripherals doesn't need to be a breaking change either, so as long as well keep the things covered reasonably simple I hope you wouldn't run into too many versioning problems.

Do we have any crates right now that would use traits like this? I'm imagining device drivers that want to be given some kind of SPI or UART they can use. smoltcp's Device trait comes to mind (albeit for slightly higher level ethernet devices). It would be worth seeing what people are already using and checking anything new at least meets those requirements.

jamesmunns commented

Hey, I still have the GitHub space that i mentioned above: https://github.com/knurling-rs/knurling @adamgreig you might be interested in the scenario I described above, if we have traits that cover peripherals, we could write drivers for common components like sensors, etc.

I'm happy to contribute the knurling space if anyone finds the pun (knurling: making bare metal easier to handle) as funny as I do. It might be useful to group things similarly to how tokio does.

adamgreig commented

It looks like knurling-traits covers the same sort of stuff as cortex-m-hal?

Yep, I'd like to have traits covering peripherals so that you could imagine writing crates that nicely implement interacting with specific devices. Though I don't think the lack of such available traits stops you (since you can just supply your own trait and require the crate user to implement it); it's just it would be nicer if we could all share traits where possible.

(it's a good pun!)

japaric commented

Renamed cortex-m-hal to embedded-hal. If someone wants to bikeshed that further please an issue on that repo issue tracker.

  • 👍3
Kixunil commented

I like embedded-hal!

jamesmunns commented

I think we call all solidly call this closed, as embedded-hal now exists :)