feature/optional_i2c_bus #16671

malcolm-sparkfun · 2025-01-29T22:16:35Z

Summary

This pull request gives the option to not pass an I2C Bus ID when creating a machine I2C object on RP2 and ESP32. If the ID is not provided on RP2, the default bus ID (PICO_DEFAULT_I2C) is used. For ESP32, I2C_NUM_0 is used. This would allow users on both platforms to simply declare an I2C object with machine.I2C() without passing any arguments, thus creating an object with the default I2C ID, SCL, and SDA.

Our use case is a higher-level python driver that creates machine.I2C objects. We would like the driver to be portable to many different RP2 and ESP32 boards without having to explicitly specify these arguments.

Testing

Tested on RP2 with Raspberry Pi Pico W:

Created machine.I2C object by passing no arguments to I2C(). Verified that it successfully could use the scan() method to detect a connected sensor.
Exercised I2C object by using it with an example program performing reads and writes to a sensor.

Tested on ESP32 with Sparkfun Thing Plus - ESP32 WROOM (Micro-B):

Created machine.I2C object by passing no arguments to I2C(). Verified that it successfully could use the scan() method to detect a connected sensor.
Exercised I2C object by using it with an example program performing reads and writes to a sensor.

Trade-offs and Alternatives

For ESP32, this required removal of the MP_MACHINE_I2C_CHECK_FOR_LEGACY_SOFTI2C_CONSTRUCTION() macro in machine_hw_i2c_make_new(). This macro would cause the I2C object to be created with an ID of -1 if not explicitly specified. This would cause a SoftwareI2C object to be created. Thus, this could break backwards compatibility for ESP32 if users have code where they don't specify the ID (or specify it as -1) and expect that to lead to creation of a SoftwareI2C object. However, this is a legacy construction and likely should be removed anyways. It is a more intuitive behavior for creation of an I2C object with no ID argument to get an I2C object with default ID rather than an entirely different SoftwareI2C object.
As with any default argument that is optionally supplied by a user, making it optional means that it may be more frequently overlooked. The default may sometimes be used when the user actually should look into it deeply enough to decide which ID/Bus they want to use.

github-actions · 2025-01-29T22:24:34Z

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:    +0 +0.000% TEENSY40
        rp2:    -8 -0.001% RPI_PICO_W
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32:    +0 +0.000% VIRT_RV32

Gadgetoid · 2025-02-03T10:04:18Z

ports/esp32/machine_i2c.c


    // Parse args
    enum { ARG_id, ARG_scl, ARG_sda, ARG_freq, ARG_timeout };
    static const mp_arg_t allowed_args[] = {
-        { MP_QSTR_id, MP_ARG_REQUIRED | MP_ARG_OBJ, {.u_obj = MP_OBJ_NULL} },
+        { MP_QSTR_id, MP_ARG_OBJ, {.u_obj = MP_OBJ_NULL} },


You can use { MP_QSTR_id, MP_ARG_INT, {.u_int = I2C_NUM_0} }, here to avoid the default and MP_OBJ_NULL checks below.

Note: Retrieving the value then becomes mp_int_t i2c_id = args[ARG_id].u_int

Gadgetoid · 2025-02-03T10:21:22Z

ports/rp2/machine_i2c.c

@@ -97,7 +97,7 @@ static void machine_i2c_print(const mp_print_t *print, mp_obj_t self_in, mp_prin
 mp_obj_t machine_i2c_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *all_args) {
    enum { ARG_id, ARG_freq, ARG_scl, ARG_sda, ARG_timeout };
    static const mp_arg_t allowed_args[] = {
-        { MP_QSTR_id, MP_ARG_REQUIRED | MP_ARG_OBJ },
+        { MP_QSTR_id,  MP_ARG_OBJ, {.u_rom_obj = MP_ROM_NONE} },


Still not ideal but I'd probably do:

#ifdef PICO_DEFAULT_I2C { MP_QSTR_id, MP_ARG_INT, {.u_int = PICO_DEFAULT_I2C} }, #else { MP_QSTR_id, MP_ARG_INT, {.u_int = -1} }, #endif

Rather than inline the checks below. Saves code.

(Not really sure why this was ever MP_ARG_OBJ?)

It doesn't feel like this breaks the API, but it seems to break expectations on Pico since - afaict - machine.I2C() will not give you a properly configured default I2C instance. (It doesn't handle PICO_DEFAULT_I2C_SCL_PIN and PICO_DEFAULT_I2C_SDA_PIN so I2C will wiggle into the void and thus your objective - portable code - wont quite be met?)

Edit crossed out erroneous statement: I2C gets initialised by since freq == 0:

micropython/ports/rp2/machine_i2c.c

Lines 149 to 158 in 53f1a3a

if (n_args > 1 || n_kw > 0 || self->freq == 0) {

self->freq = args[ARG_freq].u_int;

i2c_init(self->i2c_inst, self->freq);

self->freq = i2c_set_baudrate(self->i2c_inst, self->freq);

self->timeout = args[ARG_timeout].u_int;

gpio_set_function(self->scl, GPIO_FUNC_I2C);

gpio_set_function(self->sda, GPIO_FUNC_I2C);

gpio_set_pulls(self->scl, true, 0);

gpio_set_pulls(self->sda, true, 0);

}

Gadgetoid · 2025-02-03T10:27:42Z

I kinda like where you're going with this, though afaict it doesn't make machine.I2C() work out of the box on the Pico since, by default, no I2C pins are configured. I should probably poke that because it doesn't feel like I'm right, even though my foggy Monday morning sniffing around the code suggests as much.

Of course, to play devil's advocate here, your library shouldn't construct machine.I2C() instances internally at all- they should be user supplied! (and yes I say this with the full weight of understanding that I am throwing stones in a 12 year old glass house 😆)

dpgeorge · 2025-02-08T10:59:34Z

I actually quite like this idea. After all, you can already do things like network.WLAN() to get the default WLAN interface (which is usually STA, ie it's equivalent to network.WLAN(network.WLAN.IF_STA)).

though afaict it doesn't make machine.I2C() work out of the box on the Pico since, by default, no I2C pins are configured

I think it will work on Pico. There are default pins for all I2C objects which should get configured the first time machine.I2C() is instantiated.

Of course, to play devil's advocate here, your library shouldn't construct machine.I2C() instances internally at all- they should be user supplied!

I agree with this. A driver usually doesn't need to import machine at all.

Gadgetoid · 2025-02-08T16:36:38Z

I think it will work on Pico. There are default pins for all I2C objects which should get configured the first time machine.I2C() is instantiated.

Oof I think I see how this works, falls through to the check for freq == 0 (the default value in machine_i2c_obj) and initialises MICROPY_HW_I2Cn_SCL and MICROPY_HW_I2Cn_SDA.

There is a comment saying as much 😆 but I can see why I missed that.

malcolm-sparkfun · 2025-02-20T20:01:05Z

Thank you for your feedback. I have made the suggested changes. Let me know if there's anything else I should do before this is merged.

ports/esp32/machine_i2c.c

This commit gives the option to not pass an I2C Bus ID when creating a machine I2C object. If the ID is not provided, the default bus ID (which is `PICO_DEFAULT_I2C`) is used. This allows users to simply declare an I2C object with `machine.I2C()` without passing any arguments, thus creating an object with the default I2C ID, SCL, and SDA. Signed-off-by: Malcolm McKellips <malcolm.mckellips@sparkfun.com>

Similar to the previous commit, this allows constructing an I2C instance without specifying an ID. The default ID is I2C_NUM_0. Signed-off-by: Malcolm McKellips <malcolm.mckellips@sparkfun.com>

ricksorensen · 2025-04-09T20:43:19Z

My experience with a XIAO C3 running with the GENERIC_C3 build:
MicroPython v1.25.0-preview.531.gef8282c71 on 2025-04-09; XIAOC3 with ESP32C3
is that both i2=I2C() and i2=I2C(0) hang the REPL with no return requiring a hardware reset.

Using 1.24.1 release build, i2=I2C(0) hangs the REPL as above, but i2=I2C() fails with

Warning: I2C(-1, ...) is deprecated, use SoftI2C(...) instead
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: 'scl' argument required
>>> i2=I2C(0)

I will look at what is causing the hang. Note that on the XIAO S3 both i2=I2C() and i2=I2C(0) work. Maybe because there are 2 HW I2C channels available?

malcolm-sparkfun mentioned this pull request Jan 29, 2025

Create a pr for micropython that enables the default I2C bus to be used if no parameters provided. sparkfun/pdp-qwiic-micro-python#391

Closed

2 tasks

Gadgetoid reviewed Feb 3, 2025

View reviewed changes

dpgeorge mentioned this pull request Feb 9, 2025

samd/boards: Add definitions for Generic boards, Adafruit NeoKey Trinkey and Adafruit QT PY SAMD21. #16629

Merged

dpgeorge added port-rp2 port-esp32 labels Feb 12, 2025

dpgeorge approved these changes Feb 27, 2025

View reviewed changes

dpgeorge reviewed Feb 27, 2025

View reviewed changes

ports/esp32/machine_i2c.c Show resolved Hide resolved

malcolm-sparkfun added 2 commits February 28, 2025 15:17

esp32/machine_i2c: Make I2C bus ID arg optional with default.

c143eb5

Similar to the previous commit, this allows constructing an I2C instance without specifying an ID. The default ID is I2C_NUM_0. Signed-off-by: Malcolm McKellips <malcolm.mckellips@sparkfun.com>

dpgeorge force-pushed the feature/optional_i2c_bus branch from 67f83c0 to c143eb5 Compare February 28, 2025 04:20

dpgeorge merged commit c143eb5 into micropython:master Feb 28, 2025
10 checks passed

This was referenced Mar 12, 2025

When miso=None is set, the default miso pin changed state. #16912

Open

ports/rp2: Prevent machine.SPI settings from persisting through a soft reset #16917

Open

ports/rp2: Optional spi_id and optional disabling of MISO pin #16922

Closed

malcolm-sparkfun mentioned this pull request Mar 18, 2025

mimxrt/machine_i2c.c: Make I2C bus ID arg optional with default. #16956

Closed

ricksorensen mentioned this pull request Apr 10, 2025

ESP32_GENERIC_C3/mpconfigboard.h: Add defaults for I2C pins. #17113

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

feature/optional_i2c_bus #16671

feature/optional_i2c_bus #16671

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

	if (n_args > 1 \|\| n_kw > 0 \|\| self->freq == 0) {
	self->freq = args[ARG_freq].u_int;
	i2c_init(self->i2c_inst, self->freq);
	self->freq = i2c_set_baudrate(self->i2c_inst, self->freq);
	self->timeout = args[ARG_timeout].u_int;
	gpio_set_function(self->scl, GPIO_FUNC_I2C);
	gpio_set_function(self->sda, GPIO_FUNC_I2C);
	gpio_set_pulls(self->scl, true, 0);
	gpio_set_pulls(self->sda, true, 0);
	}

Uh oh!

feature/optional_i2c_bus #16671

feature/optional_i2c_bus #16671

Uh oh!

Conversation

Summary

Testing

Trade-offs and Alternatives

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!