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

drivers/bmp180: implementation cleanup #5679

Merged
merged 3 commits into from
Jul 27, 2016

Conversation

aabadie
Copy link
Contributor

@aabadie aabadie commented Jul 22, 2016

This PR makes a little bit of cleanup in the bmp180 sensor driver implementation.

Following #5589, I also made some tests and noticed that this line was not needed. As I only tried with a samr21-xpro board, it needs confirmation by someone else.

@PeterKietzmann PeterKietzmann self-assigned this Jul 25, 2016
@PeterKietzmann PeterKietzmann added this to the Release 2016.10 milestone Jul 25, 2016
@PeterKietzmann PeterKietzmann added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: drivers Area: Device drivers labels Jul 25, 2016
@aabadie
Copy link
Contributor Author

aabadie commented Jul 25, 2016

@PeterKietzmann, I applied the timing change requested by @keestux in #5589. Tested here, it works.

@PeterKietzmann
Copy link
Member

Thx. If @keestux confirms until tomorrow I will merge this PR as bugfix.

@PeterKietzmann PeterKietzmann added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jul 25, 2016
@keestux
Copy link
Contributor

keestux commented Jul 25, 2016

I'm a bit overloaded. I'll try to look at it asap

@keestux
Copy link
Contributor

keestux commented Jul 26, 2016

Yes, I confirm that it works.
I've tested my particular error case (ENABLE_DEBUG in auto_init.c and build with -O0). This works.
And the normal case (-Os, without ENABLE_DEBUG in auto_init.c) works too.

@PeterKietzmann
Copy link
Member

Thanks @aabadie and thanks @keestux for testing. Lets go with this PR and close #5589 instead. All right?

@PeterKietzmann
Copy link
Member

I got the order to ask our release manager @kYc0o whether it is ok to merge this PR as a bugfix or if the clean-up is too much during feature freeze. Sorry for not acting as I promised :/

@keestux
Copy link
Contributor

keestux commented Jul 26, 2016

@PeterKietzmann Yes, go ahead and close #5589

@miri64 miri64 modified the milestones: Release 2016.07, Release 2016.10 Jul 26, 2016
@miri64
Copy link
Member

miri64 commented Jul 26, 2016

If you say it's a bug-fix I would add it to the current release.

@kYc0o
Copy link
Contributor

kYc0o commented Jul 26, 2016

Of course if it's a bug-fix it should be in. Can anyone ACK this?

@PeterKietzmann
Copy link
Member

ACK and go

@PeterKietzmann PeterKietzmann merged commit 5c37377 into RIOT-OS:master Jul 27, 2016
@aabadie aabadie deleted the fix_type_bmp180 branch October 3, 2016 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants