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: during init retry getting ID because it may fail #5589

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions drivers/bmp180/bmp180.c
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,12 @@ int bmp180_init(bmp180_t *dev, i2c_t i2c, uint8_t mode)
/* Acquire exclusive access */
i2c_acquire(dev->i2c_dev);

/* Check sensor ID */
char checkid;
i2c_write_byte(dev->i2c_dev, BMP180_ADDR, BMP180_REGISTER_ID);
i2c_read_reg(dev->i2c_dev, BMP180_ADDR, BMP180_REGISTER_ID, &checkid);
/* Check sensor ID (retry a few times, it may fail at first) */
char checkid = 0;
for (size_t ix = 0; checkid == 0 && ix < 3; ++ix) {
i2c_write_byte(dev->i2c_dev, BMP180_ADDR, BMP180_REGISTER_ID);
Copy link
Contributor

Choose a reason for hiding this comment

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

during my tests, I also noticed that this line is not needed one can directly access the ID register with the next line.

Copy link
Contributor

Choose a reason for hiding this comment

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

I created #5679. Can you give it a try ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For me it does not make a difference. It still fails when building with -O0.

But your changes seems to be correct. With the default -Os it works for me.

BTW. Why didn't you remove

i2c_write_byte(dev->i2c_dev, BMP180_ADDR, (char)BMP180_CALIBRATION_AC1);

It seems useless too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, forget about my last BTW (about leaving out the write_byte). It turns out it is necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again.
The above mentioned i2c_write_byte still does not make sense. Why is it needed? I don't think it is. But if I leave it out I'm getting all zeroes for the calibration.

To solve that I need a delay between the BMP180_REGISTER_ID and the BMP180_CALIBRATION_AC1.

Here is a question to you. Can you try the following?

diff --git a/drivers/bmp180/bmp180.c b/drivers/bmp180/bmp180.c
index e772c3a..ad771cc 100644
--- a/drivers/bmp180/bmp180.c
+++ b/drivers/bmp180/bmp180.c
@@ -73,10 +73,10 @@ int bmp180_init(bmp180_t *dev, i2c_t i2c, uint8_t mode)
         i2c_release(dev->i2c_dev);
         return -1;
     }
+    xtimer_usleep(3);

     char buffer[22] = {0};
     /* Read calibration values, using contiguous register addresses */
-    i2c_write_byte(dev->i2c_dev, BMP180_ADDR, (char)BMP180_CALIBRATION_AC1);
     if (i2c_read_regs(dev->i2c_dev, BMP180_ADDR, BMP180_CALIBRATION_AC1, buffer, 22) < 0) {
         DEBUG("[Error] Cannot read calibration registers.\n");
         i2c_release(dev->i2c_dev);

Copy link
Contributor

Choose a reason for hiding this comment

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

@keestux, I tried in #5679 and it indeed works better. Let me know if it's the same for you. We could then merge #5679 as a bug fix.

i2c_read_reg(dev->i2c_dev, BMP180_ADDR, BMP180_REGISTER_ID, &checkid);
}
if (checkid != 0x55) {
DEBUG("[Error] Wrong device ID\n");
return -1;
Expand Down