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

exception on variant matching #101

Open
michael-betz opened this issue May 28, 2020 · 4 comments
Open

exception on variant matching #101

michael-betz opened this issue May 28, 2020 · 4 comments

Comments

@michael-betz
Copy link

I'm getting an exception when running KiBoM, using the +<VARIANT> and -<VARIANT> syntax
in the config field:

KiBOM version 1.7.1
 Output directory: '/home/michael/kicad/Marble-Mini'
 Input: /home/michael/kicad/Marble-Mini/AMC_FMC_Carrier-PcbDoc.xml
 Configuration file: /home/michael/kicad/Marble-Mini/bom.ini
 PCB variant: standalone
Traceback (most recent call last):
  File "/usr/lib/python3.8/pdb.py", line 1701, in main
    pdb._runmodule(mainpyfile)
  File "/usr/lib/python3.8/pdb.py", line 1546, in _runmodule
    self.run(code)
  File "/usr/lib/python3.8/bdb.py", line 580, in run
    exec(cmd, globals, locals)
  File "/home/michael/kicad/KiBoM/kibom/__main__.py", line 3, in <module>
    """
  File "/home/michael/kicad/KiBoM/kibom/__main__.py", line 189, in main
    result = writeVariant(input_file, output_dir, output_file, variant, pref)
  File "/home/michael/kicad/KiBoM/kibom/__main__.py", line 54, in writeVariant
    groups = net.groupComponents(components)
  File "/home/michael/kicad/KiBoM/kibom/netlist_reader.py", line 344, in groupComponents
    if g.matchComponent(c):
  File "/home/michael/kicad/KiBoM/kibom/component.py", line 503, in matchComponent
    if c == self.components[0]:
  File "/home/michael/kicad/KiBoM/kibom/component.py", line 117, in __eq__
    if self.isFixed() != other.isFixed():
  File "/home/michael/kicad/KiBoM/kibom/component.py", line 354, in isFixed
    if opt[1:].lower() == self.prefs.pcbConfig.lower():
AttributeError: 'list' object has no attribute 'lower'
Uncaught exception. Entering post mortem debugging

this patch fixes it

diff --git a/kibom/component.py b/kibom/component.py
index 2eef873..0943855 100644
--- a/kibom/component.py
+++ b/kibom/component.py
@@ -346,12 +346,12 @@ class Component():
 
         for opt in opts:
             # Options that start with '-' are explicitly removed from certain configurations
-            if opt.startswith('-') and opt[1:].lower() == self.prefs.pcbConfig.lower():
+            if opt.startswith('-') and opt[1:].lower() in self.prefs.pcbConfig:
                 result = False
                 break
             if opt.startswith("+"):
                 result = False
-                if opt[1:].lower() == self.prefs.pcbConfig.lower():
+                if opt[1:].lower() in self.prefs.pcbConfig:
                     result = True
 
         # by default, part is not fixed
@t3hcatpaw
Copy link

I've found some more potential issues in the original code and in the patch above but I'm not 100% sure about the DNC stuff because I've never come across that property before. I've already committed fixes and a new feature to a fork of your repo but would like to discuss this before submitting a PR to make sure my understanding is correct.

For what I understand so far, a part marked DNC is considered to never change in any variant, effectively meaning, 'always populate.' In that case, and according to the function name, isFixed() should return True whenever a part is indeed fixed, i.e.

  • has a value field containing a DNC keyword,
  • has an empty fit field,
  • or has a fit field containing a DNC keyword.

Is that correct?

@set-soft
Copy link
Contributor

The meaning is different: parts marked as DNC shouldn't be replaced by generic equivalents.
Is just a special mark in the BoM list to inform that this component is critical.
Look PR #106 for the correct implementation.
I introduced it in PR #84 but it got broken when self.prefs.pcbConfig changed from string to list.
It also contained stuff related with variants just because I used isFitted as template.
To make things worst the meaning is inverted isFixed was returning False for DNC components. It worked because the code used dnc=" (DNC)" if not self.isFixed() else "") (not)

@SchrodingersGat
Copy link
Owner

@t3hcatpaw has this been fixed for you now with the update in #106 ?

@set-soft
Copy link
Contributor

This should be fixed by #105
The exception was in isFixed:

  File "/home/michael/kicad/KiBoM/kibom/component.py", line 117, in __eq__
    if self.isFixed() != other.isFixed():
  File "/home/michael/kicad/KiBoM/kibom/component.py", line 354, in isFixed
    if opt[1:].lower() == self.prefs.pcbConfig.lower():

This comparisson was removed by #105

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

4 participants