pybricks
[Feature] Support external package imports
Describe the bug We may want to eventually include certain frozen packages in some firmwares. For example, asyncio may be useful for working with rfcomm sockets.
However, there are certain issues with pb_builtin_import that prevent this from working. First, it explicitly does not handle the MP_IMPORT_STAT_DIR return code, which indicates that the imported name is a directory in the frozen content.
Second, it appends ".py" to the imported name. This seems to work okay for names that are individual python files. But, it does not work for package names, like asyncio. The package is represented in the mp_frozen_names list as "asyncio/__init__.py\0". Appending ".py" to the asyncio name causes a mismatch with the path separator.
And yet, if we didn't add the .py suffix, then we wouldn't be able to match top-level files like _builtin_port_view.py.
One thing I do notice: the comment above this function says, "// IMPORTANT: this needs to be kept in sync with mp_builtin___import___default().", and, looking at mp_builtin__import_default(), it does appear that that function has moved on significantly from the last time we took a snapshot of it.
Not sure what to do about this right now -- probably just get by without asyncio! -- but it might be worth looking at at some point.
Can you give an example of where new packages are needed that can't be done with what we have? See also https://github.com/pybricks/support/issues/2444.
One thing I do notice: the comment above this function says, "// IMPORTANT: this needs to be kept in sync with mp_builtin___import___default().", and, looking at mp_builtin__import_default(), it does appear that that function has moved on significantly from the last time we took a snapshot of it.
They are identical, except that we added support for our import. See below. Maybe you are asking to enable MICROPY_ENABLE_EXTERNAL_IMPORT? That could be a consideration once we have a proper file system at some point.
diff --git a/py/builtinimport.c b/py/builtinimport.c
index 0611926fd..cbb03fd2f 100644
--- a/py/builtinimport.c
+++ b/py/builtinimport.c
@@ -646,12 +646,49 @@ mp_obj_t mp_builtin___import___default(size_t n_args, const mp_obj_t *args) {
if (module_obj != MP_OBJ_NULL) {
return module_obj;
}
+
// Now try as an extensible built-in (e.g. `time`).
module_obj = mp_module_get_builtin(module_name_qstr, true);
if (module_obj != MP_OBJ_NULL) {
return module_obj;
}
+ // Check for presence of user program in user RAM.
+ mpy_info_t *info = mpy_data_find(module_name_qstr);
+
+ // If a downloaded module was found but not yet loaded, load it.
+ if (info) {
+ // Create new module.
+ mp_module_context_t *module_context = mp_obj_new_module(module_name_qstr);
+
+ // Execute the module in that context.
+ execute_rom_mpy_in_context(module_context, info);
+
+ // Return the newly imported module.
+ return MP_OBJ_FROM_PTR(module_context);
+ }
+
+ // Allow importing of frozen modules if any were included in the firmware.
+ #if MICROPY_MODULE_FROZEN_MPY
+ void *modref;
+ int frozen_type;
+ const char *ext = ".py";
+ char module_path[(1 << (8 * MICROPY_QSTR_BYTES_IN_LEN)) + sizeof(ext)] = { 0 };
+ strcpy(module_path, mp_obj_str_get_str(args[0]));
+ strcpy(module_path + qstr_len(module_name_qstr), ext);
+ if (mp_find_frozen_module(module_path, &frozen_type, &modref) == MP_IMPORT_STAT_FILE && frozen_type == MP_FROZEN_MPY) {
+ // Create new module to be returned.
+ mp_module_context_t *module_context = mp_obj_new_module(module_name_qstr);
+ mp_obj_t module_obj = MP_OBJ_FROM_PTR(module_context);
+
+ // Execute frozen code in the new module context.
+ const mp_frozen_module_t *frozen = modref;
+ module_context->constants = frozen->constants;
+ do_execute_proto_fun(module_context, frozen->proto_fun);
+ return module_obj;
+ }
+ #endif
+
// Couldn't find the module, so fail
#if MICROPY_ERROR_REPORTING <= MICROPY_ERROR_REPORTING_TERSE
mp_raise_msg(&mp_type_ImportError, MP_ERROR_TEXT("module not found"));
I didn’t notice there were two instances of that function. Thanks! And now the “external” makes a little more sense since the asyncio package is in “extmod”.
I will try the async approach mentioned in the other issue. Should work okay.
To answer the question of what I was trying to do with it. I've implemented an rfcomm socket API using the stream protocol in Micropython.
mp_stream_p_t pb_type_rfcomm_socket_stream_p = {
.read = pb_type_rfcomm_socket_read,
.write = pb_type_rfcomm_socket_write,
.ioctl = pb_type_rfcomm_socket_ioctl,
};
With the rest of the firmware as it is now, you have to use it something like this:
from pybricks.tools import run_task, wait
from pybricks.experimental.btc import scan, rfcomm_connect, rfcomm_listen
async def do_ping_pong():
print("Start bluetooth connection...")
sock = await rfcomm_connect("5C:F3:70:96:8F:AA", timeout=10000)
try:
print("Connected to: ", sock)
buf = bytearray("ping\n", "ascii")
while buf:
n = sock.write(buf)
buf = buf[n:]
print("Sent ping")
buf = bytearray(10)
while buf[4] != ord("\n"):
sock.readinto(buf)
print(buf)
finally:
sock.close()
run_task(do_ping_pong())
This does work. However, it feels not ideal. With asyncio's stream, it would look more like this:
async def do_ping_pong():
print("Start bluetooth connection...")
sock = await rfcomm_connect("5C:F3:70:96:8F:AA", timeout=10000)
try:
print("Connected to: ", sock)
sock = Stream(sock)
sock.write(bytearray("ping\n", "ascii"))
await sock.drain()
line = await sock.readline()
print(line)
finally:
sock.close()
Not the end of the world and as you point out I can just poll the socket in micropython with short waits between to get a similar effect. I wonder if the asyncio stuff might be more efficient -- presumably those guys implemented it for some reason or another. But for now I am happy enough with the solution you proposed.
We could also just write the async versions of these functions in C and indeed I may do that.
Thanks for the update. I think we can go with the approach in #2274 so we can just await the write operation to keep things consistent for the user.