Skip to content
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

[Feature] Restore frozen directory to include frozen modules #829

Closed
laurensvalk opened this issue Dec 6, 2022 · 15 comments
Closed

[Feature] Restore frozen directory to include frozen modules #829

laurensvalk opened this issue Dec 6, 2022 · 15 comments
Labels
enhancement New feature or request

Comments

@laurensvalk
Copy link
Member

We used to have a folder called "frozen" in the each of the bricks/ folders, and something in the Makefile that would freeze all of the contents into the firmware. We can probably restore that. It shouldn't take any space for anyone who isn't using that feature.

Originally posted by @laurensvalk in #828 (reply in thread)

@laurensvalk
Copy link
Member Author

laurensvalk commented Dec 6, 2022

This could be useful for large modules such as those made by @Novakasa in https://github.com/orgs/pybricks/discussions/828#discussioncomment-4322426.

@laurensvalk laurensvalk added the enhancement New feature or request label Dec 6, 2022
@laurensvalk
Copy link
Member Author

This is a first step:

diff --git a/bricks/_common_stm32/make.mk b/bricks/_common_stm32/make.mk
index b9d8e32e..9c12a968 100644
--- a/bricks/_common_stm32/make.mk
+++ b/bricks/_common_stm32/make.mk
@@ -50,6 +50,9 @@ USER_C_MODULES = $(PBTOP)
 
 include ../../micropython/py/mkenv.mk
 
+# Frozen Python code
+FROZEN_MANIFEST ?= ../_common_stm32/manifest.py
+
 # qstr definitions (must come before including py.mk)
 QSTR_DEFS = ../_common/qstrdefs.h
 QSTR_GLOBAL_DEPENDENCIES = ../_common/mpconfigport.h ../_common_stm32/mpconfigport.h
@@ -353,6 +356,17 @@ SRC_QSTR += $(PY_EXTRA_SRC_C) $(PY_STM32_SRC_C) $(PYBRICKS_PYBRICKS_SRC_C)
 # Append any auto-generated sources that are needed by sources listed in SRC_QSTR
 SRC_QSTR_AUTO_DEPS +=
 
+ifneq ($(FROZEN_MANIFEST),)
+CFLAGS += -DMICROPY_QSTR_EXTRA_POOL=mp_qstr_frozen_const_pool
+CFLAGS += -DMICROPY_MODULE_FROZEN_MPY
+CFLAGS += -DMPZ_DIG_SIZE=16
+MPY_TOOL_FLAGS += -mlongint-impl none
+endif
+
+ifneq ($(FROZEN_MANIFEST),)
+CFLAGS += -DMICROPY_MODULE_FROZEN_STR
+endif
+
 # Main firmware build targets
 TARGETS := $(BUILD)/firmware.zip

But we'd also need some work to our import function to facilitate this.

@Novakasa
Copy link

Novakasa commented Jan 13, 2023

I actually tried something very similar to this yesterday using cygwin on windows:
https://github.com/Novakasa/pybricks-micropython/tree/restore-frozen

When trying to compile a frozen module, I get the error:
image

