horizon-eda / horizon

Horizon is a free EDA package
https://horizon-eda.org/
GNU General Public License v3.0
1.08k stars 79 forks source link

Not all rules are checked on fab output #750

Open frmdstryr opened 9 months ago

frmdstryr commented 9 months ago

Well I think #247 was working but something broke because it doesn't seem to be giving a warning/error anymore? If I open a project with errors and try to export it doesn't run rules. If I manually run rules and get errors it still lets me export without any warning.

Spent half the day trying to rework the qfn chips on my last board thinking I had a shorted solder joint. Finally gave up and pulled off all the chips to find it still shorted, then tested an unpopulated board and saw was that was also shorted...

Re-opened the board ran the rules and see I had moved vias and forgot to update planes.

image

frmdstryr commented 9 months ago

It appears to be because it was a clearance check and the warning is not done for clearance rules. It works if I add this.


diff --git a/src/imp/fab_output_window.cpp b/src/imp/fab_output_window.cpp
index ed6bb1aa..1c1a5b45 100644
--- a/src/imp/fab_output_window.cpp
+++ b/src/imp/fab_output_window.cpp
@@ -366,7 +366,12 @@ void FabOutputWindow::generate()
     RulesCheckCache cache(core);
     const auto r = rules_check(*core.get_rules(), RuleID::PREFLIGHT_CHECKS, core, cache, &cb_nop);
     const auto r_plane = rules_check(*core.get_rules(), RuleID::PLANE, core, cache, &cb_nop);
-    if (r.level != RulesCheckErrorLevel::PASS || r_plane.level != RulesCheckErrorLevel::PASS) {
+    const auto r_clearance = rules_check(*core.get_rules(), RuleID::CLEARANCE_COPPER, core, cache, &cb_nop);
+    if (
+        (r.level != RulesCheckErrorLevel::PASS)
+        || (r_plane.level != RulesCheckErrorLevel::PASS)
+        || (r_clearance.level != RulesCheckErrorLevel::PASS)
+    ) {
         Gtk::MessageDialog md(*this, "Preflight or plane checks didn't pass", false /* use_markup */,
                               Gtk::MESSAGE_ERROR, Gtk::BUTTONS_NONE);
         md.set_secondary_text("This might be due to unfilled planes or overlapping planes of identical priority.");
:

``
carrotIndustries commented 9 months ago

The clearance check isn't run on fab. output since it can take a very long time.

frmdstryr commented 9 months ago

Hmm, yeah it is noticeably slower. I have 2 potential suggestions.

If there are any board changes since rules were last run show question asking to rerun? Not sure if this will help since it can easily be skipped (in my case I didn't think about the planes in the internal layers).

The other thought is to always do a post export run of checks with a status checkmark or something. This way it does not slow the export and is not in the way but can still alert if something fails (like CI). What do you think?