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 changeable i2c address to BME280 usermod #3966

Merged
merged 3 commits into from May 14, 2024

Conversation

LordMike
Copy link

@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
Collaborator

Please do not force push changes.

Copy link
Collaborator

@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.

usermods/BME280_v2/usermod_bme280.h Outdated Show resolved Hide resolved
@LordMike
Copy link
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
Collaborator

According to the library documentation that may be sufficient.

@LordMike
Copy link
Author

Seems to work as well.

@LordMike LordMike requested a review from blazoncek May 14, 2024 16:12
@blazoncek blazoncek merged commit 5f41de8 into Aircoookie:0_15 May 14, 2024
18 checks passed
@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.

None yet

2 participants