And when no modules are present (e.g. no lines in manifest.py or no .py in frozen modules folder:
image

Is that what you mean be some work left on the import function or am I seeing a different issue?

@laurensvalk
Copy link
Member Author

There's a bit more to it. Trying now, hopefully will report back soon :)

@laurensvalk
Copy link
Member Author

I didn't finish the import part yet, but the following might be good enough for you. It will just always run bricks/_common_stm32/modules/my_code.py just before your script starts.

Add these files:

bricks/_common_stm32/manifest.py

freeze_as_mpy("modules")

bricks/_common_stm32/modules/my_code.py

print("hello, data!")
data = 123

And make these changes:

diff --git a/bricks/_common_stm32/make.mk b/bricks/_common_stm32/make.mk
index b9d8e32e..9c12a968 100644
--- a/bricks/_common_stm32/make.mk
+++ b/bricks/_common_stm32/make.mk
@@ -50,6 +50,9 @@ USER_C_MODULES = $(PBTOP)
 
 include ../../micropython/py/mkenv.mk
 
+# Frozen Python code
+FROZEN_MANIFEST ?= ../_common_stm32/manifest.py
+
 # qstr definitions (must come before including py.mk)
 QSTR_DEFS = ../_common/qstrdefs.h
 QSTR_GLOBAL_DEPENDENCIES = ../_common/mpconfigport.h ../_common_stm32/mpconfigport.h
@@ -353,6 +356,17 @@ SRC_QSTR += $(PY_EXTRA_SRC_C) $(PY_STM32_SRC_C) $(PYBRICKS_PYBRICKS_SRC_C)
 # Append any auto-generated sources that are needed by sources listed in SRC_QSTR
 SRC_QSTR_AUTO_DEPS +=
 
+ifneq ($(FROZEN_MANIFEST),)
+CFLAGS += -DMICROPY_QSTR_EXTRA_POOL=mp_qstr_frozen_const_pool
+CFLAGS += -DMICROPY_MODULE_FROZEN_MPY
+CFLAGS += -DMPZ_DIG_SIZE=16
+MPY_TOOL_FLAGS += -mlongint-impl none
+endif
+
+ifneq ($(FROZEN_MANIFEST),)
+CFLAGS += -DMICROPY_MODULE_FROZEN_STR
+endif
+
 # Main firmware build targets
 TARGETS := $(BUILD)/firmware.zip
 
diff --git a/pybricks/pybricks.c b/pybricks/pybricks.c
index 8bf141aa..50bf0980 100644
--- a/pybricks/pybricks.c
+++ b/pybricks/pybricks.c
@@ -2,6 +2,7 @@
 // Copyright (c) 2018-2022 The Pybricks Authors
 
 #include "py/mpconfig.h"
+#include "shared/runtime/pyexec.h"
 #include "py/obj.h"
 #include "py/objmodule.h"
 #include "py/objstr.h"
@@ -122,6 +123,7 @@ void pb_package_pybricks_init(bool import_all) {
         if (import_all) {
             pb_package_import_all();
         }
+        pyexec_file_if_exists("my_code.py");
         nlr_pop();
     } else {
         // Print initialization or import exception.

@Novakasa
Copy link

Novakasa commented Jan 13, 2023

Thanks for this! Unfortunately this still seems to have the same problem as I had yesterday:
error compiling <my file>
OSError: 2
I have done a clean compilation, but maybe something in my environment is outdated or something.

Novakasa added a commit to Novakasa/pybricks-micropython that referenced this issue Jan 13, 2023
Novakasa added a commit to Novakasa/pybricks-micropython that referenced this issue Jan 13, 2023
@laurensvalk
Copy link
Member Author

Here is a ready made version that you can try: https://github.com/pybricks/pybricks-micropython/commits/frozencode

Importing also works now. You should be able to put any number of files in the modules folder. There is one example to start with.

I'll clean it up to make it properly optional later.

@laurensvalk
Copy link
Member Author

I've tested this on Prime Hub only, but it should also work on City Hub. It builds fine for both at least.

@Novakasa
Copy link

Novakasa commented Jan 13, 2023

Yeah unfortunately I still get this OSError: 2. Master does compile fine though. I'll see if I can setup my environment again or test on ubuntu these next few days. Thanks for working on this!!

@laurensvalk
Copy link
Member Author

laurensvalk commented Jan 13, 2023

Your CI seems to be producing artifacts, so you could take a copy of my branch, add some of your modules to the modules folder, and download the firmware from your CI build. E.g. https://github.com/Novakasa/pybricks-micropython/actions/runs/3914767544

@laurensvalk
Copy link
Member Author

This is done now. See usage instructions in https://github.com/pybricks/pybricks-micropython/tree/master/bricks/_common/modules.

If you want to use CI approach @Novakasa, make sure to add exceptions for your files to .gitignore to e.g. change it to:

*.py
!test.py

@laurensvalk
Copy link
Member Author

It shouldn't take any space for anyone who isn't using that feature.

This is achieved by enabling the extra import code only if frozen modules are present.

@Novakasa
Copy link

image
works using the CI builds 👍

@laurensvalk
Copy link
Member Author

Yes! Now, how many lines of code will you insert, and how much download-and-run time does this save you? 😄

@Novakasa
Copy link

Novakasa commented Jan 14, 2023

I'm inserting 190 lines for my communication module, and my train program download goes from ~5kB to ~3kB. I'll keep experimenting on how much more complexity I am now able to handle in my train program, which could make some exciting things possible!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants