blocksds / sdk

Main BlocksDS SDK repository
https://blocksds.github.io/docs/
130 stars 5 forks source link

The asynchronous file loading example is broken #132

Closed AntonioND closed 3 months ago

AntonioND commented 4 months ago

https://github.com/blocksds/sdk/blob/5332802633773b8cb0bd7797c9d16f470170dbfa/examples/filesystem/async_loading/source/main.c

This example doesn't work on hardware (only one file will be read correctly, the other one won't even be opened by fopen()).

I suspect that the issue is that we haven't implemented the functions mentioned here: https://github.com/picolibc/picolibc/discussions/310

But simply defining the functions in libnds isn't enough to override the default stubs.

AntonioND commented 3 months ago

Just an update on this.

I've created a prototype branch here https://github.com/blocksds/libnds/commit/8bc457e852b5e3f2264ea0d6db82a6871f2d933c with an implementation of the libc locks that are required for using libc functions in multithreaded environments.

This requires a change in picolibc: https://github.com/WonderfulToolchain/wf-picolibc/pull/1

The maintainer of picolibc mentions that if you implement all locking functions and variables in your own code, the ones of picolibc (placeholder functions) won't be included, but this doesn't seem to work for libnds because it's another library, not the final application: https://github.com/picolibc/picolibc/discussions/310

For example, this patch makes the example work: https://github.com/blocksds/sdk/commit/a62fa3f036c64ba69cbbe6082aff1ae0e5558314

However, this is all a draft, and I'm happy to change the approach to overriding the picolibc placeholders.

AntonioND commented 3 months ago

I've applied this patch to libnds https://github.com/blocksds/libnds/commit/8539afd00840d8d83b9453920e57049e518caf3d and deleted the locks branch. While this isn't fully working yet, it doesn't hurt to have it in libnds (at worst, it will be ignored), and it makes my life easier when testing things.

AntonioND commented 3 months ago

This patch has made the functions be included correctly: https://github.com/blocksds/libnds/pull/90 Thanks to Asie and Keith for their help. I ended up going with one of the suggestions in this discussion: https://github.com/picolibc/picolibc/discussions/310

I've also had to revert the changes I sent to wf-picolibc: https://github.com/WonderfulToolchain/wf-picolibc/pull/2

EDIT: The remaining work is to clean the locking functions and add some half-decent error handling.

AntonioND commented 3 months ago

Ok, I think this is ready. The following 3 commits take care of it:

Documentation: https://github.com/blocksds/sdk/commit/2ada1c5510770653559a01b8c9af4b2f8a94a233