Add config writing capability #99

cw · 2020-08-28T10:59:08+08:00

cw commented

2020-08-28 10:59:08 +08:00

First-time contributor

The new function write_str has been tested. It works with both long (more than 100 characters) and short strings.

https://git.m-labs.hk/M-Labs/artiq-zynq/issues/63 The new function write_str has been tested. It works with both long (more than 100 characters) and short strings.

cw referenced this pull request

2020-08-28 11:01:04 +08:00

implement config writes #63

sb10q commented

2020-08-28 11:25:11 +08:00

This needs to be used in the management interface so that artiq_coremgmt can write config keys across the network.

This needs to be used in the management interface so that ``artiq_coremgmt`` can write config keys across the network.

sb10q reviewed 2020-08-28 11:25:52 +08:00

src/runtime/src/config.rs Outdated

						
				@ -106,0 +109,4 @@

				            let root_dir = fs.root_dir();

				            let mut file;

				            let isascii_and_short =  data.is_ascii() & (data.len() <= 100);

sb10q commented

2020-08-28 11:25:52 +08:00

A better variable name would be use_config_txt.

A better variable name would be ``use_config_txt``.

sb10q reviewed 2020-08-28 11:27:06 +08:00

src/runtime/src/config.rs Outdated

						
				@ -106,0 +107,4 @@

				    pub fn write_str<'b>(&mut self, key: &str, data: &str) -> Result<'b, ()>{

				        if let Some(fs) = &self.fs {

				            let root_dir = fs.root_dir();

				            let mut file;

sb10q commented

2020-08-28 11:27:06 +08:00

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

First-time contributor

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

sb10q reviewed 2020-08-28 11:28:16 +08:00

src/runtime/src/config.rs Outdated

						
				@ -106,0 +131,4 @@

				            if isascii_and_short {

				                match root_dir.remove(config_key_bin) {

				                    Ok(_) => {},

				                    Err(_) => {}

sb10q commented

2020-08-28 11:28:16 +08:00

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

sb10q reviewed 2020-08-28 12:36:22 +08:00

src/runtime/src/config.rs Outdated

						
				@ -106,0 +125,4 @@

				            file.truncate()?;

				            file.write_all(&[key,"="].concat().as_bytes())?;

				            file.write_all(data.as_bytes())?;

sb10q commented

2020-08-28 12:36:22 +08:00

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``?

sb10q reviewed 2020-08-28 12:37:11 +08:00

src/runtime/src/config.rs Outdated

						
				@ -106,0 +130,4 @@

				            if use_config_txt {

				                match root_dir.remove(config_key_bin) {

				                    Ok(_) => {},

				                    Err(e) => match e.kind() {