Improved program download, multi-file projects, and persistent storage on Powered Up hubs #106
Conversation
laurensvalk
added
the
software: pybricks-micropython
There was a problem hiding this comment.
Nice work. I made a bunch of comments about all of the little details, but overall it looks like a nice improvement. The user program memory allocation code is a bit hard to understand so I would need more time to really go through it to understand it enough to be able to spot any potential problems or offer suggestions for improvements there. Likewise, the startup and shutdown sequencing really needs some more thought to ensure correctness.
| @@ -4,3 +4,5 @@ | |||
| #define PBSYS_CONFIG_BLUETOOTH (1) | |||
| #define PBSYS_CONFIG_HUB_LIGHT_MATRIX (1) | |||
| #define PBSYS_CONFIG_STATUS_LIGHT (1) | |||
| #define PBSYS_CONFIG_PROGRAM_LOAD (0) | |||
There was a problem hiding this comment.
It would be nice to test this code.
|
Thanks for the detailed review! I'll try going through all of them by the end of the week. I've just added the final feature for this branch, so now it is time to begin cleaning it all up. |
|
I've just added the final major feature of this branch. This adds support for multi-file projects! A script can now import a module from the same directory. This allows users to manage and re-use frequently used code snippets and classes. An example implementation for On the hub itself, they are treated as real modules, so import statements work without special behaviors. The exact format of the bundled code is up for discussion. The currently implemented format is: |
laurensvalk
changed the title
- pbsys_bluetooth_rx_get_available is used as a function - with pbsys_bluetooth_rx / pbsys_bluetooth_tx used as defines we get unused parameter warnings for static variables.
This lets the user restart the program with the button on all stm32 hubs. This is accomplished by not throwing away the .mpy file after we run it.
This fixes a long-standing REVISIT comment in main.c. We may still need to revisit the stack sizes as these limits may not have been hit in the past. On the smaller hubs, they are now set to roughly what they were before with a fixed heap.
This takes the first step towards receiving the program in pbsys. This makes sense in the long run, and it is useful now for program storage management. To make it easy to see what is going on, we make the change in several steps. In this commit, the program reception process is moved as-is. This keeps the blocking calls, but drops the MicroPython HAL references.
This makes it clearer which is main and which is the application being run on top.
This avoids hard coding it in two places. Each chunk is still broken down into chunks of 20 bytes coming from the IDE. If we change the MTU to match (or negotiate with Pybricks Code), we could use this config value to improve throughput on some hubs.
Since the main application is cancellable from pbsys and the download process was recently moved to pbsys, we don't need this anymore. This simplifies the main code and it saves a consirable amount of space.
The virtual hub has sys enabled, but doesn't currently download programs to run them. This is probably a sign that this portion should be split off to a dedicated module, but we can do that after cleaning up pbsys.
Use a single process definition instead, relying on pbsysconfig to see if something should be enabled.
These are part of pbio and don't belong in the application code.
Some builds support user programs, but not loading new programs.
- Separates the download process from the program getter. - Cleans up memory map access. - Cleans up memory addressess and size checks using the linker script.
This makes "file access" non-blocking from pbio on Prime Hub, which avoids timing problems with other peripherals. It uses only a small portion of external flash to avoid touching any existing file system made by different firmwares. This also allows us to save data on shutdown on the other hubs.
|
This has been split out to #109 (download and run), #110 (non-blocking spi), #112 (powered up storage), and https://github.com/pybricks/pybricks-micropython/tree/fix-526, so we can close this one. |
This makes the allocation of the user RAM entirely local to the pbsys_program_load module instead of having some config mixed into the linker scripts. This also lets us tune the code/heap ratio on a per-hub basis instead of hard-coding only 3K for heap. Instead of a .user_ram section, we now have a more generic .noinit section in RAM that is not zeroed during early boot. This is used for the user RAM since it is restored from nonvolatile memory on startup.
This is a major upgrade on how we load, keep, and save user scripts. Highlights:
Until now, we had to discard the downloaded user program to avoid using twice the amount of RAM when running it. This PR explores an experimental MicroPython feature (micropython/micropython#8381) that lets us run the program without making a full copy.
This means we can keep the program in RAM as we run it. In turn, this means we can run it again by just pressing the button, and also leave more RAM for user variables. But perhaps more importantly, keeping the program means we can save it on the hub on shutdown. We could not do this before because flash access is a "slow" process that blocks other things like sensor communication. But if we're shutting down anyway, that doesn't matter. This also considerably reduces the number of times you'd save something on flash.
On SPIKE hubs, discarding the program from RAM was not an issue because it could be saved on the external file system. However, this PR gives us the opportunity to solve a related problem. There are several (official and unofficial) firmwares out there for SPIKE hubs. When each firmware uses its own file system, things get messy and it is hard to explain to new users. We can work around this by storing the user program in a region of the external flash that isn't used by the file system. Programs can only be as large as the RAM (~280Kb), so not much space is needed. If we ever enable user access to the file system, we could do it the MicroPython way as long as the low-level I/O operations are not blocking.
I expect that this will not be merged soon. It will need a thorough review and extensive testing, but this should allow for some discussion and interactive review.
The Move Hub build size increases by 868 bytes, which is not bad considering what is being added. There should be room for quite a bit of optimization as well.
Some remaining tasks:
Revisions: