From cb61e9b0845536c242b90514958fe45c710eece5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois?= Date: Wed, 3 Jun 2026 16:02:39 +0200 Subject: [PATCH] P3: unify connector layout + BSDL behind one PinModel provider New PinModel interface (spec_for / layout / source) + a single apply_model( Part*, const PinModel&) that materialises missing layout pins and sets each pin's spec only where the model speaks (spec.source != None), so one source never clobbers another's. ConnectorModel wraps pin_role/pin_layout; BsdlPinModel wraps a parsed BsdlModel (indexed by port name and physical pad). set-connector-type and screen_settype now use ConnectorModel + apply_model; attach-bsdl and the restore re-apply keep calling apply_bsdl, now a thin adapter over apply_model. Behaviour-preserving: unit tests (73 cases) green and the real 8-card system re-runs identically (1517/1517 bound, same JTAG findings). Covered by test_pin_model. Co-Authored-By: Claude Opus 4.8 --- DESIGN.md | 5 ++- src/system/bsdl_model.cpp | 56 ++++++++++++++++++------------ src/system/bsdl_model.hpp | 19 +++++++++++ src/system/pin_model.cpp | 53 +++++++++++++++++++++++++++++ src/system/pin_model.hpp | 55 ++++++++++++++++++++++++++++++ src/tui/commands.cpp | 10 +++--- src/tui/screen_settype.cpp | 5 +-- tests/test_pin_model.cpp | 70 ++++++++++++++++++++++++++++++++++++++ 8 files changed, 243 insertions(+), 30 deletions(-) create mode 100644 src/system/pin_model.cpp create mode 100644 src/system/pin_model.hpp create mode 100644 tests/test_pin_model.cpp diff --git a/DESIGN.md b/DESIGN.md index 4de07c7..d2954d5 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -37,7 +37,8 @@ src/ CheckIdentityCompatible + FillIdentityNCs pin_role.{hpp,cpp} pin_role(kind, name) → PinSpec, pin_layout(kind), FillPartFromLayout(part, kind) - bsdl_model.{hpp,cpp} BsdlModel (libbsdl C-ABI wrapper) + apply_bsdl(Part*, model) + pin_model.{hpp,cpp} PinModel + apply_model(Part*, model) + ConnectorModel + bsdl_model.{hpp,cpp} BsdlModel (libbsdl wrapper) + BsdlPinModel + apply_bsdl bsdl_check.{hpp,cpp} check_pin_specs / check_jtag_chain → vector nets.{hpp,cpp} find_net / compute_all_nets / net_type_consistent analysis.{hpp,cpp} analyze_system → AnalysisReport (diff pairs, buses, anomalies) @@ -128,6 +129,8 @@ The explore screen shows the type in the signal detail header. **BSDL models (`attach-bsdl`)**: `attach-bsdl ` parses a BSDL device through `libbsdl` (wrapped by `BsdlModel`, `src/system/bsdl_model.{hpp,cpp}`), then `apply_bsdl(part, model)` binds each port to a Pin **by port name first, then by physical pad** — so a netlist that names IC pins either by signal or by package ball both bind. Each bound pin gets its `spec` set: `direction` (BSDL in/out/inout/linkage), `function` (TAP role → Jtag\*, `linkage` → Power/Ground/NoConnect by name, else Signal), `pad` (PIN_MAP ball), `source = Bsdl`. The `.bsd` path is stored on `Part::bsdl_path`, persisted via the `B` tag and re-applied on `restore`. Real-world check: an `xcku15p` FPGA in a VPX system binds 1517/1517 ports. +**Unified application (`apply_model`)**: connector layout and BSDL are two implementations of one `PinModel` interface (`src/system/pin_model.{hpp,cpp}`: `spec_for(pin_name)`, `layout()`, `source()`). `ConnectorModel` wraps `pin_role`/`pin_layout`; `BsdlPinModel` wraps a parsed `BsdlModel`, indexed by both port name and physical pad. A single `apply_model(Part*, const PinModel&)` materialises the layout pins missing from the netlist, then sets each pin's `spec` **only where the model speaks** (`spec.source != None`) — so one source never clobbers another's. `set-connector-type` and `attach-bsdl` both funnel through it (the latter via the thin `apply_bsdl` adapter); `verify` stays agnostic of where a spec came from. A future SPICE/Modelica source would be a third `PinModel`. + **`verify` (five passes)**: (1) typed pins — local mismatch between each pin's `expected_signal_type()` (derived from its `PinSpec`) and the actual signal type; (2) bridged nets — Power↔GndShield inconsistencies; (3) orphan summary `N orphan pin(s) at import (X imported NC, Y dropped singleton)` (filters out pins bridged via any `Connection::pin_map` — typically `FillIdentityNCs`-materialised); (4) **model-driven pin checks** (`check_pin_specs`): `DriveContention` (≥2 push-pull `Out` on a net), `UndrivenNet` (a **fully-modelled** net with input(s) but no driver — nets with any Unknown-direction pin are skipped, so un-modelled drivers don't cause false positives), `NcWired` (a no-connect pin on a multi-pin net); (5) **JTAG chain** (`check_jtag_chain`): collects TAP pins by `spec.function`, maps each to its net, emits `JtagTapIncomplete` / `JtagBusUnbridged` (TMS or TCK not common to all TAP devices) / `JtagChainBreak` (dangling TDO/TDI, chain fan-out, or not a single head→tail daisy chain). The BFS-reached `(module, signal)` set for any signal is shown live in `explore`'s detail pane when a signal entry is selected. **`analyze` (post-processing pass)**: `analyze_system(System*) → AnalysisReport` (`src/system/analysis.{hpp,cpp}`) is a stateless read-only pass that detects structural signal groups and anomalies. Per-module (signals are module-scoped): diff --git a/src/system/bsdl_model.cpp b/src/system/bsdl_model.cpp index f45b27f..42efa37 100644 --- a/src/system/bsdl_model.cpp +++ b/src/system/bsdl_model.cpp @@ -112,30 +112,42 @@ BsdlModel BsdlModel::from_buffer(const std::string &text, const std::string &nam return from_handle(bsdl_parse_buffer(text.data(), text.size(), name.c_str(), &opts)); } +BsdlPinModel::BsdlPinModel(const BsdlModel &model) +{ + for (const auto &port : model.ports()) { + PinSpec s; + s.function = port.function; + s.direction = port.direction; + s.pad = port.pad; + s.source = SpecSource::Bsdl; + if (!port.name.empty()) by_name_[port.name] = s; + if (!port.pad.empty()) by_pad_[port.pad] = s; + } +} + +PinSpec BsdlPinModel::spec_for(const std::string &pin_name) const +{ + auto byn = by_name_.find(pin_name); + if (byn != by_name_.end()) + return byn->second; + auto byp = by_pad_.find(pin_name); + if (byp != by_pad_.end()) + return byp->second; + return PinSpec{}; // source == None +} + +// Adapter kept for callers that hold a BsdlModel directly (attach-bsdl, the +// restore re-apply pass, tests): bind via the unified apply_model and map the +// report back to port-coverage terms. BsdlApplyReport apply_bsdl(Part *part, const BsdlModel &model) { + BsdlPinModel pm(model); + ApplyReport rep = apply_model(part, pm); + BsdlApplyReport r; - if (!part) - return r; - - r.pins_total = (int)part->size(); - for (const auto &port : model.ports()) { - Pin *pin = nullptr; - if (!port.name.empty() && part->exists(port.name)) - pin = part->get(port.name); - else if (!port.pad.empty() && part->exists(port.pad)) - pin = part->get(port.pad); - - if (!pin) { - ++r.unbound; - continue; - } - - pin->spec.function = port.function; - pin->spec.direction = port.direction; - pin->spec.pad = port.pad; - pin->spec.source = SpecSource::Bsdl; - ++r.bound; - } + r.pins_total = rep.pins_total; + r.bound = rep.set; + int ports = (int)model.ports().size(); + r.unbound = ports > rep.set ? ports - rep.set : 0; return r; } diff --git a/src/system/bsdl_model.hpp b/src/system/bsdl_model.hpp index 650db76..273d58f 100644 --- a/src/system/bsdl_model.hpp +++ b/src/system/bsdl_model.hpp @@ -1,9 +1,11 @@ #ifndef _BSDL_MODEL_HPP_ #define _BSDL_MODEL_HPP_ +#include "pin_model.hpp" #include "pin_spec.hpp" #include +#include #include class Part; @@ -39,6 +41,23 @@ private: std::vector ports_; }; +// Adapts a parsed BSDL device to the PinModel interface, indexing its ports by +// both logical name and physical pad (so a netlist that names IC pins either by +// signal or by ball both resolve). `layout()` is empty — BSDL drives specs, not +// pin materialisation, since the netlist's pin naming may differ from the port +// names. +class BsdlPinModel : public PinModel { +public: + explicit BsdlPinModel(const BsdlModel &model); + PinSpec spec_for(const std::string &pin_name) const override; + std::vector layout() const override { return {}; } + SpecSource source() const override { return SpecSource::Bsdl; } + +private: + std::unordered_map by_name_; + std::unordered_map by_pad_; +}; + // Outcome of binding a model onto a Part's pins. struct BsdlApplyReport { int pins_total = 0; diff --git a/src/system/pin_model.cpp b/src/system/pin_model.cpp new file mode 100644 index 0000000..c6f4fae --- /dev/null +++ b/src/system/pin_model.cpp @@ -0,0 +1,53 @@ +#include "pin_model.hpp" + +#include "parts.hpp" +#include "pin_name.hpp" +#include "pin_role.hpp" +#include "pins.hpp" + +#include + +ApplyReport apply_model(Part *part, const PinModel &model) +{ + ApplyReport r; + if (!part) + return r; + + // 1. Materialise canonical pins absent from the netlist (deduped by + // canonical name, like FillPartFromLayout). No-op when layout() is empty. + std::vector layout = model.layout(); + if (!layout.empty()) { + std::unordered_set existing; + for (auto &kv : *part) + existing.insert(canonical_pin_name(kv.first)); + for (const std::string &name : layout) { + if (existing.count(canonical_pin_name(name))) + continue; + if (part->exists(name)) + continue; + part->add(new Pin(name)); + ++r.materialised; + } + } + + // 2. Set each pin's spec where the model speaks for it. + r.pins_total = (int)part->size(); + for (auto &kv : *part) { + PinSpec s = model.spec_for(kv.first); + if (s.source != SpecSource::None) { + kv.second->spec = s; + ++r.set; + } + } + return r; +} + +PinSpec ConnectorModel::spec_for(const std::string &pin_name) const +{ + return pin_role(kind_, pin_name); +} + +std::vector ConnectorModel::layout() const +{ + return pin_layout(kind_); +} diff --git a/src/system/pin_model.hpp b/src/system/pin_model.hpp new file mode 100644 index 0000000..54be377 --- /dev/null +++ b/src/system/pin_model.hpp @@ -0,0 +1,55 @@ +#ifndef _PIN_MODEL_HPP_ +#define _PIN_MODEL_HPP_ + +#include "pin_spec.hpp" + +#include +#include +#include + +class Part; + +// A source of *expected* pin attributes for a part — a connector layout, a BSDL +// device, (later) a SPICE/Modelica model. Both of today's sources implement this +// interface and feed the single `apply_model()`, so `verify` stays agnostic of +// where a pin's spec came from. +struct PinModel { + virtual ~PinModel() = default; + + // Expected spec for the pin identified by `pin_name` — its logical name, and + // for models that index physical pads, its package ball too. Returns a + // default PinSpec (source == None) when the model has nothing to say. + virtual PinSpec spec_for(const std::string &pin_name) const = 0; + + // Canonical full pin-name list, used to materialise pins absent from the + // imported netlist. Empty means the model does not drive materialisation. + virtual std::vector layout() const = 0; + + // Which source this model represents (recorded in each spec it sets). + virtual SpecSource source() const = 0; +}; + +struct ApplyReport { + int pins_total = 0; ///< pins on the part after materialisation + int set = 0; ///< pins whose spec the model set + int materialised = 0; ///< pins created from layout() +}; + +// Materialise the layout pins missing from the part, then set each pin's `spec` +// from the model — only where the model actually speaks for that pin +// (`spec.source != None`), so a model never wipes another source's spec. +ApplyReport apply_model(Part *part, const PinModel &model); + +// Connector-layout model: wraps `pin_role(kind, name)` / `pin_layout(kind)`. +class ConnectorModel : public PinModel { +public: + explicit ConnectorModel(std::string kind) : kind_(std::move(kind)) {} + PinSpec spec_for(const std::string &pin_name) const override; + std::vector layout() const override; + SpecSource source() const override { return SpecSource::ConnectorModel; } + +private: + std::string kind_; +}; + +#endif // _PIN_MODEL_HPP_ diff --git a/src/tui/commands.cpp b/src/tui/commands.cpp index 8cede1f..231e314 100644 --- a/src/tui/commands.cpp +++ b/src/tui/commands.cpp @@ -8,6 +8,7 @@ #include "system/parts.hpp" #include "system/persist.hpp" #include "system/pin_role.hpp" +#include "system/pin_model.hpp" #include "system/bsdl_model.hpp" #include "system/bsdl_check.hpp" #include "system/pins.hpp" @@ -455,13 +456,12 @@ void Tui::RegisterCommands() { return; } prt->connector_type = args[2]; - int filled = FillPartFromLayout(prt, args[2]); - for (auto &kv : *prt) - kv.second->spec = pin_role(args[2], kv.first); + ConnectorModel model(args[2]); + ApplyReport rep = apply_model(prt, model); Print(mod->name + "/" + prt->name + ": connector_type = " + (args[2].empty() ? "(none)" : args[2])); - if (filled > 0) - Print("set-connector-type: materialised " + std::to_string(filled) + if (rep.materialised > 0) + Print("set-connector-type: materialised " + std::to_string(rep.materialised) + " NC pin(s) from connector layout"); }, /*prompt_for_missing=*/ false, diff --git a/src/tui/screen_settype.cpp b/src/tui/screen_settype.cpp index 168f5ba..001bda2 100644 --- a/src/tui/screen_settype.cpp +++ b/src/tui/screen_settype.cpp @@ -3,6 +3,7 @@ #include "system/modules.hpp" #include "system/parts.hpp" +#include "system/pin_model.hpp" #include "system/pin_role.hpp" #include "system/pins.hpp" #include "system/system.hpp" @@ -68,8 +69,8 @@ Component Tui::BuildSettypeScreen() { return; } prt->connector_type = settype_type; - for (auto &kv : *prt) - kv.second->spec = pin_role(settype_type, kv.first); + ConnectorModel model(settype_type); + apply_model(prt, model); std::string msg = mod->name + "/" + prt->name + " = " + (settype_type.empty() ? "(none)" : settype_type); settype_status = "applied: " + msg; diff --git a/tests/test_pin_model.cpp b/tests/test_pin_model.cpp new file mode 100644 index 0000000..0da9e71 --- /dev/null +++ b/tests/test_pin_model.cpp @@ -0,0 +1,70 @@ +#include + +#include "system/parts.hpp" +#include "system/pin_model.hpp" +#include "system/pin_spec.hpp" +#include "system/pins.hpp" + +#include +#include + +namespace { + +// A stand-in PinModel: knows VCC (power) and CLK (clock input), has a 3-pin +// canonical layout, and says nothing about anything else. +struct FakeModel : PinModel { + PinSpec spec_for(const std::string &name) const override { + PinSpec s; + if (name == "VCC") { + s.function = PinFunction::Power; + s.direction = PinDirection::Power; + s.source = SpecSource::Bsdl; + } else if (name == "CLK") { + s.function = PinFunction::Clock; + s.direction = PinDirection::In; + s.source = SpecSource::Bsdl; + } + return s; // else default: source == None + } + std::vector layout() const override { + return {"VCC", "CLK", "GND"}; + } + SpecSource source() const override { return SpecSource::Bsdl; } +}; + +} // namespace + +TEST_CASE("apply_model materialises layout pins and sets specs where the model speaks") { + Part part("U1"); + part.add(new Pin("VCC")); // already present; CLK and GND are not + + FakeModel m; + ApplyReport r = apply_model(&part, m); + + // CLK and GND materialised from layout(); VCC was already there. + CHECK(r.materialised == 2); + CHECK(part.exists("CLK")); + CHECK(part.exists("GND")); + CHECK(r.pins_total == 3); + + // Specs set only where the model speaks (VCC, CLK), not GND. + CHECK(part.get("VCC")->spec.function == PinFunction::Power); + CHECK(part.get("VCC")->spec.source == SpecSource::Bsdl); + CHECK(part.get("CLK")->spec.function == PinFunction::Clock); + CHECK(part.get("GND")->spec.source == SpecSource::None); + CHECK(r.set == 2); +} + +TEST_CASE("apply_model does not overwrite a spec the model is silent about") { + Part part("U2"); + Pin *p = new Pin("DATA"); + p->spec.function = PinFunction::Signal; + p->spec.source = SpecSource::Bsdl; // a prior model wrote this + part.add(p); + + FakeModel m; // says nothing about DATA + apply_model(&part, m); + + CHECK(part.get("DATA")->spec.function == PinFunction::Signal); + CHECK(part.get("DATA")->spec.source == SpecSource::Bsdl); +}