Seeed-Studio / grove.py

Python library for Seeedstudio Grove devices
MIT License
149 stars 97 forks source link

Bug: Wrong i2c bus number for CM4 #71

Closed michaelhaaf closed 2 months ago

michaelhaaf commented 8 months ago

Problem

The smbus instantiation logic in i2c.py selects the wrong i2c bus number for CM4 Pi.

This presents itself in the form of an uncaught IOError that crashes a python script when any of the adc methods are called, for eg:

>>> from grove.adc import ADC
>>> adc = ADC() 
>>> print(adc.read_voltage(0))
Check whether I2C enabled and   Grove Base Hat RPi  or  Grove Base Hat RPi Zero  inserted

I have written a workaround (see Bug Fix) but I don't understand the i2c logic well enough to know if this is the appropriate resolution in general.

Explanation

When the ADC class is instantiated, the bus number is chosen based on the value of GPIO.RPI_REVISION. See https://github.com/Seeed-Studio/grove.py/blob/master/grove/i2c.py#L39-L49.

I don't understand the motivation behind this choice, but I can see it causes the wrong outcome for CM4 Pis (such as the reTerminal).

The CM4 has RPi.GPIO.RPI_REVISION=0. https://raspi.tv/2014/rpi-gpio-quick-reference-updated-for-raspberry-pi-b.

The above logic results in bus=0 for the smbus used by the adc class. However, the bus should be 1, as can be seen by running i2cdetect:

pi12@pi12:~/ $ i2cdetect -y 0
     0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
00:                         -- -- -- -- -- -- -- -- 
10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
50: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
70: -- -- -- -- -- -- -- --                         

pi12@pi12:~/ $ i2cdetect -y 1
     0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
00:                         -- -- -- -- -- -- -- -- 
10: -- -- -- -- -- -- -- -- -- UU -- -- -- -- -- -- 
20: -- -- -- -- -- -- -- -- -- UU -- -- -- -- -- -- 
30: -- -- -- -- -- -- -- -- UU -- -- -- -- -- -- -- 
40: -- -- -- -- -- UU -- -- -- -- -- -- -- -- -- -- 
50: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
70: -- -- -- -- -- -- -- --                 

Environment

Example

Assume there is an analog device connected at A0, that the i2c interface is enabled, and that there are no wiring issues (as can be seen by the successful "Bug Fix" result).

Bug

>>> from grove.adc import ADC
>>> adc = ADC(0x04) # 0x04 for STM base hat, see https://github.com/Seeed-Studio/grove.py/issues/64 
>>> print(adc.read_voltage(0))
Check whether I2C enabled and   Grove Base Hat RPi  or  Grove Base Hat RPi Zero  inserted

Bug Fix

It is possible to extend the definition of the ADC class such that the bus number can be chosen at instantiation.

>>> import grove.i2c
>>> from grove.adc import ADC
>>> class customADC(ADC):
...    def __init__(self, address=0x04, bus=1)
...        self.address=address
...        self.bus=grove.i2c.Bus(bus)
...
>>> adc = customADC() 
>>> print(adc.read_voltage(0))
1628

Since we can make bus an optional parameter, this extension could be pulled upstream without breaking the interface, however, this would replace the existing bus determination logic.

Is that better? In my case, yes, since the bus determination logic does not work for my device. But, I'm not sure in general what makes the most sense, I don't understand how i2c bus numbers are determined for different devices.

Related issues

Not 100% certain, but this may be related to the following issues:

https://github.com/Seeed-Studio/grove.py/issues/70 https://github.com/Seeed-Studio/grove.py/issues/68 https://github.com/Seeed-Studio/grove.py/issues/61 https://github.com/Seeed-Studio/grove.py/issues/46

is-qian commented 2 months ago

Yes, this issue has been fixed in this commit on b22c1bd4491e6bc909d4ea860aab4b7f56af3a59 .