-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
RFC: Mechanism for allowing dynamic native modules to call port-specific functionality. #5682
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
Conversation
This allows them to be used by dynamic native modules. For example, to allow access to underlying HAL/RTOS functionality (e.g. ESP32 IDF).
… native module on ESP32.
4a6a729
to
5482680
Compare
Nice! I'm currently adding a generator to produce an espidf module so the functions can be called directly from python. Arguably that can work with any of the three approaches. As far as I can tell:
I find the last approach the cleanest overall, but I don't have a strong preference, I just want to be able to move on and write device drivers in python... |
@tve If I understand your ambitions, it's worth looking at the littlevgl micropython port. @amirgon created a way to bind ESP-IDF API functions to MicroPython. The binding generator is written in Python and is called from the MicroPython build. The result is a new espidf module in MicroPython. Very cool. refer to the section that reads: "Preprocess espidf.h by the C preprocessor. This applies all nested “includes” and produces a single preprocessed file with the entire API we want to generate. Run gen_mpy.py on the preprocessed file. This is where the magic happens, and C functions and structs are transformed into a Micropython module. The output of this step is mp_espidf.c, which is the implementation of the ESP-IDF micropython module."
that's exactly the purpose of his work. |
@miketeachman thanks for the pointer, very interesting! Also very complex! I have to take a closer look at what gets generated. It's not clear to me yet what the desired end-point is. I'd like to have more flexibility writing drivers outside the MP firmware and also about how much code is in python vs. C. |
@tve This is configurable when building Micropython. |
I'm not so sure I fully agree here. You put a single function into your demo table that conveniently does not use any esp-idf type so you didn't need to hunt down any include file. By the time you include functions for a few devices you'll (a) have to change the makefile for the dynamic module to include all the esp-idf include tree, (b) add esp-idf include files into portnativeglue.h, (c) ifdef around any esp-idf v3/v4 incompatibilities in portnativeglue.h, (d) live with the fact that compiling your native module #includes a lot of esp-idf 'cause of portnativeglue.h. Each time you want to add a function to mp_port_tab you also have to hunt down its signature definition in the esp-idf include tree, you can't just go from the manual. The above issues aren't deal breakers, but IMHO they do erode the "simpler" claim and they're the reason I decided to try the jump table, which makes the glue non-typed. |
@armirgon: you are correct, I'll have to try your tool and see how it ends up working in practice. |
@amirgon well, after over an hour of hacking around to get mpy_gen.py working on a cut down espidf.h I finally realized that the code generated only works for a built-in module, not for a dynamically loaded module... |
@amirgon you write:
I'm not sure I buy this, at least given the current script. I did a test just including "driver/pcnt.h" after removing a
|
Yes, I was trying to optimize RAM on expense of Flash. On microcontrollers such as ESP32 there is plenty of Flash that can be mapped to the MCU's address space, so you can place there all program and constants. In such case, the modules generated by
The trick is to prevent nested includes being parsed. As an example, here is an /**
* This file defines the Micorpython API to ESP-IDF
* It is used as input to gen_mpy.py to create a micropython module
**/
#if __has_include("esp_idf_version.h")
# include "esp_idf_version.h"
#endif
// Disable some macros and includes that make pycparser choke
#ifdef PYCPARSER
#define __attribute__(x)
// FreeRTOS portmaco is excluded, but we still need TickType_t
#include <stdint.h>
typedef uint32_t TickType_t;
// Micropython specific types
typedef void *mp_obj_t;
// Prevent pcnt nested includes
#define __ESP_INTR_H__
#define INC_FREERTOS_H
#define SEMAPHORE_H
#define __XTENSA_API_H__
#define _ESP32_SOC_H_
#define _SOC_PCNT_REG_H_
#define _SOC_PCNT_STRUCT_H_
#define _SOC_GPIO_SIG_MAP_H_
#define _DRIVER_GPIO_H_
#define __ESP_INTR_ALLOC_H__
// pcnt.h needs intr_handle_t which is defined in the exluded nested includes.
// Instead, define it here:
typedef void *intr_handle_t;
#else // PYCPARSER
#endif //PYCPARSER
// The following includes are the source of the esp-idf micropython module.
// All included files are API we want to include in the module
#include "driver/pcnt.h" Looking at the resulting /*
* espidf module definitions
*/
STATIC const mp_rom_map_elem_t espidf_globals_table[] = {
{ MP_ROM_QSTR(MP_QSTR___name__), MP_ROM_QSTR(MP_QSTR_espidf) },
{ MP_ROM_QSTR(MP_QSTR_esp_err_to_name), MP_ROM_PTR(&mp_esp_err_to_name_obj) },
{ MP_ROM_QSTR(MP_QSTR_esp_err_to_name_r), MP_ROM_PTR(&mp_esp_err_to_name_r_obj) },
{ MP_ROM_QSTR(MP_QSTR__esp_error_check_failed), MP_ROM_PTR(&mp__esp_error_check_failed_obj) },
{ MP_ROM_QSTR(MP_QSTR__esp_error_check_failed_without_abort), MP_ROM_PTR(&mp__esp_error_check_failed_without_abort_obj) },
{ MP_ROM_QSTR(MP_QSTR_pcnt_unit_config), MP_ROM_PTR(&mp_pcnt_unit_config_obj) },
{ MP_ROM_QSTR(MP_QSTR_pcnt_get_counter_value), MP_ROM_PTR(&mp_pcnt_get_counter_value_obj) },
{ MP_ROM_QSTR(MP_QSTR_pcnt_counter_pause), MP_ROM_PTR(&mp_pcnt_counter_pause_obj) },
{ MP_ROM_QSTR(MP_QSTR_pcnt_counter_resume), MP_ROM_PTR(&mp_pcnt_counter_resume_obj) },
{ MP_ROM_QSTR(MP_QSTR_pcnt_counter_clear), MP_ROM_PTR(&mp_pcnt_counter_clear_obj) },
{ MP_ROM_QSTR(MP_QSTR_pcnt_intr_enable), MP_ROM_PTR(&mp_pcnt_intr_enable_obj) },
{ MP_ROM_QSTR(MP_QSTR_pcnt_intr_disable), MP_ROM_PTR(&mp_pcnt_intr_disable_obj) },
{ MP_ROM_QSTR(MP_QSTR_pcnt_event_enable), MP_ROM_PTR(&mp_pcnt_event_enable_obj) },
{ MP_ROM_QSTR(MP_QSTR_pcnt_event_disable), MP_ROM_PTR(&mp_pcnt_event_disable_obj) },
{ MP_ROM_QSTR(MP_QSTR_pcnt_set_event_value), MP_ROM_PTR(&mp_pcnt_set_event_value_obj) },
{ MP_ROM_QSTR(MP_QSTR_pcnt_get_event_value), MP_ROM_PTR(&mp_pcnt_get_event_value_obj) },
{ MP_ROM_QSTR(MP_QSTR_pcnt_isr_register), MP_ROM_PTR(&mp_pcnt_isr_register_obj) },
{ MP_ROM_QSTR(MP_QSTR_pcnt_set_pin), MP_ROM_PTR(&mp_pcnt_set_pin_obj) },
{ MP_ROM_QSTR(MP_QSTR_pcnt_filter_enable), MP_ROM_PTR(&mp_pcnt_filter_enable_obj) },
{ MP_ROM_QSTR(MP_QSTR_pcnt_filter_disable), MP_ROM_PTR(&mp_pcnt_filter_disable_obj) },
{ MP_ROM_QSTR(MP_QSTR_pcnt_set_filter_value), MP_ROM_PTR(&mp_pcnt_set_filter_value_obj) },
{ MP_ROM_QSTR(MP_QSTR_pcnt_get_filter_value), MP_ROM_PTR(&mp_pcnt_get_filter_value_obj) },
{ MP_ROM_QSTR(MP_QSTR_pcnt_set_mode), MP_ROM_PTR(&mp_pcnt_set_mode_obj) },
{ MP_ROM_QSTR(MP_QSTR_pcnt_isr_handler_add), MP_ROM_PTR(&mp_pcnt_isr_handler_add_obj) },
{ MP_ROM_QSTR(MP_QSTR_pcnt_isr_service_install), MP_ROM_PTR(&mp_pcnt_isr_service_install_obj) },
{ MP_ROM_QSTR(MP_QSTR_pcnt_isr_service_uninstall), MP_ROM_PTR(&mp_pcnt_isr_service_uninstall_obj) },
{ MP_ROM_QSTR(MP_QSTR_pcnt_isr_handler_remove), MP_ROM_PTR(&mp_pcnt_isr_handler_remove_obj) },
{ MP_ROM_QSTR(MP_QSTR_PCNT_MODE), MP_ROM_PTR(&mp_PCNT_MODE_type) },
{ MP_ROM_QSTR(MP_QSTR_PCNT_COUNT), MP_ROM_PTR(&mp_PCNT_COUNT_type) },
{ MP_ROM_QSTR(MP_QSTR_PCNT_UNIT), MP_ROM_PTR(&mp_PCNT_UNIT_type) },
{ MP_ROM_QSTR(MP_QSTR_PCNT_CHANNEL), MP_ROM_PTR(&mp_PCNT_CHANNEL_type) },
{ MP_ROM_QSTR(MP_QSTR_PCNT_EVT), MP_ROM_PTR(&mp_PCNT_EVT_type) },
{ MP_ROM_QSTR(MP_QSTR_pcnt_config_t), MP_ROM_PTR(&mp_pcnt_config_t_type) },
{ MP_ROM_QSTR(MP_QSTR_C_Pointer), MP_ROM_PTR(&mp_C_Pointer_type) },
};
STATIC MP_DEFINE_CONST_DICT (
mp_module_espidf_globals,
espidf_globals_table
);
const mp_obj_module_t mp_module_espidf = {
.base = { &mp_type_module },
.globals = (mp_obj_dict_t*)&mp_module_espidf_globals
}; If this is not concise enough for you (or if you don't like using the include guard trick), another option (as I mentioned earlier) is to create some wrapper functions only around the functionality you are interested in and process these wrapper functions by the the script. |
Thanks for the tip, I'll try that and see whether I can hack the codegen to support dyn loaded modules. I understand what you were trying to do, but here we're trying to enable external drivers... |
But so far there was no demand and the only project that uses it is lvgl, so let's first see if some other project needs it.
Theoretically you could load an external mpy to flash on runtime and execute it from there. I think it's important especially for large modules where we don't want to waste a lot of RAM. |
A speed bump I just ran into is that I called |
@amirgon FYI, I tried to add support for generating dynamic modules to gen_mpy but gave up for now... too many things I don't understand. |
@tve - There is something I don't understand. If you only want to dynamically load the drivers, and you assume that esp-idf itself is statically linked, what is then your motivation to dynamically load the python wrappers for esp-idf? Why not have them statically linked together with esp-idf? |
Thanks for asking! :-) I wish there was a place to have these discussions other than hijacking someone else's PR and that the master who decides everything in the end would chime in, but here we are... My goal is to free up writing drivers and other add-ons to the esp32 port (and other ports) outside of the MP repo. As far as I can tell, a majority of PRs for the esp32 only have to be built into the firmware image because of the linking issue. Take my machine.Counter PR #5216 as an example: it only has to be in the firmware 'cause of linking against esp-idf. But it's stuck in a "I'm not sure how many other people want this" state. I don't need to make it part of MP, I just need to link against esp-idf. A number of other PRs are stuck in a similar state, e.g. "I know it's not general/portable/to-your-liking but it works for me" or "I did the best I can" or "no-one has time to really review and merge this", etc. So what I'm hoping we can get done is to add a linking method, such that when someone comes around and produces a PR to access functions X, Y, and Z then that just adds a couple of words of flash and is a no-brainer to merge. Ideally everything else they need to do stays outside of MP. Now comes the question whether this "everything else" benefits from minimal stubs to make the esp-idf functions callable from python directly. You have more experience than I do, but it seems worthwhile 'cause a lot of stuff can be written in python as far as I can tell so far. I wrote a poor-man's stub generator, you wrote the cadillac version. Either way, if my app has 2-3 drivers that need access to a handful of ESP-IDF functions each, then either stub generator can be used to produce the necessary stubs whether they end up in RAM or Flash. I don't think it makes much of a difference (I assume the app itself is much larger than 20-odd stubs). I gather that you like the idea of generating stubs for "all of esp-idf" once and for all, and bake those into the MP firmware. That probably would be nice. The concerns I have are: (1) what does that actually look like size-wise, (2) how do you even define and construct "all of esp-idf" (may be best to parse the docs and collect the set of functions described there?), (3) who signs up to maintain that (which now includes WRT Flash vs. RAM my opinion would be to work on allowing dynamically loaded modules to be executed from flash. Damien has indicated that he's open to that. I'm not sure what exactly would be involved but it doesn't seem that difficult. Overall, I'm not trying to push my solution. I'm not even sure I know yet what my solution would be! All I know is that I would much like a common solution that allows all of us to publish device drivers and other cool libraries that use ESP-IDF in a way that lets others upip install them without having to build their own version of the firmware. I can easily maintain my own fork of MP, but that's akin to taking my marbles and playing alone in my corner... |
Sorry to hijack this thread, but I keep on wondering: ideally the part of having to explicitly allow access to X/Y/Z gets skipped as well, no? Which should be possible somehow because after all X/Y/Z are already linked into the frimware so the only problem is getting their address out at runtime and passing that to the module. I don't have a CS degree but what I'm describing here is essentially what dynamic loading with ELF format does, I think. And I've done that (loading .so files as modules) on the unix port and it all just works. So can someone enlighten me: why is it not a viable solution of attempting like that instead of having a static table? Is it just because of the code size required to implement or is there more? Too hard/too much work perhaps? |
Perhaps the missing FFI support for ESP32? |
Fundamentally somewhere somehow a symbol name to address lookup needs to be made so the dynamically loaded module can be fixed-up with the correct addresses. There are 3 options that are relevant here:
The difference between this PR and my PR is whether the dynamic module gets fixed-up the way most dynamic linkers work by patching the code, or whether the code in the dynamic module does the look-up explicitly. So both PRs assume option 3. Something worth exploring: what if we put substantially all external esp-idf symbols into the mp_port_fun_table? Or all symbols occurring in the docs? |
Thanks for the explanation. So looks like main reason for not having full-blown loading (2) is code size right? Would there be a way to use (2) but at build-time have a way of telling 'don't create complete table but only put these symbols in it'? Makes code size less of a problem for those who want it. But in essence it's the same problem as getting all functions for (3) just the other way around: (3) needs a custom way to tell 'I want X/Y/Z' and (2) needs a custom way to tell 'I do not want X/Y/Z'. But I guess (2) makes it easier to just have everything work by default if code size doesn't matter. |
@stinos the issue with (2) is that the table is in the firmware and most users (I assume) install the released firmware so there's no opportunity for them to say "I do not want X/Y/Z". |
Unless it's not built int the firmware but as a bunch of separate files which get loaded on demand. Which might actually reduce code size. But has some management overhead. And might not be possible on all ports, not sure. |
I tried to generate an mp_port_fun_table for all the ESP-IDF functions that are described in the docs. Getting the list of function names isn't that difficult, it turns out. However... drumroll... if I create a table that references all of them then the code for all the functions gets linked into the executable and that causes |
Part of the discussion here is about the distinction between a static In the end, even if an |
This is an alternative to #5653 by @tve
The main benefit of this approach is that it's extremely simple and requires no modifications to the linker/mpy machinery. Maintaining the table is obvious and straightforward and a logical extension of the existing mp_fun_table.
Main downside is that it calls to port-specific functions involve an indirection but I'm not sure this is too serious.
I'm not too fond of "portnativeglue.{c,h}" but mostly this is a PoC for discussion.
Includes a modification to natmod/features0 to demonstrate using it on ESP32.