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

feat: copies repeated start condition from stm32g0xx_hal crate #138

Closed emilhoeiriis closed 3 years ago

hannobraun commented 3 years ago

Hey @emilhoeiriis, thank you for this PR, and sorry for the long wait!

Can you tell me a bit more about this PR, why it is necessary and how you have tested it? You seem to replace some code with identical code, but with worse formatting. Plus, this doesn't compile. That doesn't inspire much confidence :smiley:, so I'll need some additional information before I can consider merging this PR.

emilhoeiriis commented 3 years ago

I saw that the repeated start condition was not being set when probing the bus, so I just copied code from the stm32g0xx-hal and it behaves as expected. If you can tell me the compile error I'll find the time to address the issue.

On Sat, 20 Feb 2021 at 10:38, Hanno Braun @.***> wrote:

Hey @emilhoeiriis https://github.com/emilhoeiriis, thank you for this PR, and sorry for the long wait!

Can you tell me a bit more about this PR, why it is necessary and how you have tested it? You seem to replace some code with identical code, but with worse formatting. Plus, this doesn't compile. That doesn't inspire much confidence 😃, so I'll need some additional information before I can consider merging this PR.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/stm32-rs/stm32l0xx-hal/pull/138#issuecomment-782595810, or unsubscribe https://github.com/notifications/unsubscribe-auth/AO4JKVODE557FBK4J7L65U3S757IFANCNFSM4UVAOIGQ .

hannobraun commented 3 years ago

The CI build is failing. This is linked near the bottom of this page, above the comment text field. Here's a direct link: https://travis-ci.com/github/stm32-rs/stm32l0xx-hal/builds/208711004

All builds are failing. I haven't checked all of them, but here's one example of an error: https://travis-ci.com/github/stm32-rs/stm32l0xx-hal/jobs/459954101#L229

I've also noticed that this PR removes error checking code, as far as I can tell. I think the best approach is starting over: Identify the changes that are actually required to fix the issue you're seeing, and apply those in our codebase. Blindly copying another implementations seems like it would cause just as many problems as it might solve.

And in addition, this PR would need to be rebased on the latest master branch. There have been changes to the CI setup since this PR was opened, and I'd like to see this run with the latest configuration before I can consider merging it.

hannobraun commented 3 years ago

Closing due to inactivity.

@emilhoeiriis Feel free to update and re-open this pull request, to address the outstanding concerns.