I2C driver problems and best practices

General discussions and questions abound development of code with MicroPython that is not hardware specific.
Target audience: MicroPython Users.
Post Reply
User avatar
n-elia
Posts: 7
Joined: Wed May 19, 2021 10:57 pm

I2C driver problems and best practices

Post by n-elia » Sat Jun 18, 2022 10:45 am

Hi all,

I am maintaining a MAX30102 I2C sensor driver.

I am not aware of any best practices, so I took inspiration from I2C drivers here and there, and the library (as of today) basically requires the user to create a SoftI2C instance, which will then be used to communicate with the target sensor.

When the class gets instantiated, it performs a check for ensuring that the target sensor is connected to the I2C bus:

Code: Select all

        try:
            self._i2c.readfrom(self._address, 1)
        except OSError as error:
            if error.errno == uerrno.ENODEV:
                raise RuntimeError("Sensor not found on I2C bus.")
            else:
                raise RuntimeError(f"Error while reading from I2C bus: OSError code {error.errno}")
I obtain different behaviour with different boards when running the example script that I provided into the repository.
Assuming that I2C pins are correct, consider the following snippet:

Code: Select all

i2c = SoftI2C(sda=Pin(22), scl=Pin(21), freq=400000)
# i2c.scan()
sensor = MAX30102(i2c=i2c)
  • When using an ESP32-based board, the result is that `OSError: [Errno 19] ENODEV` is raised when the board gets a soft reset (Ctrl-D on REPL prompt). Instead, if the board is hard reset, it works as expected.
  • When using a RP2 board, the result is that `OSError: [Errno 19] ENODEV` is always raised.
If line 2 is uncommented, then the snippet always works as expected.

So, what am I missing? Is a I2C.scan() required before using the I2C bus? And why is a hard reset solving the problem with ESP32-based boards?

I may solve the problem by using a I2C.scan() call within the initial checks (when the class gets instantiated), but I'd like to understand why.

Thank you in advance for your help!

User avatar
jimmo
Posts: 2754
Joined: Tue Aug 08, 2017 1:57 am
Location: Sydney, Australia
Contact:

Re: I2C driver problems and best practices

Post by jimmo » Mon Jun 20, 2022 3:56 am

n-elia wrote:
Sat Jun 18, 2022 10:45 am
I am not aware of any best practices, so I took inspiration from I2C drivers here and there, and the library (as of today) basically requires the user to create a SoftI2C instance, which will then be used to communicate with the target sensor.
Generally the best practice for drivers is to have the bus passed to to the constructor of the driver. (That way the user is free to use SoftI2C, or whatever port-specific hardware I2C construction and pin assignment needs to happen). Sounds like that's what you're doing.
n-elia wrote:
Sat Jun 18, 2022 10:45 am
When the class gets instantiated, it performs a check for ensuring that the target sensor is connected to the I2C bus:
That looks fine, but I'm curious why you do that rather than checking if the target address is in the results from scan? (Which sounds like what you're doing now as a workaround anyway).
n-elia wrote:
Sat Jun 18, 2022 10:45 am
So, what am I missing? Is a I2C.scan() required before using the I2C bus?
No, it shouldn't be. This sounds like a bug. Just to clarify, what you're seeing is, with a device connected
* on rp2 and esp32 (but only after a soft reset), you see an OSError(ENODEV), unless you first do a scan.
* on esp32 (only after hard reset), it works straight off the bat.

I will try and reproduce here.

User avatar
karfas
Posts: 193
Joined: Sat Jan 16, 2021 12:53 pm
Location: Vienna, Austria

Re: I2C driver problems and best practices

Post by karfas » Mon Jun 20, 2022 5:02 am

n-elia wrote:
Sat Jun 18, 2022 10:45 am
When the class gets instantiated, it performs a check for ensuring that the target sensor is connected to the I2C bus:
I don't like this check during class instantiation.

In my opinion, a "driver" shouldn't talk to the hardware until instructed to do so by the application. While I see some benefit in this approach (aka "instance represents existing piece of hardware") things become complicated from a user perspective when this hardware simply isn't there (during development, because it needs to be activated by means outside the scope of the driver).

In these cases, you force the user to instantiate the class at the last possible moment. This has consequences regarding memory usage and the like, while most(?) calls to the driver functions will report failure anyway.
A few hours of debugging might save you from minutes of reading the documentation! :D
My repositories: https://github.com/karfas

User avatar
n-elia
Posts: 7
Joined: Wed May 19, 2021 10:57 pm

Re: I2C driver problems and best practices

Post by n-elia » Mon Jun 20, 2022 8:22 am

Thank you for taking time for answering my question!
jimmo wrote:
Mon Jun 20, 2022 3:56 am
That looks fine, but I'm curious why you do that rather than checking if the target address is in the results from scan? (Which sounds like what you're doing now as a workaround anyway).
I've seen that pattern in other drivers, and I thought that maybe the i2c module covers the 'not found' error as OSError(ENODEV) as well as other possible errors. In fact, read errors would still be raised when performing read operation elsewhere, so I understood that using `scan()` is no way a worse solution.
Yes, I confirm that now, as a workaround, I'm asserting that address is found after a i2c scan. And it seems to me a very clean approach.

Also, I should not catch errors within the driver, because the raised errors depend on the i2c module, which is indeed chosen within the application, right?
jimmo wrote:
Mon Jun 20, 2022 3:56 am
No, it shouldn't be. This sounds like a bug. Just to clarify, what you're seeing is, with a device connected
* on rp2 and esp32 (but only after a soft reset), you see an OSError(ENODEV), unless you first do a scan.
* on esp32 (only after hard reset), it works straight off the bat.

I will try and reproduce here.
You got it right! Thanks for trying!

---
karfas wrote:
Mon Jun 20, 2022 5:02 am
In my opinion, a "driver" shouldn't talk to the hardware until instructed to do so by the application. While I see some benefit in this approach (aka "instance represents existing piece of hardware") things become complicated from a user perspective when this hardware simply isn't there (during development, because it needs to be activated by means outside the scope of the driver).

In these cases, you force the user to instantiate the class at the last possible moment. This has consequences regarding memory usage and the like, while most(?) calls to the driver functions will report failure anyway.
Thank you for your suggestion. So, assuming that we have, e.g.:

Code: Select all

d = MyDriver() # instantiation
d.init(my_settings) # initialization
d.read() # actual usage
You'd raise errors only on initialization and actual usage, which should be the only methods that actually need to communicate using the i2c bus, right?
Also, the init(), if it's supposed to talk with the sensor, should need an explicit call and it should never be called within the constructor. (altough this behaviour would be different from internal drivers, e.g. micropython/blob/master/drivers/display/ssd1306.py)
Did I get it right?
Thank you!

User avatar
karfas
Posts: 193
Joined: Sat Jan 16, 2021 12:53 pm
Location: Vienna, Austria

Re: I2C driver problems and best practices

Post by karfas » Mon Jun 20, 2022 2:42 pm

n-elia wrote:
Mon Jun 20, 2022 8:22 am
Thank you for your suggestion. So, assuming that we have, e.g.:

Code: Select all

d = MyDriver() # instantiation
d.init(my_settings) # initialization
d.read() # actual usage
You'd raise errors only on initialization and actual usage, which should be the only methods that actually need to communicate using the i2c bus, right?
Also, the init(), if it's supposed to talk with the sensor, should need an explicit call and it should never be called within the constructor. (altough this behaviour would be different from internal drivers, e.g. micropython/blob/master/drivers/display/ssd1306.py)
This was meant as a suggestion (something to think/discuss about). Maybe it's better to copy the behaviour of existing drivers; I really don't know what is "better" or "more pythonic" in the long run. Even when this pattern is used by most drivers, this doesn't mean it's the best possible way to do it.
However, I don't see a benefit of not being able to instantiate the driver in absence of the hardware. This only causes troubles (maybe the sensor requires a little longer for initialization after power on/reset, ...). Additional, any useful embedded application will need to react to short communication failures in some meaningful way. How can this be done without an instance of the driver ?

For your special driver I would
  • remove the scan() completely (I think we can trust the user he knows the i2c address of his hardware).
  • remove check_part_id() from __init__(). If the user isn't sure about the hardware he has, he can call this function himself.
Anything else will raise exceptions from i2c.read(), i2c.write() when communication fails (a situation the user will need to handle anyway).

My dislike of i2c.scan() etc. during __init__() comes from the experience with a battery-powered ESP32 board (the LILYGO plant sensor). There the sensors get enabled by a special GPIO pin (to save some energy, I think). I was a little annoyed that I needed to create proxy objects for the real sensor drivers to cope with this.
A few hours of debugging might save you from minutes of reading the documentation! :D
My repositories: https://github.com/karfas

User avatar
n-elia
Posts: 7
Joined: Wed May 19, 2021 10:57 pm

Re: I2C driver problems and best practices

Post by n-elia » Mon Jun 20, 2022 4:25 pm

Indeed, an exchange of ideas is what I was looking for! Thank you for sharing your point of view, much appreciated.
karfas wrote:
Mon Jun 20, 2022 2:42 pm
  • remove check_part_id() from __init__(). If the user isn't sure about the hardware he has, he can call this function himself.
I agree with that, but wouldn't you put this check into, e.g., setup_sensor()? Don't you think that having this check is still good for preventing wrong usage of peripherals? I mean, what if you connect a peripheral to the wrong wire and perform an unwanted write?
karfas wrote:
Mon Jun 20, 2022 2:42 pm
Anything else will raise exceptions from i2c.read(), i2c.write() when communication fails (a situation the user will need to handle anyway).

My dislike of i2c.scan() etc. during __init__() comes from the experience with a battery-powered ESP32 board (the LILYGO plant sensor). There the sensors get enabled by a special GPIO pin (to save some energy, I think). I was a little annoyed that I needed to create proxy objects for the real sensor drivers to cope with this.
Very good point, I agree. Otherwise, it would be very difficult to deal with temporarily unavailable sensors. Moreover, the raised errors will be predictable, because they depend only on the chosen i2c library.
karfas wrote:
Mon Jun 20, 2022 2:42 pm
For your special driver I would
  • remove the scan() completely (I think we can trust the user he knows the i2c address of his hardware).
I would, but an issue is happening where RP2040 boards do not work without making a i2c scan before.
This could easily be addressed application-side, performing a i2c scan right before using the sensor (maybe it is also mandatory if you need to check if the i2c peripheral is actually powered on). I'll wait jimmo feedback before continuing, and - since I would like that the usage is straightforward for new users - then maybe I'll move the scan() from the driver to the example script. Thank you!

User avatar
jimmo
Posts: 2754
Joined: Tue Aug 08, 2017 1:57 am
Location: Sydney, Australia
Contact:

Re: I2C driver problems and best practices

Post by jimmo » Tue Jun 21, 2022 1:18 am

n-elia wrote:
Mon Jun 20, 2022 8:22 am
You got it right! Thanks for trying!
OK I tried to replicate this on an rp2040 and ESP32 but wasn't able to. No combination of SoftI2C or I2C, soft/hard reset, etc. The device always responded to i2c.readfrom(addr, 1).

Is it possible this is a quirk of the MAX30102? I can't quite see how, but if there's any chance you have another i2c device available would be good to confirm. I think in most cases the ENODEV comes from the device NACK'ing the request. Is it possible that the hard reset of your ESP32 is also power cycling the device? What if you do something more specific, supported by the device, rather than just trying to read a single byte (i.e. try and read an actual register or something?).
karfas wrote:
Mon Jun 20, 2022 5:02 am
I don't like this check during class instantiation.
Good points, especially about low-power situations. I had kind of assumed that the driver would be created as-needed, but yeah like you say this isn't great for memory (although maybe not too bad). But perhaps (also like you say) we should just trust the user got the address right.

User avatar
n-elia
Posts: 7
Joined: Wed May 19, 2021 10:57 pm

Re: I2C driver problems and best practices

Post by n-elia » Tue Jun 21, 2022 4:34 pm

jimmo wrote:
Tue Jun 21, 2022 1:18 am
OK I tried to replicate this on an rp2040 and ESP32 but wasn't able to. No combination of SoftI2C or I2C, soft/hard reset, etc. The device always responded to i2c.readfrom(addr, 1).

Is it possible this is a quirk of the MAX30102? I can't quite see how, but if there's any chance you have another i2c device available would be good to confirm. I think in most cases the ENODEV comes from the device NACK'ing the request. Is it possible that the hard reset of your ESP32 is also power cycling the device? What if you do something more specific, supported by the device, rather than just trying to read a single byte (i.e. try and read an actual register or something?).
Thanks for trying!

Following your advices, I tried with an external power supply, with the same results. I also tried to write and read actual registers, unfortunately with the same results.

Then, I tried again with both the MAX30102 and a MCP9808 (temperature sensor) on the same I2C bus. I had unexpected behaviours!

Let's consider the following main.py:

Code: Select all

 1. # main.py
 2. from utime import sleep
 3. from machine import SoftI2C, Pin
 4. 
 5. MAX3010X_I2C_ADDRESS = 0x57
 6. MCP9808_I2C_ADDRESS = 0x18
 7. 
 8. def main():
 9.     i2c = SoftI2C(sda=Pin(8), scl=Pin(9), freq=400000)
