search

stm32-rs / stm32l0xx-hal

A hardware abstraction layer (HAL) for the STM32L0 series microcontrollers written in Rust
BSD Zero Clause License
96 stars 60 forks source link

Made improvements to polling i2c driver #102

Closed apgoetz closed 4 years ago

apgoetz commented 4 years ago

Hi, I discovered a couple of issues with the blocking I2C driver while playing around with an 32L0538DISCOVERY board. This board has a custom I2C slave peripheral to measure the current of the main STM32 controller, and while writing a driver for it I stumbled upon some bugs in the HAL driver.

Could someone take a look at these changes and see if they can make it into the library?

hannobraun commented 4 years ago

Please reformat your change with cargo fmt.

This isn't a policy in this crate, and there's lots of existing code that doesn't follow this convention. I'm open to changing that, but someone would have to put in the legwork. Specifically, I'd like to see two things:

  1. Enforce formatting in CI.
  2. Adapt the existing code, so cargo fmt doesn't completely mess up readability (see https://github.com/nrf-rs/nrf-hal/commit/88b6f8793da4818cdba99f04163195250554fb14, for example).
almusil commented 4 years ago

Please reformat your change with cargo fmt.

This isn't a policy in this crate, and there's lots of existing code that doesn't follow this convention. I'm open to changing that, but someone would have to put in the legwork. Specifically, I'd like to see two things:

1. Enforce formatting in CI.

2. Adapt the existing code, so `cargo fmt` doesn't completely mess up readability (see [nrf-rs/nrf-hal@88b6f87](https://github.com/nrf-rs/nrf-hal/commit/88b6f8793da4818cdba99f04163195250554fb14), for example).

I see. Enforcing it in CI and reformatting the current code is doable. The second part is bit of work but I can create a PR for that if you want. I would like to see the fmt rules enforced.

hannobraun commented 4 years ago

The second part is bit of work but I can create a PR for that if you want.

To be clear, I'm not a fan of rustfmt and would prefer if things stayed as they ar. I can see that I'm in the minority though, and don't intend to stand in the way of progress

So I'm not asking you (or anyone) to do this. I'm just saying, if it's going to be done, that's what it takes to get me to agree :-)