eldruin / lsm303agr-rs

Platform agnostic Rust driver for the LSM303AGR ultra-compact high-performance eCompass module: ultra-low-power 3D accelerometer and 3D magnetometer
Apache License 2.0
18 stars 14 forks source link

Sensors report 0 everywhere since version 0.3 #29

Closed chrysn closed 7 months ago

chrysn commented 7 months ago

Something changed between 0.2.2 and 0.3 that makes the sensor on a microbit-v2 report all-zeros on every measurement.

(As I had 0.2.2 last time I updated and used this to develop my embedded-hal 1.0 implementation, this made me doubt my sanity -- but indeed already 0.3 has the trouble, which is embedded-hal 0.2 based.)

I have yet to bisect between 0.3 and 0.2.2 (maybe with some more linearity than regular bisection to avoid editing back and forth through API changes), and will keep this updated.

eldruin commented 7 months ago

Interesting. Which sensor? The accelerometer or the magnetometer? What about the temperature sensor? There have been pretty substantial changes between 0.2.2 and 0.3. Maybe something in this commit or this commit has gone wrong?

chrysn commented 7 months ago

Thanks, I'll reexamine those two. I'm seeing trouble with both sensors (and haven't looked at the temperature sensor because that was unavailable in the last good version)

By the current stage of bisection I've come to think that I'm probably just doing the setup wrong (because that's something I have to vary in the course of bisection, causing variation in addition to the history being tested).

If nothing else, the outcome of this should be at least an observation of how the setup can go wrong, and unless it adds too much state, possibly a new error state indicating that what you are trying to read is not set up in a way that is being read does not produce any sensible data (for I'm definitely not testing in a zero-g environment).

chrysn commented 7 months ago

So this is my test code that changed its behavior:

        let mut device = Lsm303agr::new_with_i2c(i2c::I2CDevice::new(i2cdev));
        device.init().unwrap();
        // device.set_accel_mode(&mut riot_wrappers::ztimer::Clock::usec(), AccelMode::Normal).unwrap();
        // device.set_accel_odr(&mut riot_wrappers::ztimer::Clock::usec(), Hz50).unwrap();
        println!("{:?}", device.acceleration());

It changed from printing something to printing Ok(Acceleration { x: 0, y: 0, z: 0, mode: Normal, scale: G2 }) in ff1d5885168e67197c5d6420801ec160b1060ae1.

Should this be working, or is this a usage error?

[edit: Commented out the set_ lines because they don't make the difference crossing the referenced commit]

chrysn commented 7 months ago

As I can also induce the breakage in the old version by making the write_read behave like embedded-hal specifies, it is now my working hypothesis that the sensor requires a stop condition before reading. The hardware I'm working with (nrf52 wrapped in RIOT OS) handles stop conditions pretty automatically, so any subtle code change (possibly even alignment of the TEXT section) can make the difference between whether or not the read after the write happens early enough that a STOP condition is set, or is done without a STOP. Verifying...

chrysn commented 7 months ago

Alternative hypothesis, now that I have debug wires and an oscilloscope on it: The hardware won't write from flash, only from RAM. As the writes are silently null (instead of asking the register name), the null register is selected, and then all answers are null. That gets me up to 0.3, and is clearly an implementation error on the OS side.

Keeping this open and updated until I've ruled out that the remaining trouble may not be coming from this crate (but at this point it's becoming unlikely).

chrysn commented 7 months ago

This was eventually resolved by three different bugs in the underlying driver:

At any rate, no fault of this crate (if anyone were to play a game of blame's I'd split it equally between Nordic for some weird choices in the hardware, RIOT for having buggy drivers and myself for not doing more diverse tests) -- sorry for the noise.

eldruin commented 7 months ago

No worries. I am glad for the thorough investigation.