10.     
11.     while(True):
12.         print(i2c.scan())
13.         print(i2c.readfrom(MCP9808_I2C_ADDRESS, 1))
14.         print(i2c.readfrom(MAX3010X_I2C_ADDRESS, 1))
15.         sleep(1)
16. 
17. if __name__ == '__main__':
18.     main()
These are the results I obtained:
  • If run as is, the results are as expected:

    Code: Select all

    MicroPython v1.19.1 on 2022-06-18; TinyS3 with ESP32-S3-FN8
    Type "help()" for more information.
    >>> 
    ────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
    
    MPY: soft reboot
    [24, 87]
    b'\x00'
    b'\x15'
    [24, 87]
    b'\x00'
    b'\x15'
    Traceback (most recent call last):
      File "main.py", line 20, in <module>
      File "main.py", line 17, in main
    KeyboardInterrupt: 
    MicroPython v1.19.1 on 2022-06-18; TinyS3 with ESP32-S3-FN8
    Type "help()" for more information.
    >>> 
    
  • If line 13 is commented out, everything works as expected:

    Code: Select all

    MicroPython v1.19.1 on 2022-06-18; TinyS3 with ESP32-S3-FN8
    Type "help()" for more information.
    >>> 
    ────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
    
    MPY: soft reboot
    [24, 87]
    b'\x15'
    [24, 87]
    b'\x15'
    [24, 87]
    b'\x15'
    [24, 87]
    b'\x15'
    Traceback (most recent call last):
      File "main.py", line 20, in <module>
      File "main.py", line 17, in main
    KeyboardInterrupt: 
    MicroPython v1.19.1 on 2022-06-18; TinyS3 with ESP32-S3-FN8
    Type "help()" for more information.
    >>> 
    
  • If line 12 is commented out, everything works as expected (!):

    Code: Select all

    MicroPython v1.19.1 on 2022-06-18; TinyS3 with ESP32-S3-FN8
    Type "help()" for more information.
    >>> 
    ────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
    
    MPY: soft reboot
    b'\x00'
    b'\x15'
    b'\x00'
    b'\x15'
    b'\x00'
    b'\x15'
    b'\x00'
    b'\x15'
    Traceback (most recent call last):
      File "main.py", line 20, in <module>
      File "main.py", line 17, in main
    KeyboardInterrupt: 
    MicroPython v1.19.1 on 2022-06-18; TinyS3 with ESP32-S3-FN8
    Type "help()" for more information.
    >>> 
    
  • If lines 12,13 are commented out, ENODEV is raised:

    Code: Select all

    MicroPython v1.19.1 on 2022-06-18; TinyS3 with ESP32-S3-FN8
    Type "help()" for more information.
    >>> 
    ────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
    
    MPY: soft reboot
    Traceback (most recent call last):
      File "main.py", line 20, in <module>
      File "main.py", line 16, in main
    OSError: [Errno 19] ENODEV
    MicroPython v1.19.1 on 2022-06-18; TinyS3 with ESP32-S3-FN8
    Type "help()" for more information.
    >>> 
    
I have no idea of what's happening. The Maxim sensor works only the 2nd time I use the I2C bus. It seems to me that your hint is correct and the Maxim sensor is involved!

Edit: please take a look to attached picture.
Attachments
issue_maxim_max30102_i2c_mp.jpg
issue_maxim_max30102_i2c_mp.jpg (208.82 KiB) Viewed 3552 times

User avatar
karfas
Posts: 193
Joined: Sat Jan 16, 2021 12:53 pm
Location: Vienna, Austria

Re: I2C driver problems and best practices

Post by karfas » Thu Jun 23, 2022 10:23 am

jimmo wrote:
Tue Jun 21, 2022 1:18 am
I had kind of assumed that the driver would be created as-needed, but yeah like you say this isn't great for memory (although maybe not too bad).
This makes me wonder how you design your applications. Do your sensor interfaces all include code similar to the following ?

Code: Select all

def sensor_read(addr, ...):
	drv = Driver(addr)
	val = drv.read(...)
	return val
The tendency in my programs is more "put all cannons in position before shooting" - this might come from C-on-embedded experience.

Regarding the MAX30102 problems:
For me, this looks like a unclean bus (maybe amplified by a not-real-good interface of the MAX30102).
@n-elia:
Did you try to use low-valued pull up resistors (~5k) and/or short delays between accessing the different devices on your bus ?
A few hours of debugging might save you from minutes of reading the documentation! :D
My repositories: https://github.com/karfas

User avatar
n-elia
Posts: 7
Joined: Wed May 19, 2021 10:57 pm

Re: I2C driver problems and best practices

Post by n-elia » Mon Jun 27, 2022 7:57 pm

karfas wrote:
Thu Jun 23, 2022 10:23 am
For me, this looks like a unclean bus (maybe amplified by a not-real-good interface of the MAX30102).
@n-elia:
Did you try to use low-valued pull up resistors (~5k) and/or short delays between accessing the different devices on your bus ?
Hi,
I'm using a breakout board which has 4.7kOhm pull up resistors.
I tried using delays, with no success.

I still can't get why it works after a I2C scan!

Post Reply