Add support for variants of the Value field

[eeintech]

Current KiBOM code supports loading/unloading a part depending on the exported variant.
However, it does not support changing the value of Value field to support different component values in different variants.

This is a straight forward implementation to handle this, so no hurt feeling if you see potential flaws 😄
Happy to resolve feedback as necessary.

[set-soft]

Hi @eeintech !
What about a more generic approach? We could replace "VARIANT:FIELD" by "FIELD" when "VARIANT" is matched. I like the ":" separator, but I think KiCad 7 will internally use "." as separator. So this could be configurable.
Do you have any real world example to share (to use it as testbed)?

[eeintech]

@set-soft I'm a little confused by your proposal, do you mean controlling the format of the board_variant = ['default'] configuration option in the bom.ini file? Separator isn't used anywhere in my approach, I am keeping the same format for board_variant as I believe it was implemented.

For the test part and example, do you want me to add this to the test/kibom-test KiCad project?

[set-soft]

Sorry if I wasn't clear @eeintech
Here is a better explanation:

Your solution only changes the Value of the component. What about other fields?
As an example: My schematics contain the Digi-key part number for each component. I use it to make clear which component is, even if the component comes from other provider. So changing the value for some variant isn't enough. The Digi-key part number must be changed according to it.
A bizarre cases: What if we want to also change the footprint?

KiCad 6 changes the schematic file format. In the new file format this problem is addressed. Even when KiCad 6 won't support variants, we don't even know if KiCad 7 will do. But the format was designed to support field names like this VARIANT.FIELD. So you could have Production.Value to change the Value field used for the Production variant.

What I say is why not implementing it in a similar way. I think KiCad 7 will hide the variant part to the user, showing only the filed name when the user selects the Production variant. But we could use this mechanism as an approach that will be easy to migrate in the future.

I personally think that : is a better separator. This is because : is used by KiCad to separate the library from the component. In any case the separator should be configurable.

About the example: I'm just asking if you have any real life example that can be used for tests. Not a regression test for KiBoM, just some schematic to play with the concept. We can create some artificial test case. But most bugs pops up in real life examples ;-)

I maintain a tool called KiBot. KiBot can generate BoMs using KiBoM or an internal BoM generator. Today I committed an implementation of what I'm describing: INTI-CMNB/KiBot@986f0c7

I added a very silly regression test to check this is working. The configuration file is:

# Example KiBot config file
kibot:
  version: 1

variants:
  - name: 'production'
    comment: 'Production variant'
    type: kibom
    file_id: '_(production)'
    variant: production

  - name: 'test'
    comment: 'Test variant'
    type: kibom
    file_id: '_(test)'
    variant: test
    field_changer: true
    field_variant_separator: ':'

outputs:
  - name: 'bom_internal'
    comment: "Bill of Materials in CSV format"
    type: bom
    dir: BoM

  - name: 'bom_internal_production'
    comment: "Bill of Materials in CSV format for production"
    type: bom
    dir: BoM
    options:
      variant: production

  - name: 'bom_internal_test'
    comment: "Bill of Materials in CSV format for test"
    type: bom
    dir: BoM
    options:
      variant: test

This generates 3 BoM files for the default, production and test variants. The test variant is enabling the field changer using the : as separator.

Component R1 is a 1k resistor, but contains a field named test:Value with a value of 3k3. So the test variant generates the following BoM:

Row,Description,Part,References,Value,Footprint,Quantity Per PCB,Datasheet,Config,test:Value
1,Unpolarized capacitor,C,C1 C2,1nF,,2,~,"-production,+test +production,+test",
2,Resistor,R,R1,3k3,,1,~,,3k3

With R1 different than what you get in the default variant:

Row,Description,Part,References,Value,Footprint,Quantity Per PCB,Datasheet,Config,test:Value
1,Resistor,R,R1 R2,1k,,2,~,-test,3k3

[eeintech]

@set-soft Totally understood your proposal and really like it!

I've just pushed an update which should match your expectation 😃

Here is a test project: test_variants.zip
There are two variants specified in the bom.ini file: DEV and PROD.

Note that the generated BOM files (CSV) are included so you can have a look at the result without needing to generate the BOMs yourself, feel free to play with the design and overwrite them!

For DEV we want to populate all parts, and we want R1 to be set to a lower value so that D1 can shine more.
The following fields are updated in this variant: Description, Value, Manufacturer and Manufacturer Part Number.
For the the rest of fields, we don't expect to show them in the BOM so ignore them.

For PROD we don't want to populate D2 and R2 as we don't deem them necessary.

It is a simple but real world use-case which hope satisfy what you had in mind!

EDIT: I forgot to mention that KiBOM does not like duplicate entries in the [IGNORE_COLUMNS] of the bom.ini configuration file. The DEV:XXX are raising an error if two or more are un-commented. There will be the need for a follow-up fix.

[set-soft]

I was looking at the code and the example. One thing that I'm not really sure is why you need to add another mechanism to exclude components from some variant.

I understand that the mechanism you use is the "natural" one when using VARIANT:FIELD mechanism.

But KiBoM already has a mechanism for it. Taking the example you sent: R2 and D2 has "PROD=dnp", but KiBoM is designed to use "Config=-PROD".

I'm not against having more than one mechanism, but I'm not sure about it enabled by default. Also I got the impression that the code allows using "PROD:XXX=dnp".

Again, I'm not saying this is wrong or bad. Just sounds too fuzzy.

It would be nice to know what @SchrodingersGat thinks about it.

[eeintech]

@set-soft Thanks for taking a look!

But KiBoM already has a mechanism for it. Taking the example you sent: R2 and D2 has "PROD=dnp", but KiBoM is designed to use "Config=-PROD".

You are right, I'm allowing this behavior because there is a tiny step from <VARIANT>:<FIELD>=<NEW_VALUE> to <VARIANT>=dnf and it makes a lot of more sense to visually than seeing <CONFIG>=-<VARIANT> where <CONFIG> needs to also be set in the bom.ini, instead of just relying on the variant names already pre-set. We have up to 3-4 variants in our designs and seeing a list of variants fields vs one big field with some + and - signs is really a game changer!

I'm not against having more than one mechanism, but I'm not sure about it enabled by default.

Sounds safe to disable it by default. Do you mind if I add a bom.ini variable to enable it?

Also I got the impression that the code allows using "PROD:XXX=dnp".

I'm going to fix that immediately 👍

[set-soft]

I'm merging your patches with my fork to check some details.
The first problem I'm experimenting is with this code:

        # Check if variants were defined in configuration
        try:
            variants = pref.pcbConfig
        except:
            variants = [None]

Using it I'm getting ["default"] when no .ini is used. It makes the output file name to contain _(default) which isn't a good thing.

I think you can't copy pref.pcbConfig into variants. The idea is that pref.pcbConfig is an implicit variant. Doing it you make it explicit.

Also: the result of indicating more than one variant in the bom.ini is not the same as indicating the same list in the command line.

The command line option uses the variants one by one, the board_variant option of the .ini joins the variants. So I think this part of the patch must be removed.

[eeintech]

It makes the output file name to contain _(default) which isn't a good thing.

Why isn't this a "good thing"? One of my colleagues requested that we always append the variant, but I have to admit this was unintentional.

The command line option uses the variants one by one, the board_variant option of the .ini joins the variants. So I think this part of the patch must be removed.

I was under the impression that a list of variants in the bom.ini file wouldn't be processed without changing this part of the code, or at least it did not in my test. I'll have to double check again.

[set-soft]

Hi @eeintech !

It makes the output file name to contain _(default) which isn't a good thing.

Why isn't this a "good thing"? One of my colleagues requested that we always append the variant, but I have to admit this was unintentional.

Over than 90% of people doesn't even know about variants. IMHO the options are:

1. Keep the current behavior: variants are included in the name only when explicitly indicated in the command line.
2. We change the default name template to exclude the %V. So people wanting to have the variant in the name does it explicitly.
3. Detect the default case as something special and avoid expanding %V when the default variant spec comes from the bom.ini. In your patch this could be achieved checking for preferences.pcbConfig equal to ['default'] before copying it to variants.

I think @SchrodingersGat should decide what to do. Changing the default behavior is not a good idea, this is what I mean.

The command line option uses the variants one by one, the board_variant option of the .ini joins the variants. So I think this part of the patch must be removed.

I was under the impression that a list of variants in the bom.ini file wouldn't be processed without changing this part of the code, or at least it did not in my test. I'll have to double check again.

Nope. In fact all the code makes reference to preferences.pcbConfig. Take a look at your own patch:

if variant_name.lower() in preferences.pcbConfig:

The command line option is copied at the beginning of writeVariant using:

    if variant is not None:
        preferences.pcbConfig = variant.strip().lower().split(',')

Here variant is an argument passed from main using:

    for variant in variants:
        result = writeVariant(input_file, output_dir, output_file, variant, pref)

Note that the command line option uses ; to separate the variants. But each variant can contain more than one name using , as separator.

In the bom.ini you can specify only one list of names, using , as separator. But in the command line you can specify multiple lists of names. Each list is used to generate one BoM output.

The default bom.ini defines a variant with a list of length 1: ['default']. This is just a mechanism equivalent to [None]. We could discuss if this is a good implementation, perhaps will be better to make it more similar and use None in the .ini file. But current .ini files contain board_variant = default and it means: no variants.

[set-soft]

I'm not against having more than one mechanism, but I'm not sure about it enabled by default.

Sounds safe to disable it by default. Do you mind if I add a bom.ini variable to enable it?

I think this is a good idea.

Also note that defining a field named VARIANT:Value with any of the DNF names assigned to it will exclude the component. This is because KiBoM excludes components with DNF as value.

[eeintech]

3. Detect the default case as something special and avoid expanding %V when the default variant spec comes from the bom.ini. In your patch this could be achieved checking for preferences.pcbConfig equal to ['default'] before copying it to variants.

I'm personally (strongly) voting for this one, because that way one can control the output name with the variant list from the bom.ini file and in the command line. It would be pretty annoying for us to have it only in the command line, because you'll have to run a different command for each project. Instead one would just setup the bom.ini file for each project and keep the same command in KiCad BOM export window across all his projects.

Regarding how to declare variants, there are two ways:

• in the command line with the -r argument
• in the bom.ini

As explained above, our favorite way reason is the later so this is why I added this piece of code:

        # Check if variants were defined in configuration
        try:
            variants = pref.pcbConfig
        except:
            variants = [None]

In current master, it wouldn't load the variants from the bom.ini file.
And that's because variants are not loaded in the __main__.py file, unless there are specified in command line argument.
Therefore I wanted to change this behavior and enable control from the bom.ini file for the reason listed above (same command across multiple projects).
Do you have a better way to support this?

Note that the command line option uses ; to separate the variants. But each variant can contain more than one name using , as separator.

I do not understand this part, do you have examples? Do you mean one can run:

python KiBOM_CLI.py -r v01_a,v02_a;v01_b,v02_b "project.xml" .

What is the use-case for handling two separators?

Sounds safe to disable it by default. Do you mind if I add a bom.ini variable to enable it?

I think this is a good idea.

Ok great, I'll add this variable in the mix.

Also note that defining a field named VARIANT:Value with any of the DNF names assigned to it will exclude the component. This is because KiBoM excludes components with DNF as value.

This is a useful note, thanks for the reminder 😃

[set-soft]

Hi @eeintech !

In current master, it wouldn't load the variants from the bom.ini file.

I tried it in my fork and it works. The only problem is with the name, it doesn't contain the variant. But it works. Note that I applied your patches to my fork, but then I removed this part of the code because it makes the regression tests to fail. This is because a lot of outputs changes their name adding _(default).

And that's because variants are not loaded in the __main__.py file, unless there are specified in command line argument.

They take effect, check the preferences.pcbConfig value.

I modified one of my regression tests to verify that variants can be selected from the .ini

You can try it running:

make single_test SINGLE_TEST=test_variants_issue_SG136_production

Note that the command line option uses ; to separate the variants. But each variant can contain more than one name using , as separator.

I do not understand this part, do you have examples? Do you mean one can run:

python KiBOM_CLI.py -r v01_a,v02_a;v01_b,v02_b "project.xml" .

Yes you can.

What is the use-case for handling two separators?

The above example will generate two files:

1. For variants v01_a and v02_a enabled.
2. For variants v01_b and v02_b enabled.

[set-soft]

3. Detect the default case as something special and avoid expanding %V when the default variant spec comes from the bom.ini. In your patch this could be achieved checking for preferences.pcbConfig equal to ['default'] before copying it to variants.

I'm personally (strongly) voting for this one

I'm commiting a patch to my fork implementing it. Is just an adaptation of your patch:

--- a/kibom/__main__.py
+++ b/kibom/__main__.py
@@ -234,7 +234,11 @@ def main():
     if args.variant is not None:
         variants = args.variant.split(';')
     else:
-        variants = [None]
+        # Check if variants were defined in configuration
+        if pref.pcbConfig != ['default']:
+            variants = pref.pcbConfig
+        else:
+            variants = [None]
 
     # Generate BOMs for each specified variant
     for variant in variants:

BTW: here is what KiBot can do with variants: https://inti-cmnb.github.io/kibot_variants_arduprog/

[eeintech]

@set-soft Holy molly, I just went through the KiBot variant example page you sent, very impressive tool!

I did not grasp the power of KiBot the first time around, just looking at the GitHub page. It does look like 100 times better than this little patch for variants, because, in addition to the BOM, it manipulates schematic PDFs, fabrication layer outputs and 3D models too. And that, we would love it ❤️

Now regarding this PR, I'm mostly committed to finish the work started without disrupting any current KiBOM behavior.
Thank you for the little update to the variant handler. I'm mostly missing the variable in the bom.ini file to enable this new variant handling behavior, I'll work on this.

Let me know if you think of anything else.

[eeintech]

@set-soft I integrated your change into this PR and added the preference option for enabling this new behavior.

Besides the README, does this bom.ini template file need to be updated?
https://github.com/SchrodingersGat/KiBoM/blob/master/test/bom.ini

[set-soft]

Besides the README, does this bom.ini template file need to be updated?
https://github.com/SchrodingersGat/KiBoM/blob/master/test/bom.ini

I think updating it is not necesary.

Now we need @SchrodingersGat review.

[eeintech]

Awesome 😄

Let's give Oliver some time, he's a busy person!

Merry Christmas!

[set-soft]

I found a small missing detail (while incorporating the last changes to my fork).
You should add the new option to the code that generates a default bom.ini.
Take a look at the last chunk of this patch:

--- a/kibom/preferences.py
+++ b/kibom/preferences.py
@@ -42,6 +42,7 @@ class BomPref:
     OPT_DEFAULT_BOARDS = "number_boards"
     OPT_DEFAULT_PCBCONFIG = "board_variant"
     OPT_CONFIG_FIELD = "fit_field"
+    OPT_COMPLEX_VARIANT = "complex_variant"
     OPT_HIDE_HEADERS = "hide_headers"
     OPT_HIDE_PCB_INFO = "hide_pcb_info"
     OPT_DATASHEET_AS_LINK = "datasheet_as_link"  # (#112)
@@ -70,6 +71,7 @@ class BomPref:
         self.hidePcbInfo = False
         self.configField = "Config"  # Default field used for part fitting config
         self.pcbConfig = ["default"]
+        self.complexVariant = False  # To enable complex variant processing
         self.refSeparator = " "
 
         self.backup = "%O.tmp"  # If no .INI file is provided we create *.tmp back-ups
@@ -172,6 +174,7 @@ class BomPref:
             self.hideHeaders = self.checkOption(self.OPT_HIDE_HEADERS, default=False)
             self.hidePcbInfo = self.checkOption(self.OPT_HIDE_PCB_INFO, default=False)
             self.configField = self.checkStr(self.OPT_CONFIG_FIELD, default=self.configField).lower()
+            self.complexVariant = self.checkOption(self.OPT_COMPLEX_VARIANT, default=False)
             self.boards = self.checkInt(self.OPT_DEFAULT_BOARDS, default=1)
             self.refSeparator = self.checkStr(self.OPT_REF_SEPARATOR, default=self.refSeparator).strip("\'\"")
             self.pcbConfig = self.checkStr(self.OPT_DEFAULT_PCBCONFIG, default=self.pcbConfig[0])
@@ -269,6 +272,7 @@ class BomPref:
         cf.set(self.SECTION_GENERAL, '; Default PCB variant if none given on CLI with -r')
         cf.set(self.SECTION_GENERAL, self.OPT_DEFAULT_PCBCONFIG, ','.join(self.pcbConfig))
 
+        self.addOption(cf, self.OPT_COMPLEX_VARIANT, self.complexVariant, comment="If '{opt}' option is set to 1, the complex variant field processing is enabled".format(opt=self.OPT_COMPLEX_VARIANT))
         self.addOption(cf, self.OPT_HIDE_HEADERS, self.hideHeaders, comment="If '{opt}' option is set to 1, column headers aren't included in the output file".format(opt=self.OPT_HIDE_HEADERS))
         self.addOption(cf, self.OPT_HIDE_PCB_INFO, self.hidePcbInfo, comment="If '{opt}' option is set to 1, PCB info isn't included in the output file".format(opt=self.OPT_HIDE_PCB_INFO))

Merry Christmas!

[eeintech]

@set-soft Sorry for the late response, I've just added the missing piece 😃

Happy new year!

[eeintech]

Seems like the Travis build passed but GitHub hasn't updated the status...

[SchrodingersGat]

@eeintech @set-soft the build has actually passed, not sure why the status is frozen.

In any case, I've finally had some time after holidays, work, etc, to have a look at this. I think it is a really clever implementation.

Is there anything on your end stopping me from merging this in now?

[eeintech]

@SchrodingersGat Green light on my side!

@set-soft Is there any chance KiBot can get the same feature for variant management? You mentioned it earlier, not sure if you actually pulled it to master. My question is: can schematic PDF also be showing different component values depending on the variant?