Skip to content

Commit dbc89ab

Browse files
authored
Prefer KiCad style-1 pin names for symbol variants (#553)
Fixes regression introduced in 1587a79 (unnamed-pin fallback unification).
1 parent 35cbbc6 commit dbc89ab

File tree

3 files changed

+180
-6
lines changed

3 files changed

+180
-6
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ and this project adheres to Semantic Versioning (https://semver.org/spec/v2.0.0.
2929
### Fixed
3030

3131
- Standardized KiCad unnamed-pin handling: empty/placeholder names now fall back to pin numbers in both import and runtime symbol loading, fixing `Unknown pin name` errors for imported components.
32+
- KiCad symbol variant parsing now selects one style per unit using named-pin coverage (tie: lowest style index), avoiding pin-name overrides while supporting `_N_0` symbols.
3233

3334
## [0.3.42] - 2026-02-13
3435

crates/pcb-eda/src/kicad/symbol.rs

Lines changed: 59 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ use crate::{Part, Pin, PinAt, Symbol};
22
use anyhow::Result;
33
use pcb_sexpr::{parse, Sexpr, SexprKind};
44
use serde::Serialize;
5-
use std::collections::HashMap;
5+
use std::cmp::Reverse;
6+
use std::collections::{BTreeMap, HashMap};
67
use std::fs;
78
use std::path::Path;
89
use std::str::FromStr;
@@ -147,6 +148,7 @@ pub(super) fn parse_symbol(symbol_data: &[Sexpr]) -> Result<KicadSymbol> {
147148
raw_sexp: Some(Sexpr::list(symbol_data.to_vec())),
148149
..Default::default()
149150
};
151+
let mut nested_pin_groups: BTreeMap<u32, Vec<NestedStylePins>> = BTreeMap::new();
150152

151153
for prop in &symbol_data[2..] {
152154
if let SexprKind::List(prop_list) = &prop.kind {
@@ -168,31 +170,82 @@ pub(super) fn parse_symbol(symbol_data: &[Sexpr]) -> Result<KicadSymbol> {
168170
}
169171
}
170172
_ if prop_name.starts_with("symbol") => {
171-
// This is the nested symbol section which may contain pins
172-
parse_symbol_section(&mut symbol, prop_list);
173+
// Nested symbol sections contain unit/style-specific graphics + pins.
174+
let (unit, style) = nested_symbol_unit_style(prop_list);
175+
let pins = parse_symbol_section(prop_list);
176+
let named_pin_count = pins.iter().filter(|p| is_named_pin(p)).count();
177+
nested_pin_groups
178+
.entry(unit)
179+
.or_default()
180+
.push(NestedStylePins {
181+
style,
182+
named_pin_count,
183+
pins,
184+
});
173185
}
174186
_ => {}
175187
}
176188
}
177189
}
178190
}
179191

192+
for (_unit, style_candidates) in nested_pin_groups {
193+
if let Some(best) = style_candidates
194+
.into_iter()
195+
.max_by_key(|c| (c.named_pin_count, Reverse(c.style)))
196+
{
197+
symbol.pins.extend(best.pins);
198+
}
199+
}
200+
180201
Ok(symbol)
181202
}
182203

183-
// New function to parse the nested symbol section which contains pins in new format
184-
fn parse_symbol_section(symbol: &mut KicadSymbol, section_data: &[Sexpr]) {
204+
struct NestedStylePins {
205+
style: u32,
206+
named_pin_count: usize,
207+
pins: Vec<KicadPin>,
208+
}
209+
210+
fn is_named_pin(pin: &KicadPin) -> bool {
211+
!pin.name.is_empty() && pin.name != "~"
212+
}
213+
214+
// Parse pins from a nested symbol section.
215+
fn parse_symbol_section(section_data: &[Sexpr]) -> Vec<KicadPin> {
216+
let mut pins = Vec::new();
185217
for item in section_data {
186218
if let SexprKind::List(pin_data) = &item.kind {
187219
if let Some(SexprKind::Symbol(type_name)) = pin_data.first().map(|s| &s.kind) {
188220
if type_name == "pin" {
189221
if let Some(pin) = parse_pin_from_section(pin_data) {
190-
symbol.pins.push(pin);
222+
pins.push(pin);
191223
}
192224
}
193225
}
194226
}
195227
}
228+
pins
229+
}
230+
231+
fn nested_symbol_unit_style(section_data: &[Sexpr]) -> (u32, u32) {
232+
section_data
233+
.get(1)
234+
.and_then(|n| n.as_str().or_else(|| n.as_sym()))
235+
.map(|name| {
236+
// Parse trailing `_<unit>_<style>` without constraining the base name.
237+
let mut parts = name.rsplitn(3, '_');
238+
let style = parts
239+
.next()
240+
.and_then(|s| s.parse().ok())
241+
.unwrap_or_default();
242+
let unit = parts
243+
.next()
244+
.and_then(|s| s.parse().ok())
245+
.unwrap_or_default();
246+
(unit, style)
247+
})
248+
.unwrap_or((0, 0))
196249
}
197250

198251
fn parse_pin_common(pin_data: &[Sexpr]) -> KicadPin {

crates/pcb-eda/tests/test_eda.rs

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,126 @@ fn test_nested_unnamed_pin_preserved_and_uses_number_as_signal_name() {
178178
assert_eq!(pin_map.get("2"), Some(&"2".to_string()));
179179
}
180180

181+
#[test]
182+
fn test_style_1_pin_names_preferred_over_alternate_style() {
183+
let content = r#"(kicad_symbol_lib
184+
(version 20211014)
185+
(generator "test")
186+
(symbol "Demo:Diode"
187+
(symbol "Diode_1_1"
188+
(pin unspecified line
189+
(at 0 0 0)
190+
(length 2.54)
191+
(name "K")
192+
(number "1")
193+
)
194+
(pin unspecified line
195+
(at 10.16 0 180)
196+
(length 2.54)
197+
(name "A")
198+
(number "2")
199+
)
200+
)
201+
(symbol "Diode_1_2"
202+
(pin unspecified line
203+
(at 0 10.16 270)
204+
(length 2.54)
205+
(name "")
206+
(number "1")
207+
)
208+
(pin unspecified line
209+
(at 0 -10.16 90)
210+
(length 2.54)
211+
(name "")
212+
(number "2")
213+
)
214+
)
215+
)
216+
)"#;
217+
218+
let lib = SymbolLibrary::from_string(content, "kicad_sym").unwrap();
219+
let symbol = lib.first_symbol().unwrap();
220+
assert_eq!(symbol.pins.len(), 2);
221+
222+
let pin_map: HashMap<_, _> = symbol
223+
.pins
224+
.iter()
225+
.map(|pin| (pin.number.clone(), pin.signal_name().to_string()))
226+
.collect();
227+
228+
assert_eq!(pin_map.get("1"), Some(&"K".to_string()));
229+
assert_eq!(pin_map.get("2"), Some(&"A".to_string()));
230+
}
231+
232+
#[test]
233+
fn test_style_0_only_symbol_is_supported() {
234+
let content = r#"(kicad_symbol_lib
235+
(version 20211014)
236+
(generator "test")
237+
(symbol "Demo:Header"
238+
(symbol "Header_1_0"
239+
(pin passive line
240+
(at 0 0 0)
241+
(length 2.54)
242+
(name "P1")
243+
(number "1")
244+
)
245+
(pin passive line
246+
(at 0 -2.54 0)
247+
(length 2.54)
248+
(name "P2")
249+
(number "2")
250+
)
251+
)
252+
)
253+
)"#;
254+
255+
let lib = SymbolLibrary::from_string(content, "kicad_sym").unwrap();
256+
let symbol = lib.first_symbol().unwrap();
257+
assert_eq!(symbol.pins.len(), 2);
258+
259+
let pin_map: HashMap<_, _> = symbol
260+
.pins
261+
.iter()
262+
.map(|pin| (pin.number.clone(), pin.signal_name().to_string()))
263+
.collect();
264+
265+
assert_eq!(pin_map.get("1"), Some(&"P1".to_string()));
266+
assert_eq!(pin_map.get("2"), Some(&"P2".to_string()));
267+
}
268+
269+
#[test]
270+
fn test_style_tie_uses_lowest_style_number() {
271+
let content = r#"(kicad_symbol_lib
272+
(version 20211014)
273+
(generator "test")
274+
(symbol "Demo:Tie"
275+
(symbol "Tie_1_0"
276+
(pin passive line
277+
(at 0 0 0)
278+
(length 2.54)
279+
(name "A0")
280+
(number "1")
281+
)
282+
)
283+
(symbol "Tie_1_1"
284+
(pin passive line
285+
(at 0 0 0)
286+
(length 2.54)
287+
(name "A1")
288+
(number "1")
289+
)
290+
)
291+
)
292+
)"#;
293+
294+
let lib = SymbolLibrary::from_string(content, "kicad_sym").unwrap();
295+
let symbol = lib.first_symbol().unwrap();
296+
assert_eq!(symbol.pins.len(), 1);
297+
assert_eq!(symbol.pins[0].number, "1");
298+
assert_eq!(symbol.pins[0].signal_name(), "A0");
299+
}
300+
181301
#[test]
182302
fn test_pcm2903cdb_manufacturer() {
183303
test_symbol_option_property(

0 commit comments

Comments
 (0)