Move program download to pbsys

[laurensvalk]

#106 is too big to review in one go, so I'm splitting it up in several smaller pull requests. I'm running out of time for today, so I'll mark it as a draft for now. I'd still need to review some of the comments on this part from #106 as well.

---

pbsys: Move program download out of MicroPython.

Just like starting and stopping the program, (down)loading a program is
a task for pbsys. Using a protothread, the download procedure is now
much easier to follow.

Program data is preserved, so that it can be restarted with the button.
This paves the way for supporting persistent programs on all hubs.

By cleaning up the way timeouts work, the user is now less likely to see
a message about an incompatible mpy file when accidentally typing
something in the I/O pane of Pybricks Code.

This also drops the shutdown longjmp. The download thread is cancellable,
and the user program was already cancellable, so we don't need this anymore.

This simplifies the main code and it saves about 500 bytes on Move Hub,
which turns this commit into a slight decrease in code size.

---

This PR aims to keep almost everything working as it did before. Since this contains only a portion of #106, there are two main regressions that will be addressed in future work:

• MicroPython makes a copy of the program when it loads it, so programs take about twice as much RAM. This will be addressed with a MicroPython update that runs it without copying.
• Flash storage on the SPIKE hubs is not currently used for anything, so programs are not saved to the hub at the moment.

[laurensvalk]

PROCESS_THREAD(pbsys_program_receive_process, ev, data) {

@dlech dlech 7 days ago
This looks like it should just be a PT_THREAD rather than a process.

This is done now, in the form below.

• pbsys_program_load_receive_start starts the protothread
• pbsys_program_load_receive_end keeps scheduling it until complete

Curious to hear if this should perhaps be done differently.

int main(int argc, char **argv) {

    pbio_error_t err;

    pbio_init();
    pbsys_init();

    // Keep loading and running user programs until shutdown flag is set.
    while (!pbsys_status_test(PBIO_PYBRICKS_STATUS_SHUTDOWN)) {

        // Run program receive thread until completion. It cancels on shutdown.
        static pbsys_main_program_t program;
        pbsys_program_load_receive_start();
        while ((err = pbsys_program_load_receive_end(&program)) == PBIO_ERROR_AGAIN) {
            pbio_do_one_event();
        }

        // On a successful transfer, run the main application. The application
        // must register the appropriate callbacks in sys/user_program so that
        // it can be canceled on shutdown.
        if (err == PBIO_SUCCESS) {
            pbsys_main_application(&program);
        }
    }

    // shutdown stuff ...

---

thinking about it a bit more, a fixed heap might actually be a good thing. as the firmware static RAM usage grows over time, the heap will shrink, so users that maxed out heap usage may start getting out of memory errors when they upgrade the firmware.

Making (program data + heap) constant would be fine, so that growing bss won't change anything for users.

I wouldn't limit the program data / heap individually more than needed though. Until now, users could balance between large programs and large amounts of user variables. It would be nice to keep it that way.

[dlech]

Curious to hear if this should perhaps be done differently.

We could combine the start and end functions into a single function and move event loop inside of that function.

Making (program data + heap) constant would be fine, so that growing bss won't change anything for users.

Agreed. User program binary + MicroPython heap is what I meant by "heap". Maybe a better/more generic short name for the combined memory area is "user RAM"?

[laurensvalk]

We could combine the start and end functions into a single function and move event loop inside of that function.

Done. Some fixes are pushed as a separate commit to make it easier to review what changed. I'll squash it later.

[laurensvalk]

Maybe a better/more generic short name for the combined memory area is "user RAM"?

I have now cleaned it up as follows:

    /* The user_ram section between ebss and sstack is used for two purposes:
       - The system uses the first portion to store user program data.
       - The system can then run an application, which has read-only access to
         the user data. The remaining space is used by the application as heap.
    */
    _pbsys_program_load_user_ram_start = _ebss;
    .user_ram :
    {
        . = ALIGN(4); 
        /**
         * The full storage may be loaded to RAM on boot, so we need to have
         * at least as much RAM available. We also want to ensure there is
         * at least 2K of additional RAM for the application heap.
         */
        . = . + _pbdrv_block_device_storage_size + 2048;
        . = ALIGN(4); 
    } >RAM

thinking about it a bit more, a fixed heap might actually be a good thing. as the firmware static RAM usage grows over time, the heap will shrink, so users that maxed out heap usage may start getting out of memory errors when they upgrade the firmware.

I don't have strong opinion on this one, so maybe we could defer it to a separate issue to expedite this PR?

For what it's worth, I think it's OK to use what is available rather than disabling an arbitrary amount of RAM, especially on the smaller hubs.

Keeping an eye on bss could be a good way to encourage ourselves to make it smaller like we've done with build size. We've discussed making some infrequently used objects dynamic (e.g. the drivebases).