M-Labs
/
artiq-zynq
10
1
Fork 19
Code Issues 13 Pull Requests Releases Wiki Activity

Add config writing capability #99

Closed
cw wants to merge 3 commits from (deleted):master into master
pull from: (deleted):master
merge into: M-Labs:master
Conversation 1 Commits 3 Files Changed 1 +11 -7
1 changed files with 11 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 22db44fdb9 - Show all commits

18
src/runtime/src/config.rs
View File

@ -1,7 +1,7 @@
use crate::sd_reader; use crate::sd_reader;
use core::fmt; use core::fmt;
use alloc::{string::FromUtf8Error, string::String, vec::Vec}; use alloc::{string::FromUtf8Error, string::String, vec::Vec};
use core_io::{self as io, BufRead, BufReader, Read, Write}; use core_io::{self as io, BufRead, BufReader, Read, Write, ErrorKind};
use libboard_zynq::sdio; use libboard_zynq::sdio;
@ -107,19 +107,18 @@ impl Config {
pub fn write_str<'b>(&mut self, key: &str, data: &str) -> Result<'b, ()>{ pub fn write_str<'b>(&mut self, key: &str, data: &str) -> Result<'b, ()>{
if let Some(fs) = &self.fs { if let Some(fs) = &self.fs {
let root_dir = fs.root_dir(); let root_dir = fs.root_dir();
let mut file;
sb10q commented 2020-08-28 11:27:06 +08:00
Outdated
Review
Copy Link

AFAICT this does not need a forward declaration.
You can just do let mut file = match... below. The mut is perhaps not needed either.

AFAICT this does not need a forward declaration. You can just do ``let mut file = match...`` below. The ``mut`` is perhaps not needed either.
cw commented 2020-08-28 11:31:36 +08:00
Outdated
Review
Copy Link

If mut is not used, I get "cannot borrow as mutable" when calling truncate and write_all

If mut is not used, I get "cannot borrow as mutable" when calling truncate and write_all
let isascii_and_short = data.is_ascii() & (data.len() <= 100); let use_config_txt = data.is_ascii() & (data.len() <= 100);
let file_path; let file_path;
sb10q commented 2020-08-28 11:25:52 +08:00
Outdated
Review
Copy Link

A better variable name would be use_config_txt.

A better variable name would be ``use_config_txt``.
let config_key_bin = &["/CONFIG/", key, ".BIN"].concat(); let config_key_bin = &["/CONFIG/", key, ".BIN"].concat();
if isascii_and_short { if use_config_txt {
file_path = "/CONFIG.TXT"; file_path = "/CONFIG.TXT";
} else { } else {
file_path = config_key_bin; file_path = config_key_bin;
} }
file = match root_dir.create_file(file_path) { let mut file = match root_dir.create_file(file_path) {
Ok(f) => f, Ok(f) => f,
Err(_) => root_dir.open_file("/CONFIG.TXT")? Err(_) => root_dir.open_file("/CONFIG.TXT")?
}; };
@ -128,10 +127,15 @@ impl Config {
file.write_all(&[key,"="].concat().as_bytes())?; file.write_all(&[key,"="].concat().as_bytes())?;
file.write_all(data.as_bytes())?; file.write_all(data.as_bytes())?;
sb10q commented 2020-08-28 12:36:22 +08:00
Outdated
Review
Copy Link

Why not a single file.write_all(&[key, "=", data].concat().as_bytes())? (or similar) ?
Also, won't that erase all the other keys from config.txt?

Why not a single ``file.write_all(&[key, "=", data].concat().as_bytes())?`` (or similar) ? Also, won't that erase all the other keys from ``config.txt``?
if isascii_and_short { if use_config_txt {
match root_dir.remove(config_key_bin) { match root_dir.remove(config_key_bin) {
Ok(_) => {}, Ok(_) => {},
Err(_) => {} Err(e) => match e.kind() {
sb10q commented 2020-08-28 12:37:11 +08:00
Outdated
Review
Copy Link

You could simply add a Err(ErrorKind::NotFound) branch to the main match statement...

You could simply add a ``Err(ErrorKind::NotFound)`` branch to the main ``match`` statement...
ErrorKind::NotFound => {},
sb10q commented 2020-08-28 11:28:16 +08:00
Outdated
Review
Copy Link

You want to report errors that are not "the file doesn't exist".

You want to report errors that are not "the file doesn't exist".
_ => {
return Err(Error::IoError(e));
}
}
}; };
} }