Skip to content

RFC: enabling external drivers #5618

Description

@tve

I've been reviewing a bunch of older esp32 pull requests and am a little puzzled. A bunch fall into the category of essentially exposing ESP-IDF functionality. For example, I2S #4471, smartconfig #4404, machine.Counter #5496, adjtime #5453, sigma-delta #5452, probably CAN #5310. I assume other ports have the same situation, e.g. with the STM32 HAL.

I'm wondering whether there is a way to enable these types of drivers without having to pull them into the MP core given that the PRs are not exactly moving very fast and are often difficult to review and to reach agreement upon. With the new ability to put C code into loadable MPY modules the main thing that seems to be missing is the ability to link.

I'm wondering whether an initial approach where PRs that only add functions to the mp_fun_table are readily accepted such that the drivers can be implemented in external repos could work. (The new functions could go into a specially marked section such that they could be replaced by a more sophisticated mechanism later.) Thoughts?

Pros:

  • reduces PR bottleneck
  • allows multiple driver models to co-exist, e.g. evolution of more sophisticated APIs
  • allows machine-dependent drivers without "polluting" MP core repo

Cons:

  • makes it harder to locate code, e.g. libraries and drivers scattered all around github
  • may make higher-level libraries incompatible, e.g., lib A uses driver A while lib B uses driver B for the same peripheral
  • reduces ability to create portable drivers by scattering code
  • does it make debugging the native code harder?

See also #5463 (preliminary PR for docs on how to add functions to table)

Activity

  1. MrSurly commented on Feb 7, 2020

    @MrSurly
    Contributor

    I'm wondering whether there is a way to enable these types of drivers without having to pull them into the MP core given that the PRs are not exactly moving very fast and are often difficult to review and to reach agreement upon.

    You can merge them yourself and use them if desired. I did this for a project that uses the I2S functionality.

  2. jcw commented on Feb 13, 2020

    @jcw

    If µPy is serious about separately loadable/linkable extensions (which I assume - and hope - it is), then the µPy <=> extension "barrier" will need to be very carefully managed and grown: each call (in either direction) will be nearly impossible to remove or even alter once it's made available. I'm not saying anything particularly new here - just wanted to stress that there's a potentially brittle minefield ahead IMO. Perhaps µPy can avoid exposing many of its internals by offering just a script callback interface for extensions which need to query or adjust the Python context they're being called in. This might not be the most efficient approach, but it allows postponing many API decisions to when more extensions have been built, and experience gained about which parts need to be optimised.

    (in case none of this makes sense: please ignore me - I've only recently started following µPy ...)

  3. tve commented on Feb 13, 2020

    @tve
    ContributorAuthor

    @jcw: agreed. But I believe that what you write applies mainly to exporting calls "into" MP, i.e., exposing more of the MP innards. I believe/hope that many drivers can be written that don't need anything new, other than perhaps some types that have been skipped (e.g. #5641). Ideally the only calls that get added are ones for the underlying HAL-level abstractions, such as FreeRTOS, STM32 HAL, ESP-IDF.

  4. jeremyherbert commented on Feb 14, 2020

    @jeremyherbert
    Contributor

    With the current setup, I’m pretty sure if mp_fun_table changes then all existing compiled native modules will cease to work and need to be recompiled. So this makes accepting PRs to modify the table quite tricky I think.

  5. tve commented on Feb 14, 2020

    @tve
    ContributorAuthor

    Can you be more specific? Why would existing compiled modules break if the table gets extended?

  6. jeremyherbert commented on Feb 14, 2020

    @jeremyherbert
    Contributor

    I guess it depends on what the solution is to my problem in #5641 ;)

    Also see mp_ld.py, line 666, the “67 +” bit; that appears to me that symbol offsets are hard coded into the linker.

  7. jeremyherbert commented on Feb 14, 2020

    @jeremyherbert
    Contributor

    There is also the problem of what if a newer compiled module depends on some symbols which are missing because the interpreter has the old table. It makes micropython versioning much more important than it is now as you basically need to lock down an ABI as version X, and then check if the module is compatible with version X.

  8. tve commented on Feb 14, 2020

    @tve
    ContributorAuthor

    I did a little test trying to convert my machine.Counter PR #5216 to a native module. Not much joy...

    The "little test" took a whole bunch of hours. I had to add the following functions to the mp_fun_table:

    • mp_arg_parse_all
    • mp_obj_new_exception_msg_varg
    • machine_pin_get_id

    the last one being an issue/hack 'cause it's defined in the port, not in py.

    Having to define the locals_dict_table for the module dynamically isn't pretty or convenient, and having to init the type object dynamically isn't nice either. The "allowed_args" table also has to be inited dynamically.

    I still didn't get everything right, mpy_ld dies with:

      File "/home/src/esp32/micropython/tools/mpy_ld.py", line 871, in build_mpy
        kind = 7 + kind
    TypeError: unsupported operand type(s) for +: 'int' and 'str'
    

    I didn't not attempt to troubleshoot that 'cause it's bedtime and the whole process so far has been rather painful...

  9. jeremyherbert commented on Feb 14, 2020

    @jeremyherbert
    Contributor

    yep, it's a bit tricky at the moment. See also here for some points from Damien: #5643

  10. tve commented on Feb 14, 2020

    @tve
    ContributorAuthor

    Yup, I'm following that PR too... I'm gonna give it one more try, this time I'll only write the ESP-IDF calls in C. See how that works out.

  11. added a commit that references this issue on Nov 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions