i2ctarget error with missing deinit for request

[Neradoc]

Since #10040 (9.2.5 and after) this happens with the i2ctarget example:

code starting...
9.789: INFO - read request to address '0x40'
Traceback (most recent call last):
  File "code.py", line 38, in <module>
AttributeError: 'I2CTargetRequest' object has no attribute 'deinit'

This is apparently because the new default __exit__ requires the presence of deinit in the object's dictionary, and I2CTargetRequest doesn't have one. It has a close() though, (which incidentally doesn't have a docstring).

Should I2CTargetRequest have a deinit() that calls common_hal_i2ctarget_i2c_target_close() and also maybe remove the close() method ?

circuitpython/shared/runtime/context_manager_helpers.c

Line 14 in d88fe10

mp_load_method(args[0], MP_QSTR_deinit, dest);

circuitpython/shared-bindings/i2ctarget/I2CTarget.c

Lines 395 to 405 in d88fe10

	static const mp_rom_map_elem_t i2ctarget_i2c_target_request_locals_dict_table[] = {
	{ MP_ROM_QSTR(MP_QSTR___enter__), MP_ROM_PTR(&default___enter___obj) },
	{ MP_ROM_QSTR(MP_QSTR___exit__), MP_ROM_PTR(&default___exit___obj) },
	{ MP_ROM_QSTR(MP_QSTR_address), MP_ROM_PTR(&i2ctarget_i2c_target_request_address_obj) },
	{ MP_ROM_QSTR(MP_QSTR_is_read), MP_ROM_PTR(&i2ctarget_i2c_target_request_is_read_obj) },
	{ MP_ROM_QSTR(MP_QSTR_is_restart), MP_ROM_PTR(&i2ctarget_i2c_target_request_is_restart_obj) },
	{ MP_ROM_QSTR(MP_QSTR_read), MP_ROM_PTR(&i2ctarget_i2c_target_request_read_obj) },
	{ MP_ROM_QSTR(MP_QSTR_write), MP_ROM_PTR(&i2ctarget_i2c_target_request_write_obj) },
	{ MP_ROM_QSTR(MP_QSTR_ack), MP_ROM_PTR(&i2ctarget_i2c_target_request_ack_obj) },
	{ MP_ROM_QSTR(MP_QSTR_close), MP_ROM_PTR(&i2ctarget_i2c_target_request_close_obj) },
	};

[dhalbert]

I think this is the following omission: if you look in the dict_table for, say I2CTarget, you'll see:

    { MP_ROM_QSTR(MP_QSTR_deinit), MP_ROM_PTR(&i2ctarget_i2c_target_deinit_obj) },
    { MP_ROM_QSTR(MP_QSTR___del__), MP_ROM_PTR(&i2ctarget_i2c_target_deinit_obj) },

The presence of the __del__ entry is true for all objects with a finaliser, which should be true for enter/exit objects. In i2ctarget_i2c_target_make_new(), the object created is created with mp_obj_malloc_with_finaliser().

I think the same pattern should be used for I2CTargetRequest: a deinit(), use mp_obj_malloc_with_finaliser(), and add the deinit and ___del__ to the method table.

Is there any example code that uses the close??

[dhalbert]

I looked at the original PR, #1064, which mostly survives as is in the current code. I think the best thing to do here is to remove a user-accessible close() method, add deinit(), and have __exit__() call the common-hal target close() and then do a deinit(). I am working on a PR.

[jbrelwof]

I'm curious - beyond searching the Adafruit and community bundles, is there any standard(-ish) process to search for method usage in a wider body of existing CircuitPython code? Obviously it needs to be something a bit more intelligent than simply grep close ..., but there are a variety of options for finding references to python method use that (try to and usually succeed at least partially) filter on the type, i.e. find invocations of close() on an I2CTargetRequest. But I'm not sure what you would search - searching github for CircuitPython currently returns nearly 4K repos, add MicroPython and you're pushing 17K. Is there any existing list of more widely used (but not in the Adafruit/community bundles) modules for testing / API deprecation checks?

[dhalbert]

I search GitHub in general, both in the Adafruit org and elsewhere, and often do a general websearch, if I can make it specific enough.