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

Assembly: Implement Bill Of Materials #14198

Merged
merged 2 commits into from
Jun 17, 2024
Merged

Conversation

PaddleStroke
Copy link
Contributor

@PaddleStroke PaddleStroke commented May 21, 2024

Implement BOM object / tool. Fixes #12456

image

@github-actions github-actions bot added the WB Assembly Related to the Integrated Assembly Workbench label May 21, 2024
@PaddleStroke
Copy link
Contributor Author

@wwmayer I have a technical issue here, perhaps you know what is happening?
I am trying to make a subclass of Spreadsheet::Sheet object called Bom. This is working fine.
My problem is that I can't subclass SpreadsheetGui::ViewProviderSheet. It is giving me the build errors :

In file included from /usr/include/x86_64-linux-gnu/qt5/QtCore/QPointer:1,
from /home/runner/work/FreeCAD/FreeCAD/src/Mod/Spreadsheet/Gui/ViewProviderSpreadsheet.h:27,
from /home/runner/work/FreeCAD/FreeCAD/src/Mod/Assembly/Gui/ViewProviderBom.h:31,
from /home/runner/work/FreeCAD/FreeCAD/src/Mod/Assembly/Gui/AppAssemblyGui.cpp:30:
/usr/include/x86_64-linux-gnu/qt5/QtCore/qpointer.h: In instantiation of ‘T* QPointer::data() const [with T = SpreadsheetGui::SheetView]’:
/usr/include/x86_64-linux-gnu/qt5/QtCore/qpointer.h:92:14: required from ‘QPointer::operator T*() const [with T = SpreadsheetGui::SheetView]’
/home/runner/work/FreeCAD/FreeCAD/src/Mod/Spreadsheet/Gui/ViewProviderSpreadsheet.h:82:16: required from here
/usr/include/x86_64-linux-gnu/qt5/QtCore/qpointer.h:86:14: error: invalid static_cast from type ‘QObject*’ to type ‘SpreadsheetGui::SheetView*’
86 | { return static_cast<T*>( wp.data()); }
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~

The ViewProviderBom.h is very minimal.
Apparantly it's the line QPointer<SheetView> view;
in ViewProviderSpreadsheet.h that is causing the issue. I'm not quite sure why this is giving me an error now and not in ViewProviderSpreadsheet as SheetView is only declared as class SheetView in this file.

Any help would be great.

@PaddleStroke
Copy link
Contributor Author

PaddleStroke commented May 22, 2024

Note:
adding
#include <Mod/Spreadsheet/Gui/SpreadsheetView.h>
before
#include <Mod/Spreadsheet/Gui/ViewProviderSpreadsheet.h>
in ViewProviderBom.h solve the build issue. But it feels like this header should not be here.

In AppSpreadsheetGui.cpp SpreadsheetView.h is included before ViewProviderSpreadsheet.h, so I guess its why there is no build errors on main.

@wwmayer
Copy link
Contributor

wwmayer commented May 22, 2024

I have a technical issue here, perhaps you know what is happening?

QPointer requires the full class declaration of SheetView and a forward declaration is not sufficient. This is because internally it performs a static cast of QObject to SheetView and without knowing the class declaration this isn't possible.

So, the problem is basically caused by ViewProviderSpreadsheet.h because it's not self-contained i.e. it doesn't include all headers it needs. If you look at the source files that include ViewProviderSpreadsheet.h then all of them include SpreadsheetView.h beforehand.

I will prepare a PR to include SpreadsheetView.h inside ViewProviderSpreadsheet.h.

@wwmayer
Copy link
Contributor

wwmayer commented May 22, 2024

#14205

@PaddleStroke
Copy link
Contributor Author

Thanks @wwmayer !
Btw I have another issue that keeps me running in circle :

File "\FC build\Mod\Assembly\CommandCreateBom.py", line 131, in createBomObject
self.bomObj = bom_group.newObject("Assembly::BomObject", "Bill of Materials")

'Assembly::BomObject' is not a document object type

I have compared very closely BomObject to others classes such as BomGroup that works correctly, and I cannot find any difference. The only difference is that BomObject inherit from Sheet. So I wonder if there's not another hidden problem in spreadsheet (or perhaps I'm just blind on something obvious).

@PaddleStroke
Copy link
Contributor Author

I have tried to change my BomObject so that it inherits from something else (App::DocumentObjectGroup) and it works.

So it appears that the problem comes from inheriting Spreadsheet::Sheet

@wwmayer
Copy link
Contributor

wwmayer commented May 22, 2024

It's the Assembly module to blame. It depends on Spreadsheet / SpreadsheetGui but doesn't tell it to the Python interpreter about this dependency and thus the creation of the BomObject fails because the TypeId of its parent is not registered yet.

To fix that you have to add this line to PyMOD_INIT_FUNC(AssemblyApp):
Base::Interpreter().runString("import Spreadsheet");

And this block to PyMOD_INIT_FUNC(AssemblyGui):

    // load dependent module
    try {
        Base::Interpreter().runString("import SpreadsheetGui");
    }
    catch (const Base::Exception& e) {
        PyErr_SetString(PyExc_ImportError, e.what());
        PyMOD_Return(nullptr);
    }

Then the registration of ViewProviderBom is incorrect, too. It must be PROPERTY_SOURCE(AssemblyGui::ViewProviderBom, SpreadsheetGui::ViewProviderSheet)

@PaddleStroke
Copy link
Contributor Author

Thanks very much !

@PaddleStroke
Copy link
Contributor Author

@maxwxyz @pierreporte @FreeCAD/design-working-group If you guys could give me a feedback on this first implementation?

I have a feeling that custom columns should be able to be populated from objects properties. For instance you call a column "Reference" then if the object has a Reference property, it populate the value in the bom.

Also the tool could be more powerful if we enable some kind of filtering. I am just not sure filtering on what. This needs discussion.

@pierreporte
Copy link

If you guys could give me a feedback on this first implementation?

Did you take my comments in #12456 into account? If so, it will be on the right tracks.

I have a feeling that custom columns should be able to be populated from objects properties. For instance you call a column "Reference" then if the object has a Reference property, it populate the value in the bom.

Yes exactly. Usually, it’s line number, part number, part name, material, and of course quantity. Of course it depends on what is needed by the user. The needed features are not there yet though.

Also the tool could be more powerful if we enable some kind of filtering. I am just not sure filtering on what. This needs discussion.

There is actually two kind of BOMS. The first one is for the drawings, basically the table of parts visible on the set of pages the drawing is made of, used for balloon annotations containing the line numbers. The second one is for the assembly workbench and is quite different—in fact I think that it’s not really a BOM but rather a summary of the assembly with all the properties available, some computed like mass. Filtering may be needed (say to display only the parts that weight more than 1 kg) but also sorting (by part number, by name, by mass, you get it).

Not sure if BOM objects are needed.

A test is really necessary. I’ll have to try again building FreeCAD. Hopefully it’s going to work this time.

@prokoudine
Copy link
Contributor

Also the tool could be more powerful if we enable some kind of filtering. I am just not sure filtering on what. This needs discussion.

As far as I can tell, when you create a service BOM for a technician, you don't need the entire product's BOM, you need just the part that concerns the repair. That would be one example of filtering?

@prokoudine
Copy link
Contributor

prokoudine commented Jun 3, 2024

  1. Open the bundled backhoe example
  2. Start the BOM tool
  3. Use default settings, create a BOM
  4. Add some text to descriptions
  5. Double-click the BOM object
  6. Enable the 'Only Parts' checkbox, click OK
  7. Since there are no parts, only bodies, nothing is in the spreadsheet anymore
  8. Double-click the object, disable the 'Only Parts' checkbox, click OK
  9. All custom descriptions are lost

@PaddleStroke
Copy link
Contributor Author

Well this would be expected since you are emptying the bom first. This is clearing all the custom data.

