Skip to content

MCAD-driven placement: apply IDF fixed positions to layout#876

Open
jonfodi wants to merge 1 commit into
diodeinc:mainfrom
jonfodi:fodi/mech_constraints
Open

MCAD-driven placement: apply IDF fixed positions to layout#876
jonfodi wants to merge 1 commit into
diodeinc:mainfrom
jonfodi:fodi/mech_constraints

Conversation

@jonfodi

@jonfodi jonfodi commented Jun 10, 2026

Copy link
Copy Markdown

Adds a pcb-mechanical crate that consumes IDF (.emn) exports from mechanical CAD and pins MCAD-owned components at their exact mechanical pose. Claims are matched to components by reference designator, translated through a footprint-datum catalog
(mechanical/footprint-datums.toml) that maps each mechanical datum into footprint-local coordinates, and applied to the schematic as fixed, locked placements that the layout sync enforces on every run while HierPlace packs ECAD-owned parts around them.

The .emn is discovered by convention (mechanical/.emn next to the board's .zen or at the workspace root) or declared explicitly via [board.mechanical.idf] in pcb.toml. Includes an end-to-end example (examples/usbc_mcad) with a USB-C receptacle and mounting holes fixed by the enclosure.


Note

Medium Risk
Touches layout sync placement logic and introduces new resolution failures (missing datums, ambiguous IDF paths, hash mismatches); incorrect pose math could misplace MCAD-owned parts on every sync.

Overview
Adds MCAD-driven placement so pcb layout can honor mechanical CAD exports before KiCad sync.

A new pcb-mechanical crate discovers IDF board files (.emn) via mechanical/<board>.emn or [board.mechanical.idf] in pcb.toml, parses .PLACEMENT for MCAD-owned refdes, and resolves footprint origins through mechanical/footprint-datums.toml (optional footprint_hash validation). Resolved poses are written onto schematic instances as BoardPose / Instance.placement.

pcbc layout calls apply_mcad_positions after the schematic build. The layout lens treats compiler fixed_placement as authoritative: it overrides saved KiCad placement, creates new footprints at that pose (locked), re-applies pose on updates, and excludes those parts from HierPlace auto-packing (fixed footprints count as existing content). JSON netlist parsing forwards per-instance placement.

Includes examples/usbc_mcad demonstrating USB-C and mounting holes fixed by IDF while ECAD parts stay auto-placed.

Reviewed by Cursor Bugbot for commit 17001ce. Bugbot is set up for automated code reviews on this repo. Configure here.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 3 potential issues.

View 5 additional findings in Devin Review.

Open in Devin Review

Comment thread crates/pcb-mechanical/src/placement.rs
Comment thread crates/pcb-layout/src/lib.rs Outdated
Comment on lines +128 to +142
fn resolve_footprint_pose(claim: &IdfPlacementClaim, local: LocalDatumPose) -> BoardPose {
let footprint_rotation = normalize_degrees(claim.pose.rotation_deg - local.rotation_deg);
let theta = footprint_rotation.to_radians();
let cos_t = theta.cos();
let sin_t = theta.sin();
let dx = local.x_mm * cos_t - local.y_mm * sin_t;
let dy = local.x_mm * sin_t + local.y_mm * cos_t;

BoardPose {
x_nm: mm_to_nm(claim.pose.x_mm - dx),
y_nm: mm_to_nm(claim.pose.y_mm - dy),
rotation_deg: footprint_rotation,
side: claim.pose.side,
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 IDF coordinate system assumed to match KiCad frame

The resolve_footprint_pose function at crates/pcb-mechanical/src/placement.rs:128-142 passes IDF X/Y coordinates directly through to KiCad nanometer positions without any Y-axis inversion. Standard IDF 3.0 uses a right-hand coordinate system (Y-up), while KiCad uses Y-down internally. The README explicitly states "IDF coordinates map 1:1 onto the KiCad sheet frame", making this an intentional design choice — the MCAD export is expected to already be in KiCad's coordinate frame. This works for the described workflow but would silently produce wrong results if a user feeds in a standard Y-up IDF export from SolidWorks/Fusion 360 without pre-transforming coordinates. Consider documenting this assumption prominently (e.g., in the datum TOML or idf_for_board docstring).

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@jonfodi jonfodi force-pushed the fodi/mech_constraints branch 3 times, most recently from ceb841c to 83c66e7 Compare June 12, 2026 17:02

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

Open in Devin Review

Comment on lines +49 to +54
.get(&instance_ref)
.and_then(|instance| instance.attributes.get(ATTR_FOOTPRINT))
.and_then(AttributeValue::string)
.map(str::to_owned);
let Some(footprint) = footprint else {
errors.push(format!(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 ATTR_FOOTPRINT constant assumes evaluator stores footprint under "footprint" key in attributes

The new constant ATTR_FOOTPRINT = "footprint" at crates/pcb-sch/src/lib.rs:51 is used by resolve_mcad_positions (crates/pcb-mechanical/src/placement.rs:49-54) to look up the footprint from instance.attributes. However, the Python JsonNetlistParser reads the footprint from instance.get("footprint_fpid") — a top-level JSON field, not an attribute. The test at crates/pcb-mechanical/src/placement.rs:188 manually adds "footprint" to attributes, so it passes regardless of what the real evaluator does. If the Zener evaluator stores the footprint under a different attribute key (e.g. "footprint_fpid"), datum-based hash validation would always fail with "has no footprint" for every MCAD claim. Verifying the evaluator's attribute key convention would confirm correctness.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

cursor[bot]

This comment was marked as resolved.

Adds a pcb-mechanical crate that consumes IDF (.emn) exports from
mechanical CAD and pins MCAD-owned components at their exact mechanical
pose. Claims are matched to components by reference designator,
translated through a footprint-datum catalog
(mechanical/footprint-datums.toml) that maps each mechanical datum into
footprint-local coordinates, and applied to the schematic as fixed,
locked placements that the layout sync enforces on every run while
HierPlace packs ECAD-owned parts around them.

The .emn is discovered by convention (mechanical/<board>.emn next to
the board's .zen or at the workspace root) or declared explicitly via
[board.mechanical.idf] in pcb.toml. Includes an end-to-end example
(examples/usbc_mcad) with a USB-C receptacle and mounting holes fixed
by the enclosure.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@jonfodi jonfodi force-pushed the fodi/mech_constraints branch from 83c66e7 to 17001ce Compare June 12, 2026 17:11

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 17001ce. Configure here.

Comment thread crates/pcbc/src/layout.rs
return Ok(());
}

pcb_mechanical::apply_mcad_positions(&mut schematic, zen_path, &resolution_result)?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Release skips MCAD placement

Medium Severity

pcb layout applies IDF poses via apply_mcad_positions before process_layout, but release validate_build serializes the staged schematic to netlist.json without that step. Release DRC and layout sync then run against instances missing placement, so MCAD-owned parts are not fixed or updated during release the way pcb layout enforces.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 17001ce. Configure here.

};

let datums = FootprintDatums::load_for_board(board_zen_path, &resolution.workspace_info.root)?;
let claims = idf::load_placement_claims(&emn)?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto IDF parse fails layout

Medium Severity

When mechanical/&lt;board&gt;.emn is found by convention, apply_mcad_positions always parses it. An export without a .PLACEMENT section makes load_placement_claims fail and aborts layout, even if the file was only meant as outline data or MCAD placement is not in use yet.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 17001ce. Configure here.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

Open in Devin Review

Comment on lines +61 to +64
pub(crate) fn lookup(&self, idf_package: &str, footprint: &str) -> Option<&FootprintDatum> {
self.entries.iter().find(|entry| {
entry.idf_package.eq_ignore_ascii_case(idf_package) && entry.footprint == footprint
})

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 Datum lookup uses exact footprint string match, tying it to package URI format

The FootprintDatums::lookup at crates/pcb-mechanical/src/datum.rs:61-64 does case-insensitive match on idf_package but exact (case-sensitive) match on footprint. The footprint value comes from the instance's ATTR_FOOTPRINT attribute, which is a package:// URI like package://gitlab.com/kicad/libraries/kicad-footprints@10.0.3/.... The datum catalog must use the identical URI string. This is fragile if the version tag in the URI changes (e.g., @10.0.3@10.0.4), requiring datum catalog updates. The README mentions footprint_hash as an optional integrity check, which partially mitigates stale datums but doesn't address the matching itself.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

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

Successfully merging this pull request may close these issues.

1 participant