Sensirion / embedded-sps

Embedded i2c Driver for Sensirion Particulate Matter Sensors - Download the Zip Package from the Release Page
https://github.com/Sensirion/embedded-sps/releases
BSD 3-Clause "New" or "Revised" License
45 stars 15 forks source link

Set pointer address does not work for locations greater than 0x0300 #7

Closed nseidle closed 5 years ago

nseidle commented 5 years ago

Hi, We are able to follow the datasheet to enable readings and set the address pointer to 0x0300 and read 32 bytes of measurement data (CRCs checkout and all that). However, we are using an Arduino Uno. On an Arduino the I2C buffer is limited to 32 bytes. So it is necessary to read 32 bytes of measured values, then move the pointer to 0x0320, then continue reading the measurement values. This fails. We can set the pointer to 0x0300 but any other address is read as 0xFF bytes with failed (0xFF) CRC bytes.

Is this a hardware limitation? If we are unable to set the address pointer then your Arduino example will also be limited to mass concentrations and unable to report number concentrations.

I suspect this is an issue for this repo/driver for Arduino (example only prints PM2.5) but we're not technical enough to get the driver to compile under Arduino, so I can't test the theory.

Let me know:

Thanks! -Nathan

abrauchli commented 5 years ago

Hi @nseidle thanks for the hint on the max read size. Unlike "dumb" sensors, the SPS (and all our sensors) do not allow external pointer increments, the 0x0300 is understood as a command (0x0320 is not) - all state tracking is internal. What could work is resuming with a second read. To reuse the build-in read methods with CRC-checks you'll need to split it on a CRC byte. Go for the first 5, then the other 5 32b values in a second read:

That'd be the code around line 110 in sps30.c in s16 sps30_read_measurement(struct sps30_measurement *measurement)


    ret = sensirion_i2c_read_cmd(SPS_I2C_ADDRESS, SPS_CMD_READ_MEASUREMENT,
                                 data->u16, SENSIRION_NUM_WORDS(data) / 2); // <-- only read half the words
    if (ret != STATUS_OK)
        return ret;

    // Then read the second half of the measurements into the upper half of the buffer
    ret = sensirion_i2c_read_words(SPS_I2C_ADDRESS,
                                   data[5].u16, SENSIRION_NUM_WORDS(data) / 2);
    if (ret != STATUS_OK)
                return ret;

    SENSIRION_WORDS_TO_BYTES(data->u16, SENSIRION_NUM_WORDS(data));

Give it a try and let me know if that works

nseidle commented 5 years ago

Thanks for the pointer! We really want this to work but we're still having problems.

sps30 capture

In the above image we are requesting 0x0300 and the first 6 bytes come back as expected. We then do not release the I2C bus and request 6 more bytes 1 ms later. Unfortunately the data bytes match and we doubt PM1.0 would be identical to PM2.5.

nseidle commented 5 years ago

As an aside, releasing the bus has the same output.

abrauchli commented 5 years ago

Hi @nseidle thanks for the feedback. It's not unlikely for pm1.0 and pm2.5 to report the same value, esp. towards the beginning of a measurement series. What should be different is the mass concentrations (mc, first four 32b values with indices 0:3) from the number concentration (nc, 32b values 4:8) values, as well as the typical particle size (index 9) - after the first couple of measurements are through.. I'm not currently on premises so I don't have access to the hardware. I'll give it a try when I get back and get a chance. Hopefully before the end of the week, otherwise please bump the issue by replying again.

abrauchli commented 5 years ago

In the meantime maybe the UART interface could be a viable alternative for you? We have a UART driver with almost the same C interface (it's a drop-in replacement if you discard the SHDLC error codes). https://github.com/Sensirion/embedded-uart-sps

Make sure to leave the SPS' Pin4 floating to get the UART interface.

nseidle commented 5 years ago

Thanks - we know the UART interface is available but for a variety of reasons we need the I2C interface to work.

abrauchli commented 5 years ago

Hi again. So, I asked about the inner workings and the sensor will reset the internal send buffer on every start. So the only choice you have is basically not to restart the transfer but to merely pause it. Start, Read 32 Bytes, --- Pause ---, Read 28 Bytes, Stop. The arduino i2c controller might not let you do that though, so the other alternative is bit-banging, software i2c or i2c over GPIOs, then you're not limited to the 32 Byte buffer. There are obviously downsides so you'll have to evaluate whether that's feasible. You might also need external pull-up resistors if the GPIOs don't provide them. It seems we don't yet provide a software-i2c sample implementation for arduino but the implementation should be relatively straight forward.

nseidle commented 5 years ago

Thank you so much for checking! Sounds like a hardware bug that cannot be surmounted. Indeed, bit banging is an option but far from ideal and very difficult to support across the Arduino landscape. I2C was really what set this sensor apart from many others. A bummer to hear it won't be usable. If you have any sway, please push for a better solution in future version.

abrauchli commented 5 years ago

Hi @nseidle the hardware engineers noted the issue and we'll try to avoid it in the future. Needless to say that it's in our interest for our hardware to run with a default configuration on Arduino.

On the Arduino front, maybe it's also worth investigating patching the Wire library to provide a variable rx/tx buffer size since it has definitely appeared in other contexts before: https://forum.arduino.cc/index.php?topic=46914.0

On a quick search I found the source for the esp8266 - It might work with an #ifndef BUFFER wrapping around #define BUFFER 32 (Arduino >= 0019 according to the forum post) https://github.com/esp8266/Arduino/blob/master/libraries/Wire/Wire.cpp

Can I close the issue?

nseidle commented 5 years ago

In the past we have had to dig into the Arduino code base to modify the I2C buffer size. It can be done but asking general users to do so is quite advanced and error prone. The I2C buffer size issue has been brought up with the Arduino core devs as well and, while there has been talk for years, the group has not put a method in place, primarily because very few sensors have this issue.

The ESP32 and ESP8266 cores may be more flexible, I haven't had the need to modify them before. But we generally don't build Arduino libs that don't work with the Uno.

Yep, feel free to close. We'll post the SPS30 on SparkFun with a note about the I2C limitations.

Thank you for all your help!

michapr commented 5 years ago

request for lib change was merged for ESP8266 (after have remebered about it ;) ) https://github.com/esp8266/Arduino/pull/3576#pullrequestreview-194377996 So should work in future releases here.

winkj commented 5 years ago

we just pushed a library for Arduino, using an alternative I2C master implementation (i.e. bypassing Wire) here: https://github.com/Sensirion/arduino-sps

This makes the code independent from the buffer size in the Wire library