PCBnew BOM Creation

Bug #1787329 reported by Martin Thomas
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
KiCad
Fix Released
Unknown

Bug Description

The BOM created in PCBnew gives a unsortet List of references. I created a patch which creates a list, where the references are sorted.
If someone extend the gui element with a selection between:
 - complete BOM
 - BOM distinguished by layer

Currently I codes it hard to List 1.

I attached the patch also here (which makes no sens):
https://bugs.launchpad.net/kicad/+bug/1464805

Revision history for this message
Martin Thomas (mtlaunchpad) wrote :
Revision history for this message
Martin Thomas (mtlaunchpad) wrote :

Ich changed the patch so now It contains only the patch without my git history.

Revision history for this message
Wayne Stambaugh (stambaughw) wrote :

@Martin, some comments on your patch.

Please reformat your patch per the KiCad coding policy http://docs.kicad-pcb.org/doxygen/md_Documentation_development_coding-style-policy.html. I will not be accepted with the current formatting.

You need to add the bug report links to your commit message per http://docs.kicad-pcb.org/doxygen/commit_messages.html so the get closed when/if the patch gets merged.

Remove all debugging statements that do not use wxLogTrace() for debugging output.

You added the board side to the BOM output which may break the expected output for existing users so I'm not sure this is a good idea. Ideally I would prefer that we use an intermediate xml file like we do with the schematic BOM generator so we can customize the BOM output information using xsltproc or python.

I noticed you posted the same patch here https://bugs.launchpad.net/kicad/+bug/1464805. You probably should just provide a link if the patch fixes both bugs. Posting the same patch twice is extra work for the lead developers to determine you intent.

For new source files the license should be GPL3+ not GPL2+.

Revision history for this message
Martin Thomas (mtlaunchpad) wrote :

@Wayne

I changed the patch and tried to get it more in the KiCad Coding style.
I also coded hard that the old style BOM is generated now with the former order. Thus it should not break any user space.

Revision history for this message
Martin Thomas (mtlaunchpad) wrote :

I changed the GPL2 to GPL3

Revision history for this message
Wayne Stambaugh (stambaughw) wrote :

@Martin, I tested your patch but AFAICT it does not sort items alphabetically by reference. It does some sorting but not what I expected. What is the end goal with this patch? There are still have a lot coding policy violations in your patch that need to resolved before it can be merged.

Revision history for this message
Martin Thomas (mtlaunchpad) wrote :

@ Wayne, you are right there was a bug in List 2 (as I only use list 1 for my purposes)

The difference between the current List and the new list is:

Current List:
"Id";"Designator";"Package";"Quantity";"Designation";"Supplier and ref";
1;"SYM2";"LOGO";1;"LOGO_CE";;;
2;"J2";"PSS254_2G";1;"CH6 out";;;
3;"C14,C49,C44";"CP_Elec_6.3x7.7";3;"100µ";;;
...
21;"R34,R29,R30,R31,R32,R36,R39,R45,R52,R57,R58,R60,R62,R63,R44,R56,R4,R38,R3,R24,R64,R23,R22,R18,R17,R16,R15,R6,R5,R20";"R_0805_2012Metric";30;"5,6k";;;

New List:
Id;Designator;Package;Quantity;Designation;Supplier and ref
1;"C23,C24,C28,C29,C34,C35,C43,C48,C54,C55,C59,C60";"CP_Elec_4x5.8";12;"22µ";;
2;"C14,C44,C49";"CP_Elec_6.3x7.7";3;"100µ";;
3;"C19,C20,C21,C22,C30,C31,C32,C33,C50,C51,C52,C53";"C_0805_2012Metric";12;"2,7n";;
...
19;"R3,R4,R5,R6,R15,R16,R17,R18,R20,R22,R23,R24,R29,R30,R31,R32,R34,R36,R38,R39,R44,R45,R52,R56,R57,R58,R60,R62,R63,R64";"R_0805_2012Metric";30;"5,6k";;

The current list is sorted by Footprint first, Value second and Layer third. Also the Reference list is sorted naturally.

Revision history for this message
Wayne Stambaugh (stambaughw) wrote :

You still need to fix your coding policy violations of which there are too many to list. The easiest solution would be to use uncrustify or clang-format with the formatting config files provided in the kicad source root folder to fix the file build_BOM_from_board_properties.h. build_BOM_from_board_properties.cpp should be fixed by hand to prevent unnecessarily large change set.

Revision history for this message
Martin Thomas (mtlaunchpad) wrote :

Hi Wayne

I hope the coding style violations are fixed. I fixed some violations in build_BOM_from_board_properties.cpp as I made changes in this file anyways.

Revision history for this message
Jeff Young (jeyjey) wrote :

Hi Martin,

I think this patch could use some simplification. ELEMENT and ELEMENTTYPE always hold module info, so why not just have ELEMENTS be type-def'd to std::vector<MODULE*>? (Or at least have ELEMENTS inherit from std::vector<MODULE*> if you want to continue to encapsulate getReference()).

I realise the old code also kept a separate data structure, but I think it was a mistake then too.

A couple of other comments:
1) the header should have the same name as its .cpp file
2) you don't need a routine to fetch a layer name; use GetBoard()->GetLayerName( layerID )
3) getReference() should be pluralised

Cheers,
Jeff.

Jeff Young (jeyjey)
Changed in kicad:
importance: Undecided → Wishlist
status: New → Triaged
milestone: none → 6.0.0-rc1
Revision history for this message
KiCad Janitor (kicad-janitor) wrote :

KiCad bug tracker has moved to Gitlab. This report is now available here: https://gitlab.com/kicad/code/kicad/-/issues/2210

Changed in kicad:
status: Triaged → Expired
Changed in kicad:
importance: Wishlist → Unknown
status: Expired → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.