Repository navigation
SYN-40 - A component declares how its pins escape - #164
Conversation
1be7f8b to
8226c70
Compare
rodneyosodo
left a comment
There was a problem hiding this comment.
Escape wiring looks right: off beats a method, a declared escape is honoured even when the geometry would not select it, dense grids are the fallback, and the four routing entry points all go through use_board_escapes. synth-ir/synth-router/synth-validate/synth-parser pass here.
Three things inline. (Inherited from #155 and still in this branch: requires is absent from the descriptor's language.statements, and schema_version is still hardcoded 1.1 — worth fixing once, on whichever lands first.)
71711ea to
d7e1d7b
Compare
21eddf1 to
859e16b
Compare
be058f7 to
633845c
Compare
rodneyosodo
left a comment
There was a problem hiding this comment.
Re-reviewed delta: rebase only. LGTM.
3acb317 to
a8671e6
Compare
|
[P1] Three routing-safety issues remain in the current implementation:
Please fix these before merging so an escape run cannot silently produce an unpoured, unescaped, or fabrication-invalid board. |
@SammyOina all three in 91e4407. Declared escapes travel as |
Changes requestedThe latest commit fixes the previously reported issues around plane-pour arguments, declared fanout failures, and via-in-pad gating. One blocking fail-open path remains in
For an explicit The PR is currently conflicting and has no current checks reported. Please rebase it onto |
`analyse` answers whether a package *cannot* be fanned out at the fabrication floor. That is a different question from whether it needs a dedicated escape engine, and conflating them gets the answer backwards: the iCE40's 0.8 mm ball grid passes `analyse`, because 0.8 mm clears the 0.687 mm two vias need between centres, while the board's 0.5 mm connectors fail it. Driving an escape from the findings therefore fanned out the connectors and skipped the 121-ball BGA. `grid_arrays` asks the question that matters instead: three distinct pad rows and three distinct columns make a lattice, which no library id or package name has to be trusted for. The tests pin both directions, since getting them the wrong way round is the mistake this function exists to prevent.
`min_pitch_nm` and `min_gap_nm` each scanned the pads independently, so a finding could report a pitch measured between two signal pins and a gap measured between a pin and a mounting pad. The numbers were individually true and described no single pair. `closest_pair_nm` returns both from the same pair. The verdict is unchanged; only the arithmetic it reports. The test fixture builds its pad string with `write!` into a `String` rather than `format!` over an iterator, which `clippy::pedantic` rejects and CI treats as an error.
… them Left to the maze router, a 0.8 mm ball grid comes back with a stub on every lane rather than an escape under every pad. KiCadRoutingTools ships an escape engine for exactly this and Synth never called it, which is what absmach#137 was really about once the via-geometry half was done. The adapter now runs KRT's documented order — pour, escape each package, then route — and a stage that fails degrades to a warning, because an un-escaped board still routes and saying so beats refusing to route. Two things the fabrication floor taught us, both measured on the Class C board. The escape via is sized from the declared process rather than left to the engine: 23 vias came back at the tier's smallest legal 0.56/0.3, where the default had placed 11 at a 0.2 mm drill. And `--fab-tier auto` is documented to escalate to an advanced tier for a fine-pitch escape, which is right for a human at a terminal and wrong for a board that has named its manufacturer — it rewrote the project's own minimums from 0.56/0.3 to 0.35/0.2. Both now travel with every stage. Measured against the same board without the escape: DRC errors 63 -> 57, unconnected pads 13 -> 10, segments 456 -> 352, no floor relaxed and no via below the floor. Seven of the eight TMDS lanes still do not route; the escape does not address what fails there.
The commit that wires the escape engine in carried no test of its own, and it is the one that decides whether the stage runs at all. Three cases. A board with an 11 x 11 ball grid gets `--fanout-component U1` and an escape via sized from the declared floor, 0.3 mm drill plus two 0.13 mm rings. A two-pad board gets none of the three flags, so an ordinary design does not pay for a stage it does not need. And `--fab-tier auto` resolves to `standard` on a 0.3 mm floor and `advanced` on 0.2 mm, while an explicit operator choice still wins. The fixture writes a real baseline, because the engine reads the board it will be handed rather than the request.
`escape { method underpad layers "In1.Cu", "In2.Cu" }` on a component, or
`escape { off }` to leave a package to the general router. Geometric
selection stays the default: the block overrides it rather than replacing
it, and precedence is `off` first, then what the design declared, then the
geometry.
Needing an escape is a property of the package, but *how* to escape it is a
design decision — which layers it may use, and whether a part the geometry
would pick should be left alone. Before this the engine's choice was the
only one available, and a board had no way to disagree with it.
`method` and `off` are contextual identifiers so a design already using
those words as names keeps parsing. `layers` cannot be: it is the reserved
keyword that declares the board's copper count, so the sub-setting arrives
as `KwLayers` and is matched as such.
E-SYNTH-PARSE-040 names an attribute the block does not take, and
E-SYNTH-PROCESS-004 an escape method this build does not know; both ship
their diagnostic pages.
Taken from the absmach#138 monolith, minus the parts that belonged to work which
has not landed: the `match_length` length groups, which absmach#145 closed in
favour of a netclass `max_skew`, and the netclass via-geometry constants.
The MCP call sites are re-derived against main's `evaluate_candidate_mcp`
refactor rather than the inline body they were written for, and the
candidate evaluator takes the intent too, because a declared `off` would
otherwise change which placement wins.
… exist The descriptor's constraint vocabulary is built by reflecting over the netclass, diff_pair and keepout attribute enums. A component's `escape` is a struct field, so it has no variant list to reflect and no id in that vocabulary — the limit claimed `component.escape` and shard 1 refused it. The limit itself is real; only the pointer was invented.
The two ERC fixtures assert that a board with an `escape` block raises no diagnostics. That is not enough: a grammar can accept an attribute and lower it to nothing, and the fixture passes either way. An arm written for an identifier the lexer never emits as one is unreachable, and the attribute is then silently dropped — which is exactly the shape of the netclass `max_skew` defect on absmach#134. So these read the value back. A declared method and layer list, `off` on its own, a method with no layers, a component that declares nothing, both `off` and a method together, and an unknown method reported rather than dropped. The real registry rather than an empty one, so the components resolve and the only diagnostics left are the ones the file is about.
…ter's The escape via was pinned to the declared floor and the clearance was not, so the adapter's own 0.1 mm default applied — under this board's 0.127 mm minimum and under the engine's own 0.25 mm. That is the same shape the via sizing fixed: an engine default quietly rewriting a minimum the board declared, with the independent gate then rejecting the copper for it. `--escape-clearance` now carries `policy.min_clearance_nm`, and the command test asserts 0.1270 rather than only the via geometry.
`escape { layers … }` went onto the intent as written. A name the board has
no copper for reached the engine, the fanout failed at route time, and the
package was routed unescaped behind a warning on stderr — the silent
fallback the unknown-method check exists to prevent, and worse here, because
a dense array routed without its escape comes back with a stub on every lane.
E-SYNTH-PROCESS-005 names the layers the board does have.
Checked over the finished board rather than inside `lower_escape`, because
the layer count is a statement like any other and may be declared after the
component that names a layer. There is a test for exactly that ordering.
The comment claimed `component.escape` named an id the constraint vocabulary does not contain, while the same descriptor lists `escape` under `routing_constraints.component` a hundred lines below. It contradicted the file it sits in. The limit is not about `escape`. It is about pattern and direction, which no attribute declares, so there is nothing for it to name — pointing at `escape` would claim the limit qualifies something the block can express.
`label_spots.rs` arrived on main with absmach#228 and builds a `Component` literal. This branch adds `escape` to that struct, so the test stopped compiling the moment the two met -- and because it is a test target in `synth-layout`, that took `Lint and Build` and all three shards down with it. `escape: None` beside `placement_hint`, which is where every other literal in these tests carries it.
`route_planes.py` takes `--nets` and `--plane-layers` as positionally matched lists and exits 2 without them, so the pour stage failed on every run and the escape ran on unpoured fill. KiCadRoutingTools warns about exactly that: a plane-drop via wants real copper to land on. The pairs come from the board's own zones rather than being plumbed through from Rust. Synth's exporter already writes one zone per plane net, so the board is the record of which net pours where, and reading it there keeps the two from disagreeing. A zone spanning several layers cannot be expressed in that positional form, so it is skipped rather than guessed at.
`_fanout` warned and returned the unescaped board whenever `bga_fanout.py`
failed. For a package the geometry selected that is defensible -- nothing was
promised about it, and a worse route beats no route. For a package the design
wrote an `escape { method ... }` block on it is not: the board said how that
package leaves its pads, and handing it to the maze router instead answers a
question nobody asked, silently.
The two cases were indistinguishable at the adapter boundary, because a
declared escape with no method and no layers encodes as a bare refdes exactly
like an inferred one. So the declared set now travels separately, as
`--escape-required`, and a failure on one of those raises instead of warning.
The same distinction applies to the permission. Under-pad escape on a board
that does not allow via-in-pad is skipped when it was inferred, and is an
error naming the package when it was declared -- asking for an escape the
process cannot build is a mistake worth stopping on, not routing around.
Also carries absmach#161's two fixes, which this branch had a copy of the escape
work but not of: the plane pour gets its `--nets` and `--plane-layers`, and
the under-pad method is gated on the permission reaching the escape stage.
…ethod
`_fanout` refused under-pad escape on a board without via-in-pad by
checking the global `--escape-method`, which Synth never passes, so it was
always the `underpad` default. The method a package declared in its own
`escape { method … }` block was never consulted. On such a board a package
declared `escape { method channel }` was skipped with the rest, and because
it was declared, the whole route failed with "declared escape needs
via-in-pad" -- for an escape that puts no via in a pad.
The plan is now resolved per package first: a package whose method is
under-pad is skipped (or fails the route, if it was declared) when via-in-pad
is not permitted, and every other package is escaped as asked. When nothing
is left to escape the board is returned untouched, without pouring planes,
which is what the old early return did.
Four adapter tests drive `_fanout` against a stub engine; the declared
channel case fails on the previous code.
91e4407 to
e551498
Compare
absmach#164 (component `escape` blocks) already claims E-SYNTH-PROCESS-004 for an unknown escape method and 005 for an escape layer the board does not have, with diagnostic pages and fixtures for both. This branch used 004 for an unrelated rule, so whichever merged second would give one code two meanings and two pages at the same path. The tier-not-offered-for-this-stackup rule moves to 006: the code, its diagnostic page, the doc comments that cite it and the test.
Adding `escape: None` to its component literal took `decoupling_test_board` to 102 lines, one past clippy's limit, and the lint job failed on it. The function is one literal board written out field by field, which is the case the allow exists for.
Follow-up to my previous commentThe direct
Please propagate the declared-escape requirement through plane pouring and tool-availability checks so an explicit |
…or declared escapes
|
Addressed follow-up: route_planes.py missing/failure and missing bga_fanout.py now fail closed when a declared escape is present; inferred escapes keep the previous warn-and-continue behavior. |
Follow-up required — add regression coverage for fail-closed escapesThe latest implementation correctly fails closed for explicitly declared escapes when However, the latest fix does not add regression tests for either prerequisite failure path. The existing adapter tests cover escape-method selection and via-in-pad behavior, but not:
Please add tests proving both cases raise/fail the route rather than returning an apparently successful unescaped board. Once those tests are included and pass, this should be ready for approval. |
|
Added regression tests for both fail-closed prerequisite paths (missing/failing route_planes.py and missing bga_fanout.py) with declared escapes. |
What type of PR is this?
This is a feature
What does this do?
A component declares
escape { method underpad },layers, oroffto skip the escape entirely.Which issue(s) does this PR fix/relate to?
Have you included tests for your changes?
Yes. Six IR tests reading the intent back, plus three on
escape_plan's precedence.Did you document any new/modified features?
Yes.
E-SYNTH-PARSE-040andE-SYNTH-PROCESS-004ship diagnostic pages; the descriptor gains anescapekey.Notes
Stacked on #155 and #161. Drops the monolith's
match_lengthand netclass via-geometry hunks.