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: si70xx: result bugfixes #5240

Merged
merged 1 commit into from
Apr 13, 2016

Conversation

basilfx
Copy link
Member

@basilfx basilfx commented Apr 3, 2016

While testing #5231, I found two bugs that I should have seen earlier.

  • The calculations in si70xx_get_relative_humidity could wrap around, resulting in humidity 100% where it should be ~0%. ffe3fe3 fixes this.
  • si70xx_get_revision and si70xx_get_serial start with tainted buffers. In case of I2C failures, I noticed that it still passed si70xx_test because of exactly the right values in the buffers (possibly uncleared RAM). By clearing them on beforehand, they should return an invalid revision number or serial number. 08e7e3c fixes this.

I also took the liberty of improving the test application.

@OlegHahm OlegHahm added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: drivers Area: Device drivers labels Apr 3, 2016
@OlegHahm OlegHahm added this to the Release 2016.04 milestone Apr 3, 2016
TEST_I2C_ADDR ?= 128
TEST_PIN_EN ?= 57
TEST_I2C_ADDR ?= 0x80
TEST_PIN_EN ?= GPIO_PIN\(PD,9\)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would make sense to choose something more generic here, as GPIO_PIN(0, 0), as PD is not defined for every cpu...

@haukepetersen
Copy link
Contributor

changes look good, ACK once the comment above is addressed.

@basilfx basilfx force-pushed the feature/si70xx_improvements branch from a0ca413 to ed62d6c Compare April 11, 2016 10:58
@basilfx
Copy link
Member Author

basilfx commented Apr 11, 2016

Addressed the issue and rebased.

@haukepetersen haukepetersen added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 11, 2016
@haukepetersen
Copy link
Contributor

so let's see if Murdock it fine with this...

@basilfx
Copy link
Member Author

basilfx commented Apr 11, 2016

Murdock is happy. Should I squash it into two commits (driver + test) or just one?

@basilfx
Copy link
Member Author

basilfx commented Apr 13, 2016

@haukepetersen If #5231 get's merged first, I'll rebase onto that one once more.

@haukepetersen
Copy link
Contributor

#5231 is merged, please rebase and let's merge this as soon as the CI is happy (again).

@basilfx basilfx force-pushed the feature/si70xx_improvements branch from ed62d6c to cf5e4c8 Compare April 13, 2016 11:28
@basilfx
Copy link
Member Author

basilfx commented Apr 13, 2016

Murdock is happy. I already squashed it into one commit.

@OlegHahm OlegHahm merged commit 5269017 into RIOT-OS:master Apr 13, 2016
@basilfx basilfx deleted the feature/si70xx_improvements branch April 13, 2016 13:08
@miri64
Copy link
Member

miri64 commented Apr 19, 2016

backport needed. (sorry is already in release branch)

@miri64 miri64 added Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch and removed Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch labels Apr 19, 2016
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 Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants