Skip to content
This repository was archived by the owner on Nov 11, 2025. It is now read-only.

Support external parameters in the voltage divider#17

Merged
twelho merged 2 commits into
mainfrom
vdiv-parameters
Jul 5, 2021
Merged

Support external parameters in the voltage divider#17
twelho merged 2 commits into
mainfrom
vdiv-parameters

Conversation

@twelho
Copy link
Copy Markdown
Member

@twelho twelho commented Jun 29, 2021

See the doc comments for the voltage_divider function for details. By passing external parameters as arguments to the vdiv function we can leverage the dependency resolving of the evaluator. The given parameters will subsequently be made available via En identifiers where n counts up from 1 (like is commonly done for resistors as well).

Completes the final task of #16.

@twelho
Copy link
Copy Markdown
Member Author

twelho commented Jul 2, 2021

The escaping/quoting issue in kicad_parse_gen probably originates from the function parse_split_quote_aware_int in lib.rs. I can look into it a little more deeply at some point.

Copy link
Copy Markdown
Member

@chiplet chiplet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments and questions. Otherwise LGTM 👍

Comment on lines 144 to +153
/// `voltage_divider` computes values for resistor-based voltage dividers.
/// - Usage: vdiv(<target voltage>, <divider expression>, <resistor series>, (min resistance), (max resistance))
/// - Example: vdiv(5.1, "(R1+R2)/R2*0.8", "E96", 500e3, 700e3)
/// - Usage: vdiv(<target voltage>, <divider expression>, <resistor series>,
/// {(<min resistance>, <max resistance>)}, ({extra 1}, {extra 2}, ...))
/// - Example: vdiv(5.1, "(R1+R2)/R2*E1", "E96", (500e3, 700e3), (0.8))
/// - Output: (<closest voltage>, <R1 value>, <R2 value>, ...)
/// There can be arbitrary many resistors in the divider, but they must be named "R1", "R2", etc.
/// The computed optimal resistance values are also presented in this order. The minimal and maximal
/// resistance limits are optional parameters, and only consider the sum of the resistances of all
/// resistors defined in the expression.
/// resistance pair is an optional parameter, and the limits only consider the sum of resistance of
/// all resistors defined in the expression. The "extra" parameters are optional external inputs for
/// the divider expression, and will be made available as "E1", "E2", etc. in order.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason this doc comment is not showing up in the documentation generated with cargo doc.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The vdiv module is purposefully private (not pub), so to generate docs for it requires passing the --document-private-items flag to cargo doc. But thanks for noticing this, the formatting didn't like the unescaped <> so I've fixed that now in 7be3cd0.

Comment on lines +25 to 32
let re = Regex::new(r"^R[1-9][0-9]*$").unwrap();
let mut set = HashSet::new();
e.iter_variable_identifiers().for_each(|i| {
set.insert(i);
});
e.iter_variable_identifiers()
.filter(|i| re.is_match(i)) // Match only R? identifiers
.for_each(|i| {
set.insert(i);
});
set.len()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the HashSet used here? Couldn't this be just the following?

fn resistor_identifiers(e: &Node) -> usize {
    let re = Regex::new(r"^R[1-9][0-9]*$").unwrap();
    e.iter_variable_identifiers()
        .filter(|i| re.is_match(i)) // Match only R? identifiers
        .count()
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Say you have parsed an expression of the form R2 = R1 * R1. What iter_variable_identifiers() is going to output is simply all variable identifiers of the parsed node tree in order, i.e. the list R2, R1, R1. The HashSet is used to keep track of already encountered identifiers for a relatively simple way to avoid counting a given identifier more than once.

Copy link
Copy Markdown
Member

@luxas luxas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks 👍

@twelho twelho merged commit 899bafe into main Jul 5, 2021
@twelho twelho deleted the vdiv-parameters branch July 5, 2021 11:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants