I2C Slave Driver

[Rahix]

Currently, we only support TWI/I2c in master mode. In addition to that, let's add a driver to operate the TWI peripheral in slave mode.

Cc @ToddG

[Rahix]

Absolutely, let's do this! Ok, so I'll start brainstorming:

• Are there other platforms that we could look at and model after?
• What sections of the AVR spec should I look at closely?
• What's the overall game plan?

My sense of this is that the slave needs to:

• set an interrupt on the SDA pin being pulled low
• the interrupt wakes the CPU and sets a flag
• main thread loop checks flag, if set, invokes reader function with user defined callback
• callback is given an iterator over a stream

Conceptually, it looks simple enough when looking at Arduino code: Wire.onReceive (receiveEvent);

Let's go over this:

• Are there other platforms that we could look at and model after?

In Rust, I don't know of any other I2C slave drivers so we're on our own here. Also means we can experiment with the API a bit more :)

In C/C++ there's numerous implementations though tbh I wouldn't take their API as inspiration too much. I prefer looking at them strictly to understand low-level operations. One example is the Arduino Wire library.

• What sections of the AVR spec should I look at closely?

In the ATmega328P datasheet, that's chapter 22 in general and 22.7.3. and 22.7.4. specifically. The datasheet alone is often a bit hard to work with when you have no idea what an implementation might look like. That's why I suggest looking at an existing implementation for reference. But for the details, you'll find everything in the Datasheet usually.

---

• set an interrupt on the SDA pin being pulled low
• the interrupt wakes the CPU and sets a flag

The TWI peripheral already does much more AFAIK. It will only wake you up when the master requested the address that the slave was configured for. But ideally, the API should also work without any interrupts at all.

• main thread loop checks flag, if set, invokes reader function with user defined callback
• callback is given an iterator over a stream

Not sure how exactly the API should look but in general a callback should be avoided. I'd rather go with a poll function which will only yield a result when a read or write request is incoming. This way, downstream code has maximum flexibility in how their code interacts with the driver.

There's also still some issues with indirect calls/function pointers in the LLVM AVR backend so let's steer clear of that ;)

[Rahix]

• What's the overall game plan?

First of all, the implementations should go into the macro in avr-hal-generic/src/i2c.rs.

I'd say we define a second struct for the slave, similar to the existing one for the master:

$(#[$i2c_attr])*
pub struct $I2cSlave<CLOCK: $crate::clock::Clock, M> {
    p: $I2C,
    _clock: ::core::marker::PhantomData<CLOCK>,
    sda: $sdamod::$SDA<M>,
    scl: $sclmod::$SCL<M>,
}

Similar to the other one it will also need the two ::new() and ::new_with_external_pullup() constructors. Though here, the constructors need an additional argument which is the slave address. I think (but this needs to be checked) that we do not need the speed argument.

pub fn new(
    p: $I2C,
    sda: $sdamod::$SDA<$crate::port::mode::Input<$crate::port::mode::PullUp>>,
    scl: $sclmod::$SCL<$crate::port::mode::Input<$crate::port::mode::PullUp>>,
    slave_address: u8,
) -> $I2c<CLOCK, $crate::i2c::I2cPullUp> {
    // ...
}

For the rest of the API we'll first have to look deeper into the operation of the TWI peripheral.

[Rahix]

As a second C implementation, there's also this Application Note 2565.

[ToddG]

I spent a few days studying rust macros, and I have some specific questions:

1. https://github.com/Rahix/avr-hal/blob/0c6cf1675c2724354f1adeaeee69992acd371e80/avr-hal-generic/src/i2c.rs#L181

Q: What is this form doing? It looks like an attribute macro wrapped in $(...)* Q: Is this form applied to just the immediately following struct or is it modifying the scope? I'm guessing the struct... Q: Where is i2c_attr:meta defined? I don't see it in the calling code here Q: BTW - what tooling are you using? I'm using CLion with the Arduino and Rust plugins. But I'm not able to find all the definitions to things...and without them, it's like flying blind trying to figure out what comes from where, etc.

2. How do I generate the expanded macro forms? I documented my trials here but the TLDR is these did not work:

rustc -Z unstable-options --pretty expanded chips/atmega328p-hal/src/lib.rs RUSTFLAGS="-Z unstable-options --pretty expanded" cargo build | tee build-expanded.txt

3. Struct Name : I2cSlave gives build errors

        // this works
        $(#[$i2c_attr])*
        pub struct Slave<CLOCK: $crate::clock::Clock, M> {
            p: $I2C,
            _clock: ::core::marker::PhantomData<CLOCK>,
            sda: $sdamod::$SDA<M>,
            scl: $sclmod::$SCL<M>,
        }

        // build errors with the $I2cSlave
        // $(#[$i2c_attr])*
        // pub struct $I2cSlave<CLOCK: $crate::clock::Clock, M> {
        //     p: $I2C,
        //     _clock: ::core::marker::PhantomData<CLOCK>,
        //     sda: $sdamod::$SDA<M>,
        //     scl: $sclmod::$SCL<M>,
        // }

[Rahix]

I spent a few days studying rust macros, and I have some specific questions:

The rust macro system is arcane magic, don't worry if it makes your head hurt.

Q: What is this form doing? It looks like an attribute macro wrapped in $(...)*

Basically, $(#[$i2c_attr:meta])* consumes zero or more attribute items from the call-site. It is important to note that with $(...)*, it's also okay if no matching tokens are found (i.e. zero attributes).

Q: Is this form applied to just the immediately following struct or is it modifying the scope? I'm guessing the struct...

https://github.com/Rahix/avr-hal/blob/0c6cf1675c2724354f1adeaeee69992acd371e80/avr-hal-generic/src/i2c.rs#L205-L206

here all the consumed items (if any) are applied to the I2c struct. Think, what plain rust code like

#[...]
struct Foo { ...}

would be doing.

Q: Where is i2c_attr:meta defined? I don't see it in the calling code here

Heh ;) Actually the doc-comment:

https://github.com/Rahix/avr-hal/blob/0c6cf1675c2724354f1adeaeee69992acd371e80/chips/atmega328p-hal/src/lib.rs#L30

will be expanded to an attribute like this:

#[doc("I2C based on ATmega328P's TWI peripheral")]

and that's what the macro sees :)

Q: BTW - what tooling are you using? I'm using CLion with the Arduino and Rust plugins. But I'm not able to find all the definitions to things...and without them, it's like flying blind trying to figure out what comes from where, etc.

VIM (nvim actually) + rust-analyzer. But for a lot of things, especially when macros are at play, this won't help much either I'm afraid. Usually going by the use ... statements and grep -r/git grep-ing for the name works well enough for me when hunting for a definition of a type.

---

How do I generate the expanded macro forms? I documented my trials here but the TLDR is these did not work:

cargo-expand is amazing for this. But be careful, the expansion is not necessarily what the compiler sees, so take the resulting code with a grain of salt.

Struct Name : $I2cSlave gives build errors

Sure, that's because the macro token $I2cSlave needs to be defined somewhere. I see two solutions:

• Extend the macro syntax such that call-sites needs to define the name as well.
• Use paste to concatenate $I2c and Slave, similar to what is done for the serial driver. You need to wrap the relevant part of the expansion in a crate::paste::paste!{} and then you can do [< $I2c Slave >].

[Rahix]

Q: BTW - what tooling are you using? I'm using CLion with the Arduino and Rust plugins. But I'm not able to find all the definitions to things...and without them, it's like flying blind trying to figure out what comes from where, etc.

One more thing: You can generate crate documentation locally with

cargo doc --open

which will also document all of your crates dependencies. This is a great way to find your way around unfamiliar rust code as it will also contain all of the types which were generated by macros.

[ToddG]

Rahix, thank you for all this great feedback and suggestions! I'll spend the next day or two chewing through this.

[Rahix]

Look what I just found! There is previous work, the lpc8xx-hal has an I2C slave driver: https://github.com/lpc-rs/lpc8xx-hal/blob/master/src/i2c/slave.rs

This might be a good starting ground in terms of API design.

[ToddG]

Cool! I'll check it out today.

[kellpossible]

I spent a few days studying rust macros, and I have some specific questions:

The rust macro system is arcane magic, don't worry if it makes your head hurt.

One of the reasons why I like/prefer the procedural macros, they can be easier to reason about when things get complicated.

[cromefire]

Having this would be great, as it's an easy way to pass information from a pi to an arduino and back. #93 sounds pretty much ready already, so having this merged would pretty cool.

[agausmann]

Investigating this myself, I just wanted to add a note from a comment I saw here:

I'd say we define a second struct for the slave, similar to the existing one for the master

Separating the master and slave drivers would limit some features of the TWI peripheral. I don't know how universal this is, but on some devices (e.g. ATmega32u4) the TWI peripheral can be both a master and a slave at the same time. It supports multi-master arbitration, and when arbitration is lost, the losing master can switch to slave mode and continue performing address-matching.

(Ref: ATmega16u4/32u4 datasheet, sec. 20.8)

If there are two separate structs, I2CMaster and I2CSlave or whatever, and both require ownership of the TWI peripheral and I/O pins, then master and slave configurations become mutually exclusive and this technique is not possible.

[jgerrish]

I just wanted to say thanks for the extensive back-and-forth in this PR, it's been helpful to other people.

I've been testing adding SPI secondary (slave) support to avr-hal, and this thread looks like a good example for integrating it back into the codebase.

I created a proof-of-concept by:

1. Adding a new field to the Settings structure
2. Replacing the Spi structure, instead of creating a SpiSecondary or SpiSlave
3. Naively swapping Input/Output pins (and a few other changes)

And it works. There appear to be no conflicts with the rest of the avr-hal crate, and I'm able to connect with another device acting as primary.

If I add the code back into avr-hal-generic/src/spi.rs, it's going to approximately double the size of that file. I feel there should be a better way with more understanding of Rust generics, traits and types, but I'm not there yet.

Anyways, even if I don't open a PR, this is just a comment to let you know the comments on here were helpful and that the code works.

Thanks

[Palladinium]

@jgerrish Would you be willing to share your code for the proof-of-concept if you still have it around? One of my current projects could really use an I2C slave driver implementation, and having another implementation to cross-reference (no matter how polished) would be very helpful.

[kirillfx]

Hi guys, I'm new to rust and was looking for i2c slave implementation here, but managed to implement it myself by reading Datasheet and other people's C code. It's incomplete, but works for simple read/write modes. Leaving here links for those who'll be looking for it like me. I have 2 different boards for slave and master: pro mini, pro nano, so had to had to use 2 repos:

i2c master i2c slave