pcb

MinMaskGap() doesn't know about holes

Bug #1022076 reported by Robert Drehmel
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
pcb
Fix Released
Undecided
Bert Timmerman

Bug Description

MinMaskGap() should care about holes as it does about pins, pads, and vias.
Otherwise, fab houses might complain because the solder mask doesn't have enough distance to holes.

The problem in detail:
ActionMinMaskGap() checks whether the hole's thickness plus the specified minimum mask gap is larger than the currently set mask value. If so, it sets the new mask to be the hole's thickness plus the specified minimum mask gap value.
ActionMinMaskGap() should check a hole's drilling diameter and base the mask calculation upon that.

The attached patch fixes the issue.

Revision history for this message
Robert Drehmel (r58qft) wrote :
Revision history for this message
Bert Timmerman (bert-timmerman) wrote :

Hi Robert and all,

I have pushed a topic branch "LP1022076" onto git://git.geda-project.org/pcb with the above mentioned patch.

Please "git fetch", "git checkout -b LP1022076" and test this patch for unwanted side effects.

Kind regards,

Bert Timmerman.

Changed in pcb:
status: New → In Progress
Changed in pcb:
assignee: nobody → Bert Timmerman (bert-timmerman)
Revision history for this message
Traumflug (mah-jump-ing) wrote :

Took a look at this.

The patch has no effect here. Reason is, drilling holes are usually vias with the "hole" flag, but the patch acts on Pins, only. Also, vias are usually fully covered with solder resist, no hole in the mask at all. As no sample was provided I'll have to make a hand-edited one.

I think this whould be a reasonable solution:

- Handle vias and pins the same regarding solder mask.

- Increase mask hole only if it was > zero before. Currently vias are handled this way.

- Take for both the bigger of thickness and drill hole as reference for gap calculation.

Revision history for this message
DJ Delorie (djdelorie) wrote :

Do fab houses drill such holes before or after soldermask? If after, there's no need to worry about it anyway...

Revision history for this message
Robert Drehmel (r58qft) wrote : Re: [Bug 1022076] Re: MinMaskGap() doesn't know about holes

On 08/23/2015 11:08 PM, Traumflug wrote:
> Took a look at this.
>
> The patch has no effect here. Reason is, drilling holes are usually vias
> with the "hole" flag, but the patch acts on Pins, only.

You obviously forgot about holes *in Element[]s*. These are *pins*
with the hole flag set. My tiny patch just fixes an obvious issue.
This is the exact kind of bikeshed discussions that remind me that
forking pcb some time ago was a good idea.

> Also, vias are
> usually fully covered with solder resist, no hole in the mask at all.

Many of the very big fab houses require vias to be *not covered* with
solder mask, Würth Elektronik for example.

> As
> no sample was provided I'll have to make a hand-edited one.
>
> I think this whould be a reasonable solution:
>
> - Handle vias and pins the same regarding solder mask.
>
> - Increase mask hole only if it was > zero before. Currently vias are
> handled this way.
>
> - Take for both the bigger of thickness and drill hole as reference for
> gap calculation.
>

Revision history for this message
Traumflug (mah-jump-ing) wrote :

Behaviour should be consistent across vias and pins now. Implementation as described above on banch LP1022076v2: http://git.geda-project.org/pcb/log/?h=LP1022076v2

Please test.

Revision history for this message
Traumflug (mah-jump-ing) wrote :

Hmm. While watching out for the next bug, I find this in global.h:

/* This is the extents of a Pin or Via, depending on whether it's a
   hole or not. */
#define PIN_SIZE(pinptr) (TEST_FLAG(HOLEFLAG, (pinptr)) \
                          ? (pinptr)->DrillingHole \
                          : (pinptr)->Thickness)

Reads like a nice fit for this case.

Revision history for this message
Bert Timmerman (bert-timmerman) wrote :

Hi Traumflug,

About commit dc7454e1c8c06f4bb4255acd423da41beb0180fa in topic branch "LP1022076v2": testing is a Good Thing (TM).

However, if you add a testfile for testing against regression (I see a pcb file added in "./tests/input"), then there also needs to be a test (to be executed by invoking "./run_tests.sh" in the directory "./tests") available.

Could you please add a command in "./tests/tests.list" for testing with the hid of your choice (I think Gcode or gerber are good candidates) and add a "golden" file too (the file with correct output to test against ;-) .

And please change the added filename into <hid name>_minmaskgap.pcb.

Kind regards,

Bert Timmerman.

Revision history for this message
Traumflug (mah-jump-ing) wrote :

Tests of actions aren't supported. At least that's written somewhere in the instructions on how to write tests.

And there is no "hid name", actions aren't reated to a HID.

So far this carefully crafted layout can be tested manually, only.

Revision history for this message
Traumflug (mah-jump-ing) wrote :

To follow this lack of testability: https://bugs.launchpad.net/pcb/+bug/1488220

Revision history for this message
Traumflug (mah-jump-ing) wrote :

Stuff on master. Enjoy!

Changed in pcb:
status: In Progress → Fix Committed
Traumflug (mah-jump-ing)
Changed in geda-project:
importance: Undecided → Medium
status: New → Fix Released
Revision history for this message
Chad Parker (parker-charles) wrote :

Commit released long ago

Changed in pcb:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.