@PaddleStroke
Copy link
Contributor Author

One possibility would be that custom columns creates properties in the objects.
So if you create a column 'Part Number' and fill in some information. Then it creates a 'Part Number' property in the objects of the bom and fill the entered data.

However this approach would mean that the bom tool would be modifying external files.

@pierreporte
Copy link

pierreporte commented Jun 4, 2024

One possibility would be that custom columns creates properties in the objects.

I think it should be the other way around. This feature shouldn’t change parts. A very common custom column is a number label for annotations: users don’t want this as a file attribute, if only because a part can be inserted in more than one assembly and have a different label in each.

BTW I’m still unable to build FreeCAD despite following common methods. Always getting fatal errors related to libraries, even with the Conda method.

@prokoudine
Copy link
Contributor

Okay, so... please help me out. The hierarchy is:

Assembly
  Sub-assembly
    Part
      Body

Detail sub-assemblies: on -- will list parts inside a sub-assembly, but not bodies; off - will only make a list of sub-assemblies.

Detail parts: on - will list sub-assemblies (even if 'Detail sub-assemblies' is off), parts, and bodies inside parts; off - will only list sub-assemblies and parts?

Only parts: on - will only list parts, but not sub-assemblies or bodies? off - will do nothing at all, unless other checkboxes are on?

If the above is correct, then I have two questions:

  1. Isn't there some mutual exclusivity between "Detail parts" and "Only parts"? Shouldn't they be radiobuttons?
  2. How does one get a flat list of bodies, e.g. if nuts and bolts are inserted as bodies?

@PaddleStroke
Copy link
Contributor Author

That's not it.

Detail sub-assemblies: on -- will list what is in sub-assemblies: bodies, part whatever
off - will only add an entry for the sub assembly itself

Detail parts: on - will list is in parts: bodies, part whatever
off - will only add an entry for the part itself

Only parts: do not add solids as entries (partdesign bodies etc). Only Parts.

@pierreporte
Copy link

pierreporte commented Jun 6, 2024

Here are the common BOM modes in the industry:

  • single level BOM: list of direct children of the root assembly;
  • multi-level BOM: list of direct children of the root assembly, children sub-assemblies are themselves followed by their multi-level BOM (think a tree view) with an explicit hierarchy;
  • multi-level component list: flattened multi-level BOM, duplicates are merged and their quantity added.

I don’t think that bodies should appear in BOMs. Only parts and sub-assemblies should be entitled to. But the problem is that the Part/Body dichotomy hasn’t been settled yet. In the industry, a part made of multiple bodies (a bolt and nut for example) is considered an atomic component.

@prokoudine
Copy link
Contributor

Here is an extremely quick test: insert two M6s with fasteners, then generate BOM.

Assembly's BOM tool:

image

Fasteners' own BOM tool:

image

I'd say the Fasteners' BOM tool does a better job here: better naming for the component, better counting.

@pierreporte
Copy link

pierreporte commented Jun 6, 2024

@prokoudine These problems are linked to #12139 and #12136.

I would even say that the BOM feature needs #12139 and #12136 to be solved first, and the latter needs #12141 to be fixed as well.

@PaddleStroke
Copy link
Contributor Author

My idea with the 'detail part' and 'detail sub-assemblies' is that it let you select exactly what you want to detail. So if you want 'single level' as you described, you uncheck both.

Now if you select 'detail sub-assemblies' it will do the same as what you describe in 'multi-level BOM'

For 'Detail part' the idea was to enable user to detail part content. Say you have something like :
PartA

  • sPart1
  • sPart2

So you can want either the PartA to appear. Or the sub parts as well.

And the 'only part' option is because lot of FC users are still using bodies directly in assemblies. Perhaps there should be a more general setting in assembly 'Work only with Parts' and this setting will be used in all tools (insert component, bom, perhaps others)

@PaddleStroke
Copy link
Contributor Author

@prokoudine you created 2 different solids, which while identical are distinct objects in the document.

If you create only 1, then create a link of it to have 2, then the BOM will find the quantity correctly.

@pierreporte
Copy link

pierreporte commented Jun 7, 2024

My idea with the 'detail part' and 'detail sub-assemblies' is that it let you select exactly what you want to detail. So if you want 'single level' as you described, you uncheck both.

Now if you select 'detail sub-assemblies' it will do the same as what you describe in 'multi-level BOM'

Is it possible to have better naming? The one I listed are from Windchill so actually used in the real world but they can be different. The Windchill help gives more detailed explanation about each options that could be used for tooltips:

image

What you want from this list is:

  • Multi-level components list
  • Single-level consolidated BOM
  • Multi-level BOM

The normal single level BOM could be added as well but, from experience, it’s rarely used. Replacements are PDM thing.

For 'Detail part' the idea was to enable user to detail part content. Say you have something like : PartA

* sPart1

* sPart2

So you can want either the PartA to appear. Or the sub parts as well.

And the 'only part' option is because lot of FC users are still using bodies directly in assemblies. Perhaps there should be a more general setting in assembly 'Work only with Parts' and this setting will be used in all tools (insert component, bom, perhaps others)

All this is the problem of file structure and Part/Body dichotomy. There should be first a real discussion about this subject to determine how FreeCAD should work. It is one of the rare cases where too much flexibility is detrimental. Though I can understand listing bodies in BOMs (even though I disagree), the concept of sub-parts is completely alien to me. I briefly discussed the subject with @sliptonic last month. It would have been better to settle this before 1.0 but it’s too late.

@prokoudine
Copy link
Contributor

@prokoudine you created 2 different solids, which while identical are distinct objects in the document.

If you create only 1, then create a link of it to have 2, then the BOM will find the quantity correctly.

That's not the desirable workflow though? I mean, you select multiple holes, click the right button in Fasteners, and screws get added. That's how people expect this to work, more or less. Not making links manually. What does the BOM tool in Fasteners do differently to handle objects the expected way?

@PaddleStroke
Copy link
Contributor Author

If fastener is inserting several different solid then it is a problem by itself. Fasterner tool should be smart enough to make only one object then links to it. Because if you have 100 times the same screw then it's extremly inefficient.
I guess the fastener BOM tool is checking a 'type' properties of the fasteners, and if the types matches then it adds them.

@pierreporte
Copy link

I think he meant that all selected should have a bolt in them. Two holes = two screws but only one screw per hole.

@prokoudine
Copy link
Contributor

A few more thoughts:

It's possible to reorder columns in the task panel and add new ones. But it seems to be impossible to remove columns there. I expect that some columns should not be removable, e.g. Name and Quantity, but e.g. File Name and any custom columns you add should be removable.

Should the tool stick with more standard nomenclature for types of BOMs? I could look it up in different applications for you, if you like.

@PaddleStroke
Copy link
Contributor Author

You can select an item and press delete that will delete the column.

@pierreporte
Copy link

The new assembly workbench doesn’t support the arrays of components (polar arrays for instance). It will be asked and eventually implemented. Maybe the architecture of the BOM feature should take this into account in preparation.

@prokoudine
Copy link
Contributor

prokoudine commented Jun 14, 2024

You can select an item and press delete that will delete the column.

Cool! There's no indication of that in the UI, unfortunately. Maybe the tooltip could be updated as a minimum effort change? E.g. 'Colums of the bill of materials, click 'Add column' below to create a new one, select an existing column and press Del to delete one'. Something along those lines?

Also, is there a way to rename a column? Let's say I created a column named 'Price', but I realized there's actually price per unit and total price, so I want to rename one and add another? Or do I need to remove a column (and thus lose whatever I already got there) and create new ones named correctly?

@chennes chennes merged commit 3fa0b68 into FreeCAD:main Jun 17, 2024
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WB Assembly Related to the Integrated Assembly Workbench
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Problem] Assembly: BOM export
5 participants