-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
Enable more features in the embed port #11430
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
base: master
Are you sure you want to change the base?
Conversation
Code size report:
|
9751e6c
to
0eb84d3
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #11430 +/- ##
=======================================
Coverage 98.39% 98.39%
=======================================
Files 161 161
Lines 21204 21204
=======================================
Hits 20864 20864
Misses 340 340 ☔ View full report in Codecov by Sentry. |
Okay, CI has completed successfully (including the new Check examples / embedding-full job), so no more force pushes for a bit. |
@cwalther Thank you this will be really helpful! Thanks for the really clean PR and separate commits. Only one minor comment (maybe more for @dpgeorge) -- I was hoping that as we added more embedding examples they would be much more narrow one-example-per-specific-feature. (This is something that annoys me in other SDKs, when I want to do only X but the nearest example does X+Y+Z and I have to figure out which bits are relevant to me). That said, I don't think there's anything particularly wrong with having a "kitchen sink" example either. |
I have no particular opinion regarding single-feature vs. kitchen-sink examples, although at some point the number of examples may become unwieldy or a maintenance burden. For now, as an improvement over the current state, I considered it good enough to have a “minimal” and a “maximal” example and leave it to users to interpolate between them according to their use case. But more could be added later. I was also hoping that making additions one by one in separate commits would make it easier for a user to keep them apart – if it occurs to them to check the Git history. But of course this property will fall apart over time as other changes are made. By the way, if you’re interested in my application, which is getting MicroPython on Playdate, I talked about it here a few days ago: https://youtu.be/uArei1EjJnM?t=1872. |
Very cool @cwalther; my Playdate will be arriving soon and I have always thought it'd be a great MicroPython device! However I had envisioned MicroPython running on bare metal, rather than within the Playdate environment - embedding it is an interesting idea! I'll be following closely... 😄 |
I'm keen to try and find some time to progress this soon. Sorry about the delay @cwalther So it doesn't get lost, made a comment on a discussion thread that is relevant to this PR: https://github.com/orgs/micropython/discussions/11616#discussioncomment-6007798 |
I have rebased the current work on master, which required a few adjustments (mostly for the |
I’m not sure what’s up with the intermittently failing unix port / coverage_32bit / thread_exc1.py, it seems to have absolutely nothing to do with my changes. |
@cwalther I've created a thread in the developer channel in the Playdate discord to discuss the possibility of MicroPython on Playdate; thought you might be interested 😄 |
Thanks for the notice, I’ll take a look! I haven’t been on the Playdate Discord so far (only the forum, and haven’t looked at that either for a while). By the way, I am still working on this as time permits, currently fixing some bugs in the Posix VFS. |
This is an automated heads-up that we've just merged a Pull Request See #13763 A search suggests this PR might apply the STATIC macro to some C code. If it Although this is an automated message, feel free to @-reply to me directly if |
Hi @cwalther,
Running make submodules returns:
|
I suspect the I guess |
Here come the filesystems: littlefs and POSIX. Littlefs complicates things a bit, for two reasons:
I am also hitting the limitations of the kitchen-sink example now: There are two mutually exclusive implementations of Also rebased over the Next steps: I think I now have the main features I need for my Playdate project, so I will now try this in an actual out-of-tree build in a Playdate project and see how it holds up. |
Running the embedding-full example (minus POSIX VFS) from micropython/micropython#11430 and a rudimentary REPL.
Rebased on #14344 which fixes an issue I ran into with the Playdate project. Things are looking pretty good on the Playdate front, the embedding-full example code from here (minus POSIX VFS) as well as a rudimentary REPL are running. Still lots of glue to write to hook things up to the Playdate environment. |
When MicroPython is used as a submodule and built from the containing project, e.g. for the embed port, `make submodules` fails because it goes looking for the sub-sub-module paths in the outer repository instead of in the micropython repository. Fix this by invoking git inside the micropython submodule. Signed-off-by: Christian Walther <cwalther@gmx.ch>
It compiles and runs in this state, but a lot of functionality is still missing, to be extended over the following commits. Signed-off-by: Christian Walther <cwalther@gmx.ch>
The implementation of mp_hal_stdout_tx_strn_cooked() does not belong into the library, but into the embedding application. Some applications may not want Python output to go straight to their stdout, or may not even have printf() available. Signed-off-by: Christian Walther <cwalther@gmx.ch>
To include extmod and its dependencies in the output, include the word "extmod" in make variable EMBED_EXTRA when building the port. Ideally this would be deduced automatically from the set of modules enabled in mpconfigport.h (in a more fine-grained way too), but I can't think of a simple way of achieving that. Change in mphalport.h: fixes compiler error "type name requires a specifier or qualifier" in machine_i2c.h, included from machine_i2c.c. Signed-off-by: Christian Walther <cwalther@gmx.ch>
Use FROZEN_MANIFEST as documented in docs/reference/manifest.rst. Signed-off-by: Christian Walther <cwalther@gmx.ch>
This works out of the box in the embed port. Signed-off-by: Christian Walther <cwalther@gmx.ch>
Signed-off-by: Christian Walther <cwalther@gmx.ch>
Signed-off-by: Christian Walther <cwalther@gmx.ch>
Optionally adds gmtime, localtime, mktime, time, time_ns to the time module, implemented using mp_hal_time_ns(). This could also be used by other ports. I'm unsure where to put modtime_mphal.h, it could also be in extmod. The important thing is that for MICROPY_PY_TIME_INCLUDEFILE to work it must be at the same path in both the port build (original source tree) and the application build (micropython_embed distribution), therefore not in ports/embed/port. It is named .h, mismatching the corresponding ports/*/modtime.c, because it must not be compiled separately, which naming it .c would make harder for users of the embed port - they would need to explicitly exclude it, whereas this way they can continue to just compile all the .c files found in the micropython_embed distribution except those in lib. Signed-off-by: Christian Walther <cwalther@gmx.ch>
Signed-off-by: Christian Walther <cwalther@gmx.ch>
Signed-off-by: Christian Walther <cwalther@gmx.ch>
To make use of it, as demonstrated in the embedding-full example: - Include the word(s) "littlefs1" and/or "littlefs2" in EMBED_EXTRA in micropython_embed.mk. - Enable the desired MICROPY_*VFS* features in mpconfigport.h. - Add include path "$(EMBED_DIR)/lib/littlefs" to your application build system. For demonstration, a block device implementation containing a small read-only littlefs2 image is added to the embedding-full example. The block device could have been written in C and interface to some external, possibly writable storage medium, but was written in Python and frozen for simplicity. What makes this a bit awkward is the fact that the littlefs sources are located in lib/ and do need to be compiled into the application, unlike all previous .c files from lib/ that are #included by other C files and must not be compiled separately. Therefore, introduce a new directory 'libsrc' in the embed build output that contains these files and can be added to the application build system as usual, while 'lib' can remain excluded. The headers need to stay in 'lib' because vfs_lfs.c refers to them under that path. I am also not entirely happy about having to add an additional include path to the application build, but see no way around that as long as vfs_lfs.c refers to "lib/littlefs/lfs2.h" while lfs2.c refers to "lfs2.h". Signed-off-by: Christian Walther <cwalther@gmx.ch>
In the embedding-full example, that requires disabling mphal-backed stdio again as it conflicts with the implementation brought in by vfs_posix_file.c. Signed-off-by: Christian Walther <cwalther@gmx.ch>
When `make -f micropython_embed.mk` was re-run, e.g. after modifying a frozen module, all .c and .h files in the output would get new modification dates, causing the next build of the embedding project to unnecessarily recompile everything. Avoid this by using `cp -p` to preserve the original modification date from the source tree while copying files. Signed-off-by: Christian Walther <cwalther@gmx.ch>
It continues to hold up well in the Playdate project, except for one annoyance: When Also rebased on v1.23.0 (not on master just for the nice version number). |
(Still on draft pull request micropython/micropython#11430.)
Hi @cwalther - this looks like good work. Is there any particular reason this is marked as "Draft"? Or is it ready for review now? |
It is marked as “Draft” because it is not ready for merging. Whether you want to review it already is up to you. Off the top of my head, what’s required to make it ready is at least to split the conflicting filesystem examples into two side-by-side example projects instead of having one commit undo a previous one. Maybe more splits make sense, as originally desired by jimmo. The project has been on the back burner for a while as other stuff competes for my attention, but I do intend to continue eventually. |
In preparation for using the embed port in a project, I am trying to enable more features in it than the minimalistic
examples/embedding
demonstrates. I am making good progress, however for some features I was unable to achieve my goal completely out-of-tree and required patches to the MicroPython source tree itself. This PR is my attempt to get these patches upstream.I am not done yet with enabling more features (next on the list is a filesystem), hence the draft status, but I still wanted to publish my work in progress for public scrutiny, in case people more familiar with MicroPython internals than me have better ideas on how to achieve my goals. Accordingly, in the short term, I plan to update this branch by force push occasionally, to incorporate new insights or to rebase on current master. Once I consider it done, I will remove the draft status.