hildogjr / KiCost

Build cost spreadsheet for a KiCad project.
MIT License
496 stars 97 forks source link

Wrong component quantity computation #455

Closed set-soft closed 3 years ago

set-soft commented 3 years ago

Current kicost from git computes quantities wrongly when the multiplier mechanism is used.

Using the following schematic, Schematic.pdf

We get the following XML where the most relevant parts are:

    <comp ref="R1">
      <value>0</value>
      <datasheet>~</datasheet>
      <fields>
        <field name="Description">RES SMD 0 OHM JUMPER 1/8W 0603</field>
        <field name="digikey#">CR0603-J/-000ELFCT-ND:  5</field>
        <field name="manf">Bourns</field>
        <field name="manf#">CR0603-J/-000ELF:5</field>
      </fields>
      <libsource lib="Device" part="R" description="Resistor"/>
      <sheetpath names="/" tstamps="/"/>
      <tstamp>6064C12F</tstamp>
    </comp>
    <comp ref="R4">
      <value>0</value>
      <datasheet>~</datasheet>
      <fields>
        <field name="Description">RES SMD 0 OHM JUMPER 1/8W 0603</field>
        <field name="digikey#">CR0603-J/-000ELFCT-ND</field>
        <field name="manf">Bourns</field>
        <field name="manf#">CR0603-J/-000ELF</field>
      </fields>
      <libsource lib="Device" part="R" description="Resistor"/>
      <sheetpath names="/" tstamps="/"/>
      <tstamp>6064EF62</tstamp>
    </comp>
    <comp ref="R5">
      <value>0</value>
      <datasheet>~</datasheet>
      <fields>
        <field name="Description">RES SMD 0 OHM JUMPER 1/8W 0603</field>
      </fields>
      <libsource lib="Device" part="R" description="Resistor"/>
      <sheetpath names="/" tstamps="/"/>
      <tstamp>6064F812</tstamp>
    </comp>
  </components>

R1, R4 and R5 are 0 ohms resistors.

This is a total of 7 resistors.

Here is the BoM generated by KiBot, it doesn't support KiCost multipliers, so we get wrong values: subparts-bom_r1.xlsx. But it helps to display all the relevant fields.

Now, running:

kicost --no_price -wi subparts.xml

we get: subparts.xlsx

Taking a look at the Qty cell for R1,R4,R5 we see =BoardQty*3 which is wrong.

I'll create a PR to fix it. But we need more test cases to ensure the patch doesn't break other stuff. Additionally, my patch will add qty pre-computation. In this way the Qty column we get from xlsx2csv has values we can verify, and not just 0.

Looking at the code I see some stuff that is quite confusing to me. The code seems to support the multiplier mechanism for manf#, which generates manf#_qty. But it also supports it for all distributors, generating distributor#_qty. The code even checks the multipliers are the same. Why this? Isn't manf#_qty enough? I understand the user can then ask to buy some ammount from one distributor and other ammount from other, but I don't see why this is related to the multiplier in the fields.

I also see the code is failing to consolidate subparts that are the same as regular parts, or even repeated subparts.

hildogjr commented 3 years ago

The reason of coexistence of manf#_qty and distributor#_qtys was because some time I had a equivalent part, if I purchase a different quantity, in different distributors. Something like:

MPN = PART1 digikey# = 2 : STOCK_DIG1 mouser# = 3 : STOCK_MO2

digikey# and mouser# are prioritized over MPN when searching in each of this distributors.

If I remember correct (I wrote this), I check if the quantity is the same just to warning on terminal.

I also see the code is failing to consolidate subparts that are the same as regular parts, or even repeated subparts.

I think is related with https://github.com/xesscorp/KiCost/issues/266. I didn't check this yet.

set-soft commented 3 years ago

I also see the code is failing to consolidate subparts that are the same as regular parts, or even repeated subparts.

I think is related with #266. I didn't check this yet.

Yes, I think is related.

BTW: propagation of manf# looks like a bad idea to me. I understand people is lazy, but assuming two components are the same just because their value is the same is a very bad idea. In fact I think things like manf# and distributor# are the only reliable grouping criteria. But this my opinion.

I'm finishing a "Subparts" filter for KiBot. This filter is intended to add better compatibility for KiCost users. After some tweaks (well a lot of tweaks) I'm generating an acceptable BoM for the schematic I mentioned before: subparts-bom.xlsx

I needed to add an optional field to provide Value replacements. Using Conn_01x06 pN/5 for a sensor looks really confusing to me.

set-soft commented 3 years ago

The reason of coexistence of manf#_qty and distributor#_qtys was because some time I had a equivalent part, if I purchase a different quantity, in different distributors. Something like:

MPN = PART1 digikey# = 2 : STOCK_DIG1 mouser# = 3 : STOCK_MO2

digikey# and mouser# are prioritized over MPN when searching in each of this distributors.

I don't fully understand it. Isn't this something you can choose from the spreadsheet? Do you have a real life example?

If I remember correct (I wrote this), I check if the quantity is the same just to warning on terminal.

Yes, the code warns about it.

hildogjr commented 3 years ago

Usually pins headers or borniers (for example purchase 2 x 2 pins bornier on one distributor and 1 x 4 pins in other).