Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add STM32L0x2 support to I2C API #10

Merged
merged 7 commits into from Jun 29, 2019
Merged

Add STM32L0x2 support to I2C API #10

merged 7 commits into from Jun 29, 2019

Conversation

hannobraun
Copy link
Contributor

@hannobraun hannobraun commented Jun 28, 2019

As the title says. More info in the commits.

I have an example program I tested this with. I figured it doesn't make sense to add a million example using whatever hardware people have lying around, so I kept it separate. Here it is:
https://github.com/braun-embedded/stm32l0xx-hal/blob/2250f59525407dd6564565736691804a80b83898/examples/i2c.rs

cc @lthiery

hannobraun added 7 commits Jun 28, 2019
I realize that rustfmt, which I suspect formatted this code, has high
esteem in the Rust community, but we should be clear about one thing:
rustfmt doesn't provide readable formatting, it provides consistent
formatting.

Usually these two are the same, but svd2rust-generated APIs are a clear
case where consistency and readability are opposed. I believe we should
always choose readability over consistency, hence this change.
While attempting to add support for all I2C instance of the STM32L0x2, I
discovered that the macro made it impossible to do that, as that would
have resulted in multiple `impl` blocks for `I2c` with conflicting
method definitions.

I decided to solve that problem in the following way:
- Move the `impl` block for `I2c` out of the macro.
- Abstract over the differences between I2C instances with a new trait,
  `i2c::Instance`.

I believe this approach is superior to the traditional "everything in
the macro" approach, for the following reasons:
- These macros can make the code less readable, as you often have to
  jump between macro definition and macro arguments.
- The code in the macro is deeply indented and the macro can sometimes
  confuse the syntax highlighting, exacerbating the readability
  problems.
- Any compiler error in the macro shows up multiple times, for each
  implementation that the macro generates.
- Multiple nearly identical methods are generated. I haven't checked
  whether this affects the size of the compiled code negatively, but I
  wouldn't be surprised if it did.

For these reasons, I believe that keeping as much code out of the macro
as possible is the better approach.
This seems more robust, as it doesn't depend on any leftover
configuration from previous writes, and thus can't break if those
previous writes change.
Both `send_byte` and `recv_byte` have their own loops at the beginning,
waiting until the respective operations can be started.
I have an example program running on STM32L0x2 that doesn't work without
this change.
This bit needs to be set to start a read transfer. I have an example
program running on STM32L0x2 that won't work without this change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants