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

Add alternate address to 'begfin(TwoWire)' #68

Merged
merged 1 commit into from
Nov 15, 2019
Merged

Add alternate address to 'begfin(TwoWire)' #68

merged 1 commit into from
Nov 15, 2019

Conversation

hhk7734
Copy link
Contributor

@hhk7734 hhk7734 commented Nov 11, 2019

  • Describe the scope of your change--i.e. what the change does and what parts
    of the code were modified.
    This will help us understand any risks of integrating
    the code.

When calling begin(void), if failed to init() with i2caddr=BME280_ADDRESS, it tries again with i2caddr=BME280_ADDRESS_ALTERNATE.
But it does not try again when calling begin(TwoWire). So I added that.

  • Please run any tests or examples that can exercise your modified code. We
    strive to not break users of the code and running tests/examples helps with this
    process.

examples/bme280test/bme280test.ino

line 46
-  status = bme.begin();
+  status = bme.begin(&Wire);  

@caternuson
Copy link
Contributor

Thanks for spotting that. My two cents on this - the overrides should be arranged differently to avoid the code duplication. Something like this:

bool Adafruit_BME280::begin(void) {
  return begin(BME280_ADDRESS, &Wire);
}

bool Adafruit_BME280::begin(TwoWire *theWire) {
  return begin(BME280_ADDRESS, theWire); 
}

bool Adafruit_BME280::begin(uint8_t addr) {
  return begin(addr, &Wire);
}

bool Adafruit_BME280::begin(uint8_t addr, TwoWire *theWire) {
  bool status = false;
  _i2caddr = addr;
  _wire = theWire;
  status = init();
  if (!status) {
    _i2caddr = BME280_ADDRESS_ALTERNATE;
    status = init();
  }
  return status;
}

suggestion: Carter Nelson <caternuson@gmail.com>
@hhk7734
Copy link
Contributor Author

hhk7734 commented Nov 11, 2019

@caternuson Your suggestion is so good. I corrected it.

@caternuson
Copy link
Contributor

@hhk7734 Thanks. This looks good. Did you test this on hardware?

@caternuson caternuson self-requested a review November 13, 2019 18:52
@hhk7734
Copy link
Contributor Author

hhk7734 commented Nov 15, 2019

@caternuson Yes, I did.

examples/bme280test/bme280test.ino

line 46
-  status = bme.begin();
+  status = bme.begin(0x01, &Wire);  
line 46
-  status = bme.begin();
+  status = bme.begin(&Wire3);  
line 46
-  status = bme.begin();
+  status = bme.begin(0x01);  

@caternuson
Copy link
Contributor

Cool. Thanks.

@caternuson caternuson merged commit ddec5c1 into adafruit:master Nov 15, 2019
@caternuson
Copy link
Contributor

caternuson commented Nov 15, 2019

It will be in the 1.1.0 release when it becomes available:
https://github.com/adafruit/Adafruit_BME280_Library/releases/tag/1.1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants