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

Refactor MicroPython hacks out of Pico Unicorn #463

Closed
wants to merge 1 commit into from

Conversation

Gadgetoid
Copy link
Member

This currently... does not work. It's failing on soft reset, which is fairly standard for anything touching PIO, DMA and ISRs and such.

The general premise here is to remove the static buffer in favor of runtime allocation and use MicroPython's m_tracked_calloc along with placement new to allocate Pico Unicorn - buffer and all - on MicroPython's gc_heap.

So far it sort-of half works.

We could probably move the PicoUnicorn *unicorn` into the class as a static member and make the class behave in a singleton fashion in all cases. This would help internalize the __isr, too, which currently has to float around as:

void __isr picounicorn_dma_complete() {
    if(unicorn) {
        unicorn->dma_complete();
    }
}

@Gadgetoid
Copy link
Member Author

Attempted to refactor this into a class to handle the teardown of Pico Unicorn when the garbage collector calls __del__.

Broke it. 😢

@Gadgetoid
Copy link
Member Author

Note that tracked_calloc is inappropriate for C class instance storage since it does not have a finaliser or call to __del__ so a class cannot be cleaned up and the allocated memory is lost upon soft reset.

The fundamental problem is that soft reset does not reset the pool of DMA channels or PIO so failing to track the allocated channel and clean it up (or re-use it) will either burn through DMA channels (in the claim_unused case) or lead to soft locks, weird issues with dangling handlers and all sorts of other confusion.

The nuclear option is for everything to be a MicroPython class that requires the user to specify the DMA channel, PIO and SM that should be used. Perhaps more rationally channel usage could be tracked on a per-consumer basis like GPIO is in libgpiod on Linux but this would come with a whole host of other potential pitfalls and complexities.

@Gadgetoid
Copy link
Member Author

@ZodiusInfuser perhaps this needs the GU magic 😆

@ZodiusInfuser ZodiusInfuser added c++ This issue or request relates to C++ code [- pico unicorn pack -] https://shop.pimoroni.com/products/pico-unicorn-pack labels Jan 20, 2023
@Gadgetoid
Copy link
Member Author

We should probably still update this to the techniques used by Galactic/Cosmic, but MicroPython's "Greedy Heap" fix means that we can now safely allocate RAM at compile-time and the heap will resize accordingly. We should take advantage of that where appropriate.

@Gadgetoid
Copy link
Member Author

Replaced by #711

@Gadgetoid Gadgetoid closed this Mar 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[- pico unicorn pack -] https://shop.pimoroni.com/products/pico-unicorn-pack c++ This issue or request relates to C++ code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants