Embedded-hal: Bit banging SPI implementation

[maikelwever]

For my MAX7219 crate I've translated the shiftOut function from the Arduino platform to rust, in order to aid with bit-banged serial transfer.

As proposed at https://github.com/rust-lang-nursery/embedded-wg/issues/39#issuecomment-367524427, it might be an idea to move this function to the embedded-hal crate.

I am willing to send a PR to move this to the embedded-hal crate, and extend the implementation to be on par with the Arduino one (https://github.com/arduino/Arduino/blob/master/hardware/arduino/avr/cores/arduino/wiring_shift.c#L40).

But before I make the PR, I'd like to check if there are any objections/ideas/comments regarding this, so here's what I was thinking:

• Be on par with the Arduino implementation (MSB+LSB support).
• Renaming the function to shift_out instead of shiftOut.
• Moving it to the spi.rs file here in embedded-hal, so it's importable like this: use embedded_hal::spi::shift_out.
• Implementing the shiftIn function is for another time and PR.
• Making clear in the documentation comments that this is a bit-banged implementation.

For reference, my current implementation lives here: https://github.com/maikelwever/max7219/blob/master/src/lib.rs#L127

[austinglaser]

I love the idea of having bit-banged implementations of common serial protocols -- but does it make sense in this this crate? My understanding is that embedded-hal's purpose is to define an abstraction layer, not to implement anything directly. I'd definitely favor a separate bit-bang-hal crate that implements the protocol, and possible others (like I2C) in the future.

I also think the API bears careful thinking on. Instead of a standalone function, it should probably be a type implementing the embedded_hal::spi::FullDuplex trait. Making it a function forces driver developers to choose between supporting a bit-banged or hardware-based implementation -- which should really be a choice made when putting together an application.

Perhaps something like bit_bang_hal::spi::Spi?

[japaric]

I'd definitely favor a separate bit-bang-hal crate that implements the protocol, and possible others (like I2C) in the future.

:+1: though I would probably put embedded-hal in the name. Alternatively, it could be one of the keywords of the crate.

it should probably be a type implementing the embedded_hal::spi::FullDuplex trait.

I also think it should be a (generic) type implementing the SPI trait(s). Then it can be used with SPI based driver crates.

[Kixunil]

I've proposed Wave generation trait. Could this be implemented on top of that trait or should it be different?

Obviously, receiving end will need something else, but it doesn't conflict with transmitting.

[sajattack]

https://crates.io/crates/bitbang-hal

[eldruin]

As stated above, there is a working bitbanging SPI implementation in the bitbang-hal.
I think @sajattack will probably welcome any improvements.
@maikelwever can this issue be closed?

[maikelwever]

It's been a while for me, but I think this issue can be closed indeed.