-
Notifications
You must be signed in to change notification settings - Fork 60
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
Making core no_std compatible #94
base: main
Are you sure you want to change the base?
Conversation
For what I've checked, snafu has it's own |
Mhmm! The main reason behind wanting to explore |
@pranav-bhatt you need to configure the ci to run the no_std builds/tests too |
src/message/no_std_io.rs
Outdated
@@ -0,0 +1,208 @@ | |||
//! https://docs.rs/not-io/0.1.0-alpha/not_io/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This basically is a work around for std::io::Error
and std::io::Result<>
. If you're asking about the link, then I forgot to remove it :P
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, why we need io::Error? Can you just disable it in no_std envs?
@@ -1,4 +1,5 @@ | |||
use chrono::{DateTime, Utc}; | |||
use std::prelude::v1::*; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we enable these imports around the sdk only if the no_std
feature is enabled?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the Cargo.toml
, you need to define the std feature, like in https://github.com/KodrAus/rust-no-std/blob/master/Cargo.toml and https://github.com/shepmaster/snafu/blob/master/Cargo.toml and enable it by default.
Then try to create an embedded project in your machine and try to import this library
Cargo.toml
Outdated
@@ -25,6 +25,17 @@ base64 = "^0.12" | |||
url = { version = "^2.1", features = ["serde"] } | |||
snafu = "^0.6" | |||
|
|||
#[dependencies.snafu] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In case of no_std
env, you need to import snafu like below (no need to include guide
feature)
src/message/error.rs
Outdated
Other { | ||
source: Box<dyn std::error::Error + Send + Sync>, | ||
}, | ||
IOError { source: super::no_std_io::Error }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need this variant (and the one below) in no_std envs
Can you rebase? |
@pranav-bhatt you should look at this #107 |
@pranav-bhatt can you start cleaning up this pr removing history files etc and rebasing with master? This branch diverged quite a bit from master |
Signed-off-by: [email protected] <[email protected]>
Signed-off-by: [email protected] <[email protected]>
Signed-off-by: [email protected] <[email protected]>
Signed-off-by: [email protected] <[email protected]>
Signed-off-by: [email protected] <[email protected]>
Signed-off-by: [email protected] <[email protected]>
Signed-off-by: [email protected] <[email protected]>
Signed-off-by: [email protected] <[email protected]>
Signed-off-by: [email protected] <[email protected]>
Signed-off-by: [email protected] <[email protected]>
#72
Things left to do:
DisplayError
(chrono example, serde-json example), and generally clean uperror.rs