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

Make crate compatible with no_std #86

Merged
merged 5 commits into from
Nov 14, 2024
Merged

Conversation

claudiomattera
Copy link
Contributor

This pull request makes the crate usable in no_std environments (alloc is still required).

Trivial changes:

  • I replaced all std::collections::HashSet and std::collections::HashMap with alloc::collections::BTreeSet and alloc::collections::BTreeMap.
    They should be nearly equivalent, unless there was an actual reason for using HashMap instead of BTreeMap that I missed.

  • I put the constructor for Config that takes a path and reads it from a file behind a feature std.
    On a no_std environment the client should load the configuration on their own, and pass it as a string.

  • I replaced all std::time::Duration with core::time::Duration.

Non-trivial changes:

  • std::time::Instant is not available in core, because without the operating system there is no general way to get the current time.
    I defined a trait in time::Instant, so clients can choose an appropriate representation for their platform.

    Then Driver takes a generic argument INSTANT: Instant.
    A predefined alias StdDriver is available for std platforms, using std::time::Instant, gated behind a feature.

  • std::thread::sleep() is also not available in core, and spin_sleep requires std.
    I defined a trait in time::Sleeper, clients can choose an appropriate implementation for their platform.

    Then Scheduler takes two generic arguments INSTANT: Instant and SLEEPER: Sleeper.
    A predefined alias StdScheduler is available for std platforms, using std::time::Instant and std::thread:sleep, and another one SpinScheduler, using std::time::Instant and spin_sleep::sleep, both gated behind features.

  • I added an AsyncSleeper that has an async interface (embassy is getting popular in embedded Rust).

    Scheduler is the only place that handles sleeping, and it is not so big, so I simply duplicated the logic into AsyncScheduler.
    There might be a more clever approach using maybe-async.

  • In some no_std environments, floating-point operations are not available.
    I added num-traits as a dependency, which reimplements those operations with libm.
    This is gated behind a feature std.

  • Finally, I set the right features of the dependencies so that they would also be no_std.
    Mostly, I disabled default features and enabled features = ["libm"].

The result is a crate that can be used in no_std environments, though it still needs an allocator for dynamic memory.
I guess it would require a non-trivial redesign to make it work without dynamic allocation.

Anyway, as long as a microcontroller has enough memory, it should not be an issue to enable alloc.

Moreover, I also implemented a simple firmware using embassy on a ESP32-C3.
I implemented Instant on a wrapper around embassy_time::Instant, and Sleeper on a wrapper around embassy_time::Timer::after().
But I am not sure where to put this example.
It needs a custom cargo configuration, so it cannot be easily placed in the directory example.

Note: I do not actually have a LED strip to test it :D
For now this firmware just prints the LEDs values to the serial interface.

@DavJCosby
Copy link
Owner

Thanks again for all this, incredible work. I'll take some time after classes today to review the changes.

I'd like to get this in for 0.12. That said, there are a handful of breaking changes planned for 0.20 planned (see #64 and #85), with some work already done on the decouple_colors branch that I may need some help porting over to no_std. I may reach out again when that's moved further along.

@DavJCosby
Copy link
Owner

DavJCosby commented Nov 13, 2024

Moreover, I also implemented a simple firmware using embassy on a ESP32-C3.
I implemented Instant on a wrapper around embassy_time::Instant, and Sleeper on a wrapper around embassy_time::Timer::after().
But I am not sure where to put this example.
It needs a custom cargo configuration, so it cannot be easily placed in the directory example.

I've decided to make a separate spatial_led_examples repository. It doesn't make a whole lot of sense to have ratatui as a dev dependency for this crate, and it'd be nice to have some examples for more specific setups like raspberry pi or no_std. I'll hit you up when I've got that all set up and you can dump your example in there. Once this is merged into main, you can submit a PR and get your example in there.

@DavJCosby
Copy link
Owner

DavJCosby commented Nov 13, 2024

It seems that enabling the std flag on glam doesn't automatically override libm.

This PR compared to master, compiled with std enabled.
Screenshot 2024-11-12 at 6 01 26 PM

Explicitly disabling libm and enabling std via

glam = { version = "0.29", default-features = false, features = ["std"] }
Screenshot 2024-11-12 at 6 04 35 PM

I'm a little stumped on how to make glam only compile with the libm feature if std is disabled. I could introduce a no_std flag and require that the user has either std or no_std enabled, but that feels kinda gross.

On a side note, we could probably expose core-simd for those willing to compile with nightly for some extra performance on some no_std systems.

Copy link
Owner

@DavJCosby DavJCosby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably the way to go, though I'm open to other suggestions. If you're opting out of std, you should also opt in to libm if you'll need it.

Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
src/driver/mod.rs Outdated Show resolved Hide resolved
src/scheduler/mod.rs Outdated Show resolved Hide resolved
@DavJCosby DavJCosby added the design Discussing larger design and architecture ideas label Nov 13, 2024
@claudiomattera
Copy link
Contributor Author

I pushed some of the suggested changes.
I tried to apply them from the review page, but it did not work...

@DavJCosby DavJCosby dismissed their stale review November 14, 2024 22:19

Addressed through follow-up commits.

Copy link
Owner

@DavJCosby DavJCosby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks again!

@DavJCosby DavJCosby merged commit d08b1ca into DavJCosby:master Nov 14, 2024
2 checks passed
@DavJCosby DavJCosby added this to the 0.1.2 milestone Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Discussing larger design and architecture ideas
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants