Skip to content

Add changeable i2c address to BME280 usermod#3966

Merged
blazoncek merged 3 commits intowled:0_15from
LordMike:feature/bme280_changeable_i2c
May 14, 2024
Merged

Add changeable i2c address to BME280 usermod#3966
blazoncek merged 3 commits intowled:0_15from
LordMike:feature/bme280_changeable_i2c

Conversation

@LordMike
Copy link
Copy Markdown
Contributor

@LordMike LordMike commented May 10, 2024

The BME280 usermod assumes an address of 0x76 which is incorrect for the BMP280 chip, which defaults to 0x77. This change puts the i2c address into the config and defaults to 0x76.

Notes:

  • This builds on Fix missing conversions of bme280 values #3965. I'll rebase when that's merged.
  • As of right now, WLED must be rebooted for the address change to take effect. I'm not much of a C++ developer to know the ins and outs of reinitializing the BME280I2C instance .. do I stop it? delete it? .. If that was fixed, rebooting wasn't necessary.
  • The config shows up as a text field with integers. It's my understanding that I can't manipulate with how the configs are shown, f.ex. to make a dropdown. Should it instead be a textual field which is then parsed in the code as a hex string -- or is decimals fine? ..

New config view. The I2CAddress is new - here set to 0x77.

image

@LordMike LordMike marked this pull request as draft May 10, 2024 21:20
@LordMike LordMike changed the title Draft: Add changeable i2c to BME280 usermod Add changeable i2c to BME280 usermod May 10, 2024
@LordMike LordMike force-pushed the feature/bme280_changeable_i2c branch from f597d0f to d4ba0ba Compare May 10, 2024 21:23
@LordMike LordMike changed the title Add changeable i2c to BME280 usermod Add changeable i2c address to BME280 usermod May 10, 2024
@LordMike LordMike force-pushed the feature/bme280_changeable_i2c branch from d4ba0ba to 32f875d Compare May 11, 2024 10:22
@LordMike LordMike marked this pull request as ready for review May 11, 2024 10:23
@LordMike LordMike force-pushed the feature/bme280_changeable_i2c branch from 32f875d to 43d024f Compare May 11, 2024 10:24
@blazoncek
Copy link
Copy Markdown
Contributor

Please do not force push changes.

Copy link
Copy Markdown
Contributor

@blazoncek blazoncek left a comment

Choose a reason for hiding this comment

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

I'm not happy with how sensor is initialised.

Comment thread usermods/BME280_v2/usermod_bme280.h Outdated
@LordMike
Copy link
Copy Markdown
Contributor Author

@blazoncek i just remembered. Am I not right in thinking the settings instance is not needed as a separate field?

To update it, I have to use setSettings() which means a copy is made in the bme instance. So having a copy of the settings in this class is just double the data.

So I can remove the field with settings and create the settings every time I need it instead. Yes? :)

@blazoncek
Copy link
Copy Markdown
Contributor

According to the library documentation that may be sufficient.

@LordMike
Copy link
Copy Markdown
Contributor Author

Seems to work as well.

@LordMike LordMike requested a review from blazoncek May 14, 2024 16:12
@blazoncek blazoncek merged commit 5f41de8 into wled:0_15 May 14, 2024
@LordMike LordMike deleted the feature/bme280_changeable_i2c branch May 16, 2024 16:59
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