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

Implement the classifier, and validation and defaulting engine using CUE#15

Merged
luxas merged 5 commits into
mainfrom
policy
Jul 18, 2021
Merged

Implement the classifier, and validation and defaulting engine using CUE#15
luxas merged 5 commits into
mainfrom
policy

Conversation

@luxas
Copy link
Copy Markdown
Member

@luxas luxas commented Jun 21, 2021

This makes the classifier use CUE for all policy-related things. It is good enough for our needs and lets us classify components, set default attributes and labels for components once classified, and enforce validation on fields set in KiCad.

I will still need to rebase this PR slightly.

cc @twelho @chiplet

Copy link
Copy Markdown
Member

@twelho twelho left a comment

Choose a reason for hiding this comment

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

Very good start! Just a couple of nits/questions (and of course this needs rebasing). The CUE descriptions look quite clear even though I don't have enough experience in the language yet to properly review them.

There was a TODO for supporting hierarchical/nested sheets, so if those aren't supported yet, we need to get them working before this can be utilized in full.

It might also be valuable to build a specialized CUE binary (there was some Go tool to do this IIRC) with the required functionality built in, writing stuff to a temporary directory and execing a binary in $PATH isn't very integrated (but fine for now).

Comment on lines +16 to +23
key: "footprintLibrary"
op: "Equals"
values: ["Capacitor_SMD"]
}
}, _tmpl_resistor & {
class: "resistor"
}, _tmpl_resistor & { // shunt_resistor extends all the properties of resistor
class: "shunt_resistor"
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.

There's now identifiers in camel case (footprintLibrary) and snake case (shunt_resistor) here, do we want to establish some convention for this?

Comment thread tools/kicad_rs/sample_policy.cue Outdated
#Policy: {
shunt_resistor: {
labels: {
extra: shunt: "true"
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.

This "inline" syntax looks interesting, but I guess it's valid CUE

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.

Yes, it is valid CUE, and pretty handy in quite a lot of cases actually.

}
}
capacitor: attributes: Value: {
comment: string | *"This is a capacitor :D!"
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.

Just out of curiosity, is the | operator commutative?

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.

IIRC, yes

Comment thread tools/kicad_rs/sample_policy.cue Outdated
Comment thread tools/kicad_rs/src/cue/map.cue Outdated
// where both the class-defined requirements and defaults are merged with the set values of
// the component
// This file depends on common.cue
// TODO: Figure out how to deal with multiple nested schematics
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.

Nested schematic support is quite important, those are already heavily used in Racklet.

Comment on lines +11 to +12
// The user-defined #Classifiers object specifies how various attributes are labels
// can be used to classify a component to belonging to a class.
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.

There's something wonky with this sentence, "attributes are labels can be used"?

Comment thread tools/kicad_rs/src/cue/policy_schema.cue Outdated
Comment thread tools/kicad_rs/src/policy.rs Outdated
Comment on lines +18 to +24
const CUE_COMMON_BYTES: &'static str = include_str!("cue/common.cue");
const CUE_MAP_BYTES: &'static str = include_str!("cue/map.cue");
const CUE_MAP_FILE: &'static str = "map.cue";
const CUE_REDUCE_BYTES: &'static str = include_str!("cue/reduce.cue");
const CUE_REDUCE_FILE: &'static str = "reduce.cue";
const CUE_POLICY_SCHEMA_BYTES: &'static str = include_str!("cue/policy_schema.cue");
const CUE_POLICY_SCHEMA_FILE: &'static str = "policy_schema.cue";
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.

TIL that one can import a file into the binary at compile time with a single macro.

@twelho
Copy link
Copy Markdown
Member

twelho commented Jun 28, 2021

This will need some rebasing now that #16 moved the kicad_rs files into a Cargo workspace.

@luxas luxas changed the title First stab at making the classifier fully-featured Implement the classifier, and validation and defaulting engine using CUE Jul 15, 2021
@luxas
Copy link
Copy Markdown
Member Author

luxas commented Jul 15, 2021

Rebased and ready to merge, pending a re-review @twelho

@luxas
Copy link
Copy Markdown
Member Author

luxas commented Jul 18, 2021

I'll merge this now to unblock @chiplet, I'll fix any feedback in follow-ups in future PRs

@luxas luxas merged commit 0cf2da8 into main Jul 18, 2021
@twelho twelho deleted the policy branch July 29, 2021 08:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants