Add config writing capability #99

Closed
cw wants to merge 3 commits from <deleted>:master into master

#63

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.

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
@ -106,0 +109,4 @@
let root_dir = fs.root_dir();
let mut file;
let isascii_and_short = data.is_ascii() & (data.len() <= 100);

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
@ -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;

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.

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
sb10q reviewed 2020-08-28 11:28:16 +08:00
@ -106,0 +131,4 @@
if isascii_and_short {
match root_dir.remove(config_key_bin) {
Ok(_) => {},
Err(_) => {}

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".
sb10q reviewed 2020-08-28 12:36:22 +08:00
@ -106,0 +125,4 @@
file.truncate()?;
file.write_all(&[key,"="].concat().as_bytes())?;
file.write_all(data.as_bytes())?;

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
@ -106,0 +130,4 @@
if use_config_txt {
match root_dir.remove(config_key_bin) {
Ok(_) => {},
Err(e) => match e.kind() {

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...
cw closed this pull request 2020-09-18 18:58:15 +08:00

Pull request closed

Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: M-Labs/artiq-zynq#99
There is no content yet.