devbisme / KiField

Edit/insert/delete part fields in KiCad schematics or libraries using a spreadsheet.
MIT License
70 stars 27 forks source link

[KiField Bug] Incorrectly grouping components with unrelated values during extraction #69

Closed scandey closed 3 years ago

scandey commented 3 years ago

Describe the bug When extracting from a complex circuit KiField will sometimes collect unrelated components into a line item. The issue appears to be during extraction, some components are just extracted from schematic incorrectly and silently.

To Reproduce Steps to reproduce the behavior:

  1. Run extraction on schematic with grouping turned on.
  2. Note that, for example, capacitor C6 is included in a group of 0.33uF capacitors when it should be in a group with C5 of 2pF capacitors.
  3. Note that C5 retains all the correct information for that item.
  4. Double-check that the text in the .sch file is not subtly broken, see that C5 and C6 are correctly listed as being 2pF capacitors.
  5. Note that during extraction with high debugging, C6 is extracted as if it is identical in every way to the 0.33uF capacitors elsewhere in the schematic. Other examples occur as well.

From debugging in running KiField:

> 'C5': {'Comments': '~',
        'Descriptions': '2pF 1% 100V Ceramic Capacitor 0805',
        'EM_MFR': '~',
        'EM_MFR_PN': '~',
        'EM_value': '~',
        'MFR': 'AVX',
        'MFR_PN': '~',
        'PN': 'CDR31BP2R0BBUS',
        'datasheet': '~',
        'footprint': '0805',
        'value': '2pF'},

> Exploding C5 => ['C5'].
Updating C5 field value from 2pF to 2pF
Type of C5 field value containing 2pF is s
Updating C5 field footprint from 0805 to 0805
Type of C5 field footprint containing 0805 is s
Updating C5 field datasheet from ~ to ~
Type of C5 field datasheet containing ~ is s
Updating C5 field Comments from ~ to ~
Type of C5 field Comments containing ~ is s
Updating C5 field Descriptions from 2pF 1% 100V Ceramic Capacitor 0805 to 2pF 1% 100V Ceramic Capacitor 0805
Type of C5 field Descriptions containing 2pF 1% 100V Ceramic Capacitor 0805 is s
Updating C5 field EM_MFR from ~ to ~
Type of C5 field EM_MFR containing ~ is s
Updating C5 field EM_MFR_PN from ~ to ~
Type of C5 field EM_MFR_PN containing ~ is s
Updating C5 field EM_value from ~ to ~
Type of C5 field EM_value containing ~ is s
Updating C5 field MFR from AVX to AVX
Type of C5 field MFR containing AVX is s
Updating C5 field MFR_PN from ~ to ~
Type of C5 field MFR_PN containing ~ is s
Updating C5 field PN from CDR31BP2R0BBUS to CDR31BP2R0BBUS
Type of C5 field PN containing CDR31BP2R0BBUS is s

>  'C6': {'Comments': '~',
        'Descriptions': '0.33uF 10% 50V Ceramic Capacitor 1825',
        'EM_MFR': '~',
        'EM_MFR_PN': '~',
        'EM_value': '~',
        'MFR': 'AVX',
        'MFR_PN': '~',
        'PN': 'CDR35BX334AKUS',
        'datasheet': '~',
        'footprint': '1825',
        'value': '0.33uF'},

> Exploding C6 => ['C6'].
Updating C6 field value from 0.33uF to 0.33uF
Type of C6 field value containing 0.33uF is s
Updating C6 field footprint from 1825 to 1825
Type of C6 field footprint containing 1825 is s
Updating C6 field datasheet from ~ to ~
Type of C6 field datasheet containing ~ is s
Updating C6 field Comments from ~ to ~
Type of C6 field Comments containing ~ is s
Updating C6 field Descriptions from 0.33uF 10% 50V Ceramic Capacitor 1825 to 0.33uF 10% 50V Ceramic Capacitor 1825
Type of C6 field Descriptions containing 0.33uF 10% 50V Ceramic Capacitor 1825 is s
Updating C6 field EM_MFR from ~ to ~
Type of C6 field EM_MFR containing ~ is s
Updating C6 field EM_MFR_PN from ~ to ~
Type of C6 field EM_MFR_PN containing ~ is s
Updating C6 field EM_value from ~ to ~
Type of C6 field EM_value containing ~ is s
Updating C6 field MFR from AVX to AVX
Type of C6 field MFR containing AVX is s
Updating C6 field MFR_PN from ~ to ~
Type of C6 field MFR_PN containing ~ is s
Updating C6 field PN from CDR35BX334AKUS to CDR35BX334AKUS
Type of C6 field PN containing CDR35BX334AKUS is s

From relevant portion of the schematic:

$Comp
L ESP_Passives:C C5
U 1 1 6112EA71
P 2500 3050
F 0 "C5" H 2385 3004 50  0000 R CNN
F 1 "2pF" H 2385 3095 50  0000 R CNN
F 2 "0805" H 2538 2900 50  0001 C CNN
F 3 "~" H 2500 3050 50  0001 C CNN
F 4 "~" H 2385 3104 50  0001 C CNN "Comments"
F 5 "2pF 1% 100V Ceramic Capacitor 0805" H 2385 3104 50  0001 C CNN "Descriptions"
F 6 "~" H 2385 3104 50  0001 C CNN "EM_MFR"
F 7 "~" H 2385 3104 50  0001 C CNN "EM_MFR_PN"
F 8 "~" H 2385 3104 50  0001 C CNN "EM_value"
F 9 "AVX" H 2385 3104 50  0001 C CNN "MFR"
F 10 "~" H 2385 3104 50  0001 C CNN "MFR_PN"
F 11 "CDR31BP2R0BBUS" H 2385 3104 50  0001 C CNN "PN"
    1    2500 3050
    -1   0    0    1   
$EndComp
Wire Wire Line
    2000 2900 2500 2900
Wire Wire Line
    2000 3200 2500 3200
Connection ~ 2000 3200
Connection ~ 2000 2900
$Comp
L ESP_Passives:C C6
U 1 1 6113C567
P 2500 3800
F 0 "C6" H 2385 3754 50  0000 R CNN
F 1 "2pF" H 2385 3845 50  0000 R CNN
F 2 "0805" H 2538 3650 50  0001 C CNN
F 3 "~" H 2500 3800 50  0001 C CNN
F 4 "~" H 2385 3854 50  0001 C CNN "Comments"
F 5 "2pF 1% 100V Ceramic Capacitor 0805" H 2385 3854 50  0001 C CNN "Descriptions"
F 6 "~" H 2385 3854 50  0001 C CNN "EM_MFR"
F 7 "~" H 2385 3854 50  0001 C CNN "EM_MFR_PN"
F 8 "~" H 2385 3854 50  0001 C CNN "EM_value"
F 9 "AVX" H 2385 3854 50  0001 C CNN "MFR"
F 10 "~" H 2385 3854 50  0001 C CNN "MFR_PN"
F 11 "CDR31BP2R0BBUS" H 2385 3854 50  0001 C CNN "PN"
    1    2500 3800
    -1   0    0    1   
$EndComp

Expected behavior Components from schematic files are correctly grouped

Desktop (please complete the following information):

Additional context

Bizarrely, extraction and insertion worked perfectly once on this schematic earlier this evening, but since then any attempt at extraction leads to broken output. Makes me wonder if the earlier insertion broke something subtly in the schematic files.

See below, the extraction and insertion issue may be specific to this schematic.

scandey commented 3 years ago

Two additions from this morning: I updated to development branch (ad31dbb) and it runs fine now (thank you for those quick fixes to the dependencies!). This bug still remains though.

I've tried with and without grouping, in both cases C6 appears to be interpreted incorrectly.

I did catch other corruptions this morning, so the first extraction/insertion didn't work perfectly after all. It doesn't seem to be limited to capacitors.

I reverted my schematic back to before the first use of KiField and I'm repeating changes individually from that point.

Extraction from that schematic now correctly associates components together, but weirdly, it doesn't extract all the fields. Notably the Value and Footprint fields are not extracted. This may be an unrelated bug. All user created fields that previously existed are included in the csv, but none of the default ones.

Next step is to revert to 0.1.17 and check if that specific behavior continues (to identify if it is a bug with the development branch or an issue with a corrupted schematic).

scandey commented 3 years ago

Update: 0.1.17 does include the footprint and value fields, so there is an issue with development branch failing to see the default fields. When running Kifield in debug mode I note that it is just missing the fields entirely upon extract:

> {'C1': {'Comments': '~',
        'Descriptions': '0.01uF 10% 50V Ceramic Capacitor 0805',
        'EM_MFR': '~',
        'EM_MFR_PN': '~',
        'EM_value': '~',
        'MFR': 'AVX',
        'MFR_PN': '~',
        'PN': 'CDR31BX103AKUS'}

For whatever its worth, I'm happy to privately send the schematic I'm working on if that would be helpful in debugging, but unfortunately I can't post it publicly.

xesscorp commented 3 years ago

Yes, the 0.1.18 is failing to handle the default fields correctly. I'm working on it now. I hope to have a fix soon. I hope you've been using the backup feature and didn't corrupt your design!

scandey commented 3 years ago

Yes, the 0.1.18 is failing to handle the default fields correctly. I'm working on it now. I hope to have a fix soon. I hope you've been using the backup feature and didn't corrupt your design!

Nothing permanent, just restored the schematic in git to before the first use of KiField (and I can reconstruct the changes made after that).

I'm still not sure what caused the component corruption though, so I will proceed with 0.1.17 like last night, just keeping very careful track of component values. Thank you for all your work on this tool!

scandey commented 3 years ago

Slowly isolating whats going on: it looks like this issue only affect parts that were copied and pasted. Looking deeper into my schematic, this is definitely a me problem, not a KiField problem. (Not a surprise at all!) I think I copied and pasted some low level hierarchical schematic files at some point and deleted the original, which started this whole problem.

For other 5.1.10 users, if your schematic file looks like this with static component references and extra AR Path lines that shouldn't be there, clearing all annotations and resetting them worked for me. It is pain to change comments and whatnot but it seems to have fixed the issue, allowing KiField to work properly.

$Comp
L ESP_Passives:C C59
U 1 1 5FF7F864
P 9250 6200
AR Path="/5FF2A63B/5FF4CFFF/5FF7F864" Ref="C59"  Part="1" 
AR Path="/5FF2A63B/5FF47B16/5FF7F864" Ref="C69"  Part="1" 
AR Path="/5FF2A63B/5FF48555/5FF7F864" Ref="C?"  Part="1" 
AR Path="/5FF2A63B/5FF4902A/5FF7F864" Ref="C?"  Part="1" 
AR Path="/5FF2A63B/60099BA5/5FF7F864" Ref="C85"  Part="1" 
AR Path="/5FF2A63B/60099BAB/5FF7F864" Ref="C95"  Part="1" 
AR Path="/5FF2A63B/61180680/611D92D4/5FF7F864" Ref="C?"  Part="1" 
AR Path="/5FF2A63B/61180680/611D92DA/5FF7F864" Ref="C91"  Part="1" 
AR Path="/5FF2A63B/613479F3/611D92DA/5FF7F864" Ref="C148"  Part="1" 
F 0 "C148" V 9502 6200 50  0000 C CNN
F 1 "0.1uF" V 9411 6200 50  0000 C CNN
F 2 "1210" H 9288 6050 50  0001 C CNN
F 3 "~" H 9250 6200 50  0001 C CNN
F 4 "~" H 9400 6400 50  0001 C CNN "Comments"
F 5 "0.1uF 10% 50V Ceramic Capacitor 1210" H 9400 6400 50  0001 C CNN "Descriptions"
F 6 "~" H 9400 6400 50  0001 C CNN "EM_MFR"
F 7 "~" H 9400 6400 50  0001 C CNN "EM_MFR_PN"
F 8 "~" H 9400 6400 50  0001 C CNN "EM_value"
F 9 "AVX" H 9400 6400 50  0001 C CNN "MFR"
F 10 "~" H 9400 6400 50  0001 C CNN "MFR_PN"
F 11 "CDR33BX104AKUS" H 9400 6400 50  0001 C CNN "PN"
    1    9250 6200
    0    -1   -1   0   
$EndComp
xesscorp commented 3 years ago

I'm not sure it's your problem. I definitely have an error in KiField. I've fixed it for all but one integration test. Still working on it.

scandey commented 3 years ago

Hm.. everything seems to have worked for me after complete re-annotation. I will keep looking to make sure that is actually the case.

xesscorp commented 3 years ago

I mean, we definitely saw it failing to extract the default fields, right? And that happened with my circuit which is completely separate from yours.

scandey commented 3 years ago

Oh! Sorry, there are two issues here. I should have split the default values issue in development branch into a separate issue. That is not resolved.

I've been working on 0.1.17 today, trying to identify what about my schematic was causing KiField to collect items that seemed totally unrelated into the same line item. That issue seems to be due to my confused schematic files, which was fixed by fully re-annotating.

xesscorp commented 3 years ago

Ah, OK.

xesscorp commented 3 years ago

I pushed my fixes to the development branch. Give it a try sometime and let me know if it works OK for you. I still have some other issues to address before releasing on PyPi.

scandey commented 3 years ago

It works for me! Thank you so much.