[RSDK-10267] BME280 idf-component sensor #2
Conversation
src/lib.rs
Outdated
| /// Global handle to I2C bus | ||
| static mut I2C_BUS: i2c_bus_handle_t = std::ptr::null_mut(); | ||
| /// Global handle to BME280 device | ||
| static mut BME280: bme280_handle_t = std::ptr::null_mut(); |
There was a problem hiding this comment.
these cause compilation warnings about static mut references (to be deprecated). I'm not sure what I should use instead. Looking online there's references to a const_mut compiler feature, but it isn't stable just yet.
src/lib.rs
Outdated
| esp!(bme280_read_temperature( | ||
| BME280, | ||
| &mut temperature as &mut f32 | ||
| ))?; | ||
| log::debug!("bme280 - reading pressure..."); | ||
| esp!(bme280_read_pressure(BME280, &mut pressure as &mut f32))?; | ||
| log::debug!("bme280 - reading humidity..."); | ||
| esp!(bme280_read_humidity(BME280, &mut humidity as &mut f32))?; |
There was a problem hiding this comment.
an error from any of the three readings will cause get_readings to return an error.
mattjperez
requested review from
npmenard and
stevebriskin
and removed request for
npmenard and
stevebriskin
stevebriskin
left a comment
stevebriskin
left a comment
There was a problem hiding this comment.
add a readme and license file. approving assuming that is done.
all comments are minor, address as you wish.
| const I2C_MODE_MASTER: u32 = 0x1; | ||
|
|
||
| pub fn register_models(registry: &mut ComponentRegistry) -> Result<(), RegistryError> { | ||
| registry.register_sensor("bme280", &Bme280::from_config) |
There was a problem hiding this comment.
as discussed, see if you can make this a triplet. not a blocker for merging.
There was a problem hiding this comment.
this is not possible without an adjustment to registration logic in micro-RDK, see here: https://viam.atlassian.net/browse/RSDK-7822
src/lib.rs
Outdated
| log::info!("initializing BME280: {:?}", BME280); | ||
| BME280 = bme280_create(I2C_BUS, BME280_I2C_ADDRESS_DEFAULT.try_into().unwrap()); | ||
| log::info!("initialized BME280 to : {:?}", BME280); | ||
| esp!(bme280_default_init(BME280))?; |
There was a problem hiding this comment.
nit: you have before/after log statements for everything but this init step, for consistency log here too
src/lib.rs
Outdated
| log::info!("initializing I2C_BUS: {:?}", I2C_BUS); | ||
| I2C_BUS = i2c_bus_create(bus_no, &config); | ||
| log::info!("initialized I2C_BUS to : {:?}", I2C_BUS); | ||
| log::info!("initializing BME280: {:?}", BME280); |
There was a problem hiding this comment.
nit: create and init are different things for the bme280. be clear with logging....this log should probably read "creating"
|
Also...what is the consequence of having the micrordk dependency in Cargo.toml be on the RC? Can that be a "0.4.0+"? |
|
Just realized I didn't add the corresponding drop logic to destroy the bme280 and i2c_bus. |
@npmenard @gvaradarajan tagging here since I can only add one reviewer