i2c Deref

[burrbull]

Same as #182

[burrbull]

Rebased

[jounathaen]

Is there a reason, this is part of this PR?

[burrbull]

Rebased.
cc @TheZoq2

[TheZoq2]

Sorry for taking such a long time to review this.

I'm not sure how I feel about the change. To me it kind of makes the code harder to read, but that might be because I'm fairly used to reading macro code.

In addition, the related change to the spi made the documentation harder to read, Pins is now completely useless for example https://docs.rs/stm32f1xx-hal/0.5.3/stm32f1xx_hal/spi/trait.Pins.html Though I suppose that will not happen here since the pins struct remains the same.

In the SPI case, we got the advantage of being able to do partial mapping of the SPI peripherals, but I don't think we are able to do that here.

I guess my point is that I don't think the changes in this PR make much sense, but I'd love to be convinced otherwise

[burrbull]

I can’t imagine how it can be more difficult to read than macros.

And this PR does not do anything with pin remap.
It just replaces hal macro with Deref.

[therealprof]

I have to agree with @burrbull here. Not only is that code easier to read, it also receives full benefit of Rust infrastructure including proper formatting, rust-analyzer support and usable compiler error messages when working on the implementation.

I also agree with @TheZoq2 that the documentation needs some work and is less useful after the macro removal.

[TheZoq2]

I can’t imagine how it can be more difficult to read than macros.

Maybe it's because of how used I am to the macros. I think the thing I don't like about it is how many layers of function calls there are in the initialisation. Something about it makes it hard to read.

Not only is that code easier to read, it also receives full benefit of Rust infrastructure including proper formatting, rust-analyzer support and usable compiler error messages when working on the implementation.

That's a very good point. Coupled with both of you disagreeing with me about the readability, I think we should merge this.

@burrbull Would you mind making the same change I did in #232 which was one of the main things I disliked about this change?

[burrbull]

Rebased

[TheZoq2]

Looks like you added the changelog entry to the 0.6.0 version instead of unreleased

[burrbull]

Fixed