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

embedded_storage::ReadStorage should return the number of bytes written into the array #63

Open
rfraser-spideroak opened this issue Oct 31, 2024 · 1 comment · May be fixed by #64
Open

Comments

@rfraser-spideroak
Copy link

Presently the read trait is:

fn read(&mut self, offset: u32, bytes: &mut [u8]) -> Result<(), Self::Error>;

with the idea "This should throw an error in case bytes.len() will be larger than self.capacity() - offset.". The main issue with this is it means our bytes buffer will always need to be "self.capacity() - offset" which would be needlessly wasteful for a vector to be resized every time it reaches the end of a file or one would need to have a precisely sized buffer. This prevents more usable patters in embedded such as:

while !file.is_eof() {
        let mut buffer: [u8; 16] = [0; 16];
        let characters_parsed = file.read(&mut buffer)?;
        serialize_vec.extend_from_slice(&buffer[..characters_parsed]);
}

This allows for the looping read of a fixed size buffer in a manner that extends a vector if a large amount of information needs to be collected and read at once (such as in the case of serialization and deserialization). For no-alloc this also allows a fixed size buffer to be used to read some arbitrary storage. I think that for embedded work this is far more typical interface and if the usize isn't needed by some implementations it can be dropped whereas without this implementation some storage implementations on some hardware peripherals wont work.

So I think:

fn read(&mut self, offset: u32, bytes: &mut [u8]) -> Result<(), Self::Error>;

should be changed to:

fn read(&mut self, offset: u32, bytes: &mut [u8]) -> Result<usize, Self::Error>;

which returns how many bytes were read, or an error

@rowanfr
Copy link

rowanfr commented Nov 12, 2024

An alternative could be a read_buffer method or a separate buffer storage trait for no-alloc implementation, though that might add more complexity than desired especially if trying to reach the greatest common denominator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants