Skip to content

i2ctarget error with missing deinit for request #10362

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Neradoc opened this issue May 19, 2025 · 4 comments · Fixed by #10366
Closed

i2ctarget error with missing deinit for request #10362

Neradoc opened this issue May 19, 2025 · 4 comments · Fixed by #10366
Assignees
Labels
bug regression Things that used to work but don't any longer
Milestone

Comments

@Neradoc
Copy link

Neradoc commented May 19, 2025

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 ?

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

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) },
};

@Neradoc Neradoc added bug regression Things that used to work but don't any longer labels May 19, 2025
@dhalbert
Copy link
Collaborator

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 dhalbert added this to the 10.0.0 milestone May 20, 2025
@dhalbert
Copy link
Collaborator

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.

@dhalbert dhalbert self-assigned this May 20, 2025
@jbrelwof
Copy link

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
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug regression Things that used to work but don't any longer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants