-
Notifications
You must be signed in to change notification settings - Fork 4
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
Support arbitrary byte sequence containers #3
base: main
Are you sure you want to change the base?
Conversation
Previously, the Command and Response types always used heapless::Vec to store the payload. This was awkward to use in non-embedded environments where std::vec::Vec might be preferable, or in situations where a slice is sufficient. This patch relaxes this requirement and accepts any type implementing AsRef<[u8]> (and TryFrom<&[u8]> for construction).
For reference, here is an updated version of |
pub type Result<T=()> = core::result::Result<T, Status>; | ||
|
||
pub mod aid; | ||
pub mod command; | ||
pub mod response; | ||
pub mod somebytes; |
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.
Rename to data
?
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.
Also generally wondering if we should have all these public submodules; assuming they're just implementation, we could pub use
the core data structures + traits at crate level and let the submodules be non-public.
@@ -10,12 +10,12 @@ pub enum Interface { | |||
Contactless, | |||
} | |||
|
|||
pub type Data<const S: usize> = heapless::Vec<u8, S>; |
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.
pub use data::Data
(where data::Data
is the current somebytes::Bytes
)? There's already a bunch of Bytes
types in the eco-system, if we're rolling our own it would seem to make sense to name it semantically.
/// | ||
/// This wrapper implements common traits based on the content of the byte sequence. | ||
#[derive(Clone)] | ||
pub struct Bytes<T: AsRef<[u8]>>(T); |
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 suggest pub struct Data<B: AsRef<[u8]>>(B)
here. You're also using the B
elsewhere (pointing to it meaning Bytes).
#[inline] | ||
fn into(self) -> Data<S> { | ||
fn into(self) -> heapless::Vec<u8, S> { |
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.
Maybe add an inherent into_vec
method (in case it doesn't already exist).
fn extend_from_slice(&mut self, slice: &[T]) -> Result<(), Self::Error>; | ||
} | ||
|
||
impl<T: Clone, const N: usize> TryExtendFromSlice<T> for heapless::Vec<T, N> { |
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 think it would make sense to have a alloc
feature on the crate, and conditionally implement TryExtendFromSlice for alloc::vec::Vec
?
} | ||
} | ||
|
||
impl<T: AsRef<[u8]>> fmt::Debug for Bytes<T> { |
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.
Might be nice to have the Debug display hex? We already depend on delog
, which re-implements hex-fmt
.
Not necessarily for this PR, but it would be nice to have a serde
feature, with at least a corresponding serde::ser
implementation (as this can't be done outside this crate). Even nicer if it outputs hex if serializer.is_human_readable()
, else actual bytes.
@@ -12,7 +12,8 @@ documentation = "https://docs.rs/iso7816" | |||
|
|||
[dependencies] | |||
delog = "0.1.2" | |||
heapless = "0.7" | |||
# use unreleased heapless for https://github.com/japaric/heapless/pull/255 | |||
heapless = { git = "https://github.com/japaric/heapless" } |
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.
Seems this is merged, maybe also published?
Support arbitrary status code
Previously, the Command and Response types always used
heapless::Vec
to store the payload. This was awkward to use in non-embedded environments wherestd::vec::Vec
might be preferable, or in situations where a slice is sufficient.This patch relaxes this requirement and accepts any type implementing
AsRef<[u8]>
(andTryFrom<&[u8]>
for construction).