search

pimoroni / sgp30-python

Python library for the SGP30 air quality sensor
https://shop.pimoroni.com/products/sgp30-air-quality-sensor-breakout
MIT License
38 stars 18 forks source link

Error in get_unique_id function #10

Open simon3270 opened 3 years ago

simon3270 commented 3 years ago

The SGP30 get_unique_id function returns the chip ID. The code at lines 124-126 of https://github.com/pimoroni/sgp30-python/blob/master/library/sgp30/__init__.py are:

def get_unique_id(self):
    result = self.command('get_serial_id')
    return result[0] << 32 | result[1] << 16 | result[0]

I believe that the third field on the last line should be result[2].

The code as written returned 000001120000 for my device. With my fix, it returned 000001127bf2.

I've done this as an issue rather than a Pull request, as I am not completely sure whether it is the first or the third field which should be result[2]. I realised there was a problem with the code because I've written a C++ Pico library for this device, and the unique IDs from the 2 sets of code differed. When writing the C++ code, I had assumed that the least significant word of the returned value was the third word in the data read from the chip. You may disagree ;-)

Gadgetoid commented 3 years ago

Good spot- I'm also unsure if it's the first or third (little or big endian data I guess) and currently don't have a Pi set up to test this.

Glancing at https://github.com/zinob/RPI_SGP30/blob/32d381c261cde0e8db9b0fdcc8d1d4c76c4c8618/sgp30/sgp30.py#L58, the decode is given as: [i<<8 | j for i,j in a]`` which would suggest ours should beresult[0] << 32 | result[1] << 16 | result[2]` if you look past the difference between 8-bit and 16-bit reads.