Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: Extra field without affecting grouping #268

Closed
timoscs opened this issue Aug 25, 2021 · 11 comments
Closed

Feature request: Extra field without affecting grouping #268

timoscs opened this issue Aug 25, 2021 · 11 comments

Comments

@timoscs
Copy link

timoscs commented Aug 25, 2021

It would be useful to be able to add an extra field without it affecting the grouping.

The problem with the way it currently works:

  1. make a Kicad-project which has, say, 14 100n 0603 capacitors. Add a custom field, manf# to precisely one of these, to specify the exact part number. Our production BOM export script then propagates that manf# to the other 100n caps (grouped by equal component name, footprint, and value, but crucially not by the manf#)
  2. make iBom for hand-assembling the prototype, including manf# in the Extra fields. Now there is one row with 13 100n caps which have an empty manf# field, and one row which has the one remaining 100n cap, with the manf#.

Of course, we can just not add the manf# to Extra fields, but then we have to double check the actual part number from somewhere else. Most of the time, footprint + value make that possible to deduce uniquely, but in the end that still generates a small extra chance for mistakes.

@qu1ck
Copy link
Member

qu1ck commented Aug 25, 2021

It's super easy to copy that manf# to all caps in eeschema in Tools -> Edit symbol fields dialog. If you copy paste value to a group it will be set on all components in the group.

@timoscs
Copy link
Author

timoscs commented Aug 26, 2021

That's true, but I still need to remember to copy them again if I, for example, add a new cap or change the part number (happens often at the final stages of editing a schematic), so it's a bit more cumbersome, and a tiny bit more error prone, than to autopropagate using a script as we do now. This is the workflow we've picked up from KiCost, although we're not using that anymore.

I can paste here how we implement something similar in our production BOM export script, but I won't have time to dig that up until a bit later.

@timoscs
Copy link
Author

timoscs commented Aug 26, 2021

Here's the code we're using, it's really nothing but using a custom comparator for the grouping, kicad_netlist_reader then fishes out the first non-empty field (admittedly, some error checking should be done here, i.e. if there's more than one non-empty non-equal value, at least the user should be notified). I'm not sure if this is any help at all, but since I said I'd post it:

import kicad_netlist_reader

def myEqu(self, other):
    """myEqu is a more advanced equivalence function for components which is
    used by component grouping. Normal operation is to group components based
    on their value and footprint.

    In this example of a custom equivalency operator we compare the
    value, the part name and the footprint.
    """
    result = True
    if self.getValue() != other.getValue():
        result = False
    elif self.getPartName() != other.getPartName():
        result = False
    elif self.getFootprint() != other.getFootprint():
        result = False

    return result

# Override the component equivalence operator - it is important to do this
# before loading the netlist, otherwise all components will have the original
# equivalency operator.
kicad_netlist_reader.comp.__eq__ = myEqu

...much later....

# Output component information organized by group, aka as collated:
item = 0
for group in grouped:
    del row[:]
    refs = ""
    sheets = ""

    # Add the reference of every component in the group and keep a reference
    # to the component so that the other data can be filled in once per group
    for component in group:
        if len(refs) > 0:
            refs += ", "
        refs += component.getRef()
        elemsheet = component.element.get("sheetpath", "names")[1:-1]
        if not elemsheet in sheets:
            if len(sheets) > 0:
                sheets += ", "
            sheets += elemsheet
        c = component

    # Fill in the component groups common data
    # columns = ['Item', 'Qty', 'Reference(s)', 'Value', 'LibPart', 'Footprint', 'Datasheet', 'Sheets'] + sorted(list(columnset))
    item += 1
    row.append( item )
    row.append( len(group) )
    row.append( refs );
    row.append( c.getValue() )
    row.append( c.getLibName() + ":" + c.getPartName() )
    row.append( net.getGroupFootprint(group) )
    row.append( net.getGroupDatasheet(group) )

    row.append( sheets );

    # from column 8 upwards, use the fieldnames to grab the data
    for field in columns[8:]:
        row.append( net.getGroupField(group, field) );

    writerow( out, row  )

where getGroupField from kicad_netlist_reader.py is:

def getGroupField(self, group, field):
        """Return the whatever is known about the given field by consulting each
        component in the group.  If any of them know something about the property/field,
        then return that first non-blank value.
        """
        for c in group:
            ret = c.getField(field, False)
            if ret != '':
                return ret

        libpart = group[0].getLibPart()
        if not libpart:
            return ''

        return libpart.getField(field)

@qu1ck
Copy link
Member

qu1ck commented Aug 26, 2021

I understand how it can be done but it's obviously a special case and won't work for everyone. Which means it needs an option and there are already lots of options in the plugin. I am hesitant to add something for a feature that has easy workaround.

@timoscs
Copy link
Author

timoscs commented Aug 26, 2021

I get your point, and of course this would need to be optional... how about, one more checkbox under the "Normalize field name case" checkbox, say "Group by extra fields", defaults to checked so the current behavior remains default? That's probably the minimal UI disruption possible?

I could even try to make a fork and implement that myself, you can then choose if you want to merge it back to the mainline?

@qu1ck
Copy link
Member

qu1ck commented Aug 26, 2021

I won't merge that because it's too specific for a narrow use case. What I would merge is a system where any field can be chosen to be present in bom and affect grouping or not. This ties into #167.
Also UI needs to be considered when a field that doesn't affect grouping is collapsed and there are different (or populated and empty) values there. It needs some indication.

@timoscs
Copy link
Author

timoscs commented Aug 26, 2021

Agreed on the need for indication.

My other though was indeed to add a checkbox for each row in "Extra fields", called "Group by" or something similar, but I actually expected you'd be less enthusiastic about that since it adds more clutter. The most obvious way to extend that to any field would be to change the "Extra fields" list to "Fields", and just show all fields with the usual ones selected by default.

Implementing that does seem like a slightly bigger task, I've only taken a quick look at the code but this looks like the needed changes would be to "config.py" to save the selections, then the UI changes to the dialog box, and finally the implementation in "ibom.py"?

@timoscs
Copy link
Author

timoscs commented Aug 26, 2021

...and the UI case seems to be the most difficult of these, as adding several checkboxes is apparently not possible with CheckListBox, so it would require replacing the widget with something else (although google does find a few existing options in wx for multiple checkboxes per list item, I have no idea if they'd be trivial to integrate here or not).

@qu1ck
Copy link
Member

qu1ck commented Aug 26, 2021

You can use wxGrid with some columns set to boolean renderer which draws a checkbox.

The most obvious way to extend that to any field would be to change the "Extra fields" list to "Fields"

Yeah that was the intention.

In addition to changes you mentioned also likely javascript code will need tweaks as it expects some fields to be there right now.

@timoscs
Copy link
Author

timoscs commented Aug 26, 2021

Okay, unfortunately that's probably a bit more work, I'm guessing a few days since I'm not an expert in wx or webdevelopment, than I can take on myself right now.

If you ever get around to implementing something like you described, I'll be very interested, but in the meantime I already implemented the simple non-grouping (without options, just hardcoded, but with warning for conflicting fields) for myself.

Thanks for your time, though!

@qu1ck
Copy link
Member

qu1ck commented Aug 29, 2021

No problem, I'll close this in favor of #167 .

@qu1ck qu1ck closed this as completed Aug 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants