API design for 1.0

[tcr]

Wanted to move discussion over from #23 about blockers for 1.0.

Some ideas to continue discussing:

• How would traits modeling I2C and GPIO access be defined? How would downstream consumers of the library expect to use these, and what overhead does a generic interface impose?
• How can we ensure at compile-time that the "mode" of a port is checked?
• Should we implement protocol reading on a separate thread? How would async events like pin interrupts be designed?

[tcr]

Moving over two comments from the other thread...

---

@Dr-Emann Thanks for taking the time to draft this. Is my understanding correct that you're proposing GpioPort as a trait, which different implementations depending on the underlying port mode, be it all GPIO, I2C, SPI, or UART?

---

@kevinmehall So I'm in favor of keeping the initial API simple and synchronous, but I'm not sure how to do that once we introduce pin edge/level triggers. At the point where we're writing gpio_pin_event_listen_forever(), we're basically writing a shitty event loop (and which can't have I2C/SPI communication in parallel). At that point we'd just graduate to a full event loop, message tx/rx on its own thread, and using futures/channels to trigger callbacks for reads and pin events. Is this a fair assessment given our SAMD21 protocol?

[kevinmehall]

In a purely synchronous API, I think you could get away with a wait_for_pin method for most cases.
The main use of pin change interrupts is for a device to signal that it should be polled. I originally planned a CMD_PIN_WAIT opcode for that purpose, which would wait on the coprocessor for a pin event, and then proceed with the command sequence. That never got implemented, in favor of hacking something together for compatibility with the T1 JS API.

But, this also brings up a good argument for using futures -- the command protocol supports batching commands together, so that CMD_PIN_WAIT opcode can be directly followed by e.g. an I2C read, without a round-trip to the SoC. The very first Rust API that Ken wrote made the commands and batches explicit, but it's probably lower-level than most users want to deal with. Futures would allow you to queue multiple commands without waiting for the response.

[Dr-Emann]

I've spent some time thinking, and came up with a few possible APIs

Single Owner, passed parameters

Api:

// in crate tessel, so called as tessel::ports()
ports() -> Option<Vec<Port>> // Will only return Some() once. Optionally, on Port drop, allow to be called again
// If we know we'll only have two ports, we could return Option<(Port, Port)>

struct Port {
    socket: UnixSocket, // Note: No mutex
}

Port::pin(&self, index: u8) -> GpioPin<Port> // Panics for invalid indexes

Port::enable_i2c(self, frequency: u32) -> I2CPort

// On drop, disable I2C
struct I2CPort {
    port: Port, // Ownership
    frequency: u32,
}

I2CPort::disable_i2c(self) -> Port
I2CPort::pin(&self, index: u8) -> GpioPin<I2CPort> // Panics for invalid indexes
I2CPort::i2c(&self) -> I2C

struct I2C;

I2C::send(&self, port: &mut I2CPort, data: &[u8]) // etc

struct GpioPin<P> {
    index: u8,
    _marker: PhantomData<P>,
}

GpioPin<P>::index(&self) -> u8
// By restricting with P, you can only use it with the type of port it was created with. (e.g. Port GpioPin's aren't useable with I2CPort)

GpioPin<P>::read(&self, port: &mut P) -> bool // etc

Possible Usage:

// in mod my_sensor
const FREQUENCY: u32 = ...;
struct Sensor {
    port: I2CPort,
    i2c: I2c,
    other_pin: GpioPin<I2CPort>
}

impl Sensor {
    // This sensor owns the port
    fn new(port: Port) -> Sensor {
        let port = port.enable_i2c(FREQUENCY);
        let i2c = port.i2c();
        let other_pin = port.pin(5);
        Sensor {
            port: port,
            i2c: i2c,
            other_pin: other_pin
        }
    }

    fn sense(&mut self) -> u32 {
        self.other_pin.read(&mut self.port);
        unimplemented!();
    }
}

// in mod other_sensor
// This sensor has shared ownership of the port, and it doesn't care what type of port,
// as long as you can pass in a GPIO pin from that type of port
// it should be possible to not own the port, if the port is passed in for all calls (e.g. sense(&self, &mut P))
struct SingePinSensor<P> {
    port: Arc<Mutex<P>>,
    pin: GpioPin<P>,
}

impl<P> SinglePinSensor<P> {
    fn new(port: Arc<Mutex<P>>, pin: GpioPin<P>) -> Self {
        SinglePinSensor { port: port, pin: pin }
    }

   fn sense(&self) -> u32 {
       let port = self.port.lock().unwrap();
       self.pin.read(&mut port);
       unimplemented!();
    }
}

Pros:

• Performant: By ensuring there's a single owner of a port, there's no need for a Mutex around the socket. It's still possible to share across threads, though, by wrapping a port in a mutex yourself, but you don't have to pay for it if you don't use it.
• Compile time safety: Because enable_i2c takes self by value, and GpioPin takes a port type parameter, it's known at compile time that if getting a GpioPin is successful, it cannot be misused.

Cons:

• Complexity: Much more complex to use than an API which uses locking behind the scenes.
• Quite dissimilar from the javascript API
• Some things are impossible to guarantee at compile time: It's always possible to ask for an out of bounds index for a pin.
• Nothing is threadsafe by default: It's up to the user to use a mutex if needed.
• I'm not sure if it's possible to have an async api. I think it still is, but I haven't thought about it enough.

I have a few other API ideas, but it's getting late, and this comment is getting long.

[tcr]

Mutexing should definitely be removed for GPIO pins, it can be handled by keeping the port in scope.

I like the practice of specializing a Tessel port as an I2CPort—it also scales to a hypothetical future hardware design that allows I2C + SPI (I2cSpiPort) or all comms at once (at that point... TesselPort).

Async is an issue for leveraging the full GPIO bank. I can't think of a good API where I can wait for pin interrupts on several pins simultaneously unless I'm using an event loop (or a poor emulation of one). Essentially how much value do we gain by keeping the SAMD21 socket on the main thread rather than another thread + channels or futures? (We gain flexibility by not using sysfs for I/O, but lose flexibility by each pin not owning its own file descriptor...)

(Got to work on USB maintenance for a bit, so no commits by me today.)

[Dr-Emann]

Time for another possible API.

Static References, with locking

Api:

lazy_static! { TESSEL_INSTANCE:Tessel= { /* make tessel */ }; }
tessel() -> &'static Tessel // returns &TESSEL_INSTANCE

Tessel::ports(&'static self) -> &'static [port]
struct Port {
    socket: Mutex<UnixSocket>, // Note: No Arc
    mode: Mutex<PortMode>,
    pins: Vec<AtomicUsize>, // Keeps track of how many references there are to each pin.
    // If pin is in use by, e.g. I2C, set to (!0)
}

enum PortMode {
    Normal,
    I2C (Arc<I2C>),
    Spi(Arc<...>),
    Uart(Arc<...>),,
]

Port::pin(&'static self, index: u8) -> GpioPin // Panics for invalid indexes

// On creation (or clone), increment RefCount (or panic if == !0 (means pin in use))
struct GpioPin {
    port: &'static Port,
    index: u8,
    ref_count: &'static AtomicUsize
}

GpioPin::read(&self) -> bool // etc

Port::i2c(&'static self, frequency: u32) -> Arc<I2C> // Panic if needed pins have a use count > 0
// Panic if port.mode is not I2c with the correct frequency already, or Normal
// if current mode is None, set needed pin use counts to (!0), then set current mode

Port::disable_i2c(&'static self) // Panic if not I2C or normal
// Panic if PortMode's arc is not the only I2C reference left
// It might be better to automatically disable I2C when all `I2C` structs are dropped

struct I2C {
    port: &'static Port,
    frequency: u32,
}

I2C::send(&self, data: &[u8]) // etc

Possible Usage:

// in mod my_sensor
const FREQUENCY: u32 = ...;
struct Sensor {
    i2c: I2C,
    other_pin: GpioPin,
}

impl Sensor {
    fn new(port: &'static Port) -> Sensor {
        let i2c = port.i2c(FREQUENCY);
        let other_pin = port.pin(5);
        Sensor {
            i2c: i2c,
            other_pin: other_pin
        }
    }

    fn sense(&self) -> u32 {
        self.other_pin.read();
        unimplemented!();
    }
}

// in mod other_sensor
struct SingePinSensor {
    pin: GpioPin,
}

impl SinglePinSensor {
    fn new(pin: GpioPin) -> Self {
        SinglePinSensor { pin: pin }
    }

   fn sense(&self) -> u32 {
       let port = self.port.lock().unwrap();
       self.pin.read();
       unimplemented!();
    }
}

Pros:

• Easy to use: Ports being &'static makes it easy to share, and I2C/GpioPins have references to their own ports: i2c.send(data) looks better than i2c.send(&mut port, data)
• Pre-baked thread safety: it's safe to call .tessel().ports() as many times as you want, from any number of threads, or share a single reference across multiple threads, it's all the same.
• Closer to the javascript API
• Probably easier to move to an event-loop (still need more time to think about how/if the other API would work with it.)

Cons:

• Performance: Mandatory mutex, even if everything is only referenced from one thread.
• Some things are impossible to guarantee at compile time: It's always possible to ask for an out of bounds index for a pin.
• It's not possible to tell at compile time if a port is in I2C mode. If a port is in another mode, the code will panic at runtime.

[tcr]

I spent some time thinking over this, and I want to propose the following:

1. We make the underlying Tessel Bridge API public. We introduce command batching as it existed in Ken's original API for performance.
2. We build a blocking, ownership-based API on top of this.
3. We leave room for a futures-based, multithreaded extension built on the underlying API.

Part 1. is easy and mostly implemented, the new change is to make it public. For 2., @Dr-Emann I think the above API proposals are pretty good. I wanted to propose another revision on yours which adds more compile-time guarantees:

https://play.rust-lang.org/?gist=6d0968659dafde02cb139aa05f8f30af&version=stable&backtrace=0

We probably would still require a mandatory Mutex on the Port socket, but we can guarantee if a port is in I2C mode and we can't re-borrow a borrowed Pin (because we own the gpio bank object). The ergonomics aren't JavaScript-easy but they are pretty straightforward to use.

(I don't like the "tuple hack" to get varargs but Iron uses it so there's precedent, and it's only for the Tessel API so it's not imposing on compatible code. We can add a pins_vec() method in parallel, or pins() -> Vec<> and pins_select() -> tuple)

[tcr]

Notes from #34:

• It's not perfect.
• I wanted to Mutex a sensor in a multithreaded server demo, so I was required to declare port socket as PortSocket { socket: Arc::new(Mutex::new(PortSocket::new(path))) }. This feels redundant since we still have to mutex a sensor or port in order to have multithreaded access, so probably not the right choice.
• Web server demo is in the test/ path, for reference.