I2C transaction_iter should take an iterator of mutable references

[yodaldevoid]

I was recently porting an I2C controller device to 1.0.0-alpha.7 which forced me to implement transaction and transaction_iter for the first time. I initially tried to implement transaction as follows:

fn transaction<'a>(
    &mut self,
    address: u8,
    operations: &mut [Operation<'a>],
) -> Result<(), Self::Error> {
    self.transaction_iter(address, operations)
}

This, of course, did not work as &mut [Operation<'a>] produces an iterator with Item = &mut Operation<'a> rather than the Item = Operation<'a> that transaction_iter requires.

fn transaction_iter<'a, O>(&mut self, address: u8, operations: O) -> Result<(), Self::Error>
where
    O: IntoIterator<Item = Operation<'a>>,

---

In my opinion, transaction_iter's signature should be this instead:

fn transaction_iter<'a, 'b: 'a, O>(&mut self, address: u8, operations: O) -> Result<(), Self::Error>
where
    O: IntoIterator<Item = &'a Operation<'b>>,

This signature mainly allows the implementation of transaction that I wrote above. By changing this signature almost all functions of I2C can be implemented as calls to transaction_iter even on systems without alloc support, with write_iter and write_iter_read being the exceptions. This would allow de-duplication of logic, which is rarely a bad thing. Additionally, I see no reason that transaction_iter needs an iterator over owned Operations in the first place.

Thoughts?

[Dirbaio]

If the iterator yields owned Operations, it can generate them on the fly and return them. If it yields references, it has to allocate all the operations upfront and keep them alive throughout the entire transaction_iter call, as all the yielded references must live for 'a.

However, even with the current "yields owned Operations", the same problem applies to the buffers, they must still be all allocated upfront. Given this I'm not sure transaction_iter has any benefits at all over transaction in the first place... Maybe we should remove it completely.

Also, the whole problem disappears if we change i2c to use a transaction closure like in #351...

[yodaldevoid]

You make a good point about needing a way of yielding generated operations. You also make a good point that transaction_iter doesn't really meet this need at the moment.

I'll look at putting together a PR adding something like a what is seen in #351.

[yodaldevoid]

As can be expected, while I was far too busy to actually work on this other's have worked at a solution. I think something like #392 is the right path forward.

[tgross35]

It would be nice to have a default implementation for some of these trait items. If write and read could have default implementations calling write_iter and read_iter, it would nicely reduce the overhead of what somebody needs to write.

Also, what is the reasoning behind keeping the transaction methods within the main I2c trait, rather than something like I2cTransactional? It seems to me like the way it currently is for v2 makes it tougher to write a minimal i2c implementation (without using unimplemented!).

[eldruin]

Thank you everybody for the discussion.
This method has been removed in #440. Closing.