Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Treat some DRC Errors as Warnings #18

Open
leoheck opened this issue Jun 6, 2020 · 14 comments
Open

Treat some DRC Errors as Warnings #18

leoheck opened this issue Jun 6, 2020 · 14 comments

Comments

@leoheck
Copy link

leoheck commented Jun 6, 2020

@set-soft

You have implemented a new way to filter out DRC issues with an external file in your fork of the kicad-automation-scripts following the discussion started on this issue INTI-CMNB/kicad_ci_test#5 (comment)

This approach looks promising since the designer would have control being able to decide if the DRC errors sound. For instance, when the designer is implementing something not expected by the DRC checker.

This is just a reminder, to add this functionality in your fork of the kiplot if possible.

@leoheck
Copy link
Author

leoheck commented Jun 6, 2020

An example using pcbnew_do instead of the kiplot since it does not implement it yet.
Since these errors were filtered out because they were intentional, the DRC Step is passing.

Screenshot from 2020-06-06 18-57-28

@set-soft
Copy link

set-soft commented Jun 9, 2020

Is almost ready in this branch:
https://github.com/INTI-CMNB/kiplot/tree/drc_errors
Will merge tomorrow.

@leoheck
Copy link
Author

leoheck commented Jun 9, 2020 via email

@leoheck
Copy link
Author

leoheck commented Jun 9, 2020

I am not sure how to use it with kiplot.
kiplot --help and README.md do not have any extra info about it. I will call this a bug.

image

Do you mind explaining this? I believe that I can also add something in the *.kiplot.yaml
You don't need to do it here, just update there (for other users too), and let me know. So I can get the info from there validating that I understood how to do it.

@leoheck
Copy link
Author

leoheck commented Jun 9, 2020

Found the example, I am going to test it.

image

@leoheck
Copy link
Author

leoheck commented Jun 9, 2020

Let me check.

  • filter is a user-defined string.
  • number What is this? the Error type?
  • regexp Is the error to filter

For instance, I have these 2 errors.

** Found 2 DRC errors **
ErrType(45): Courtyards overlap
    @(144.361 mm, 101.752 mm): Footprint C16 on F.Cu
    @(144.825 mm, 101.244 mm): Footprint C19 on F.Cu
ErrType(45): Courtyards overlap
    @(159.885 mm, 97.663 mm): Footprint R4 on F.Cu
    @(160.393 mm, 97.191 mm): Footprint C21 on F.Cu

How do I create a filter to filter out jut the 1st one?

I believe a second regexp (you are calling regex but regexp looks more common) is missing there. Or is it a list? As in:

regexp : [ 'Regexp_1', 'Regexp_2' ] ?

@set-soft
Copy link

  • filter is a user-defined string. Yes, and is just a comment for documentation purposes.
  • number What is this? the Error type?. Yes is the error type.
  • regex Is the error to filter. Yes

The regex term gives 16.1M hits on Google, regexp 5.8M. So I'll keep regex.

And nope, only one regular expression. Regular expressions are really powerful, you don't need more than one. Perhaps we need to adjust the flags in the code, didn't try complex matches, but one should be enough.

I think your case should be covered by:

  filters:
    - filter: 'Ignore C16/C19 overlap'
      number: 45
      regex: 'Footprint C16'
    - filter: 'Ignore R4/C21 overlap'
      number: 45
      regex: 'Footprint R4'

@leoheck
Copy link
Author

leoheck commented Jun 10, 2020

Thank you, this feature is going to be really useful.

I am thinking here that the way the error is being described does not look complete yet.

For instance, the error I want to filter out is composed of 2 overlapping items

# 1st error (this one I want to filter out)
ErrType(45): Courtyards overlap
    @(144.361 mm, 101.752 mm): Footprint C16 on F.Cu
    @(144.825 mm, 101.244 mm): Footprint C19 on F.Cu

I can also have a second error like thie one:

# 2nd error (this one have to fail, since was not intentional)
ErrType(45): Courtyards overlap <=== same as in 1st error
    @(144.361 mm, 101.752 mm): Footprint C16 on F.Cu  <=== same as in 1st error
    @(144.825 mm, 101.244 mm): Footprint C201 on F.Cu <== Not the Same!

It is the same ErrType 45, Courtyards overlap
And it also references the C16 but they differ because of the second element.

For instance, if we could add a list of regexp it would be more descriptive and complete.
And this should be evaluated as an AND operation meaning that all the items on that list have to match to have the ERROR filtered.

  filters:
    - filter: 'Ignore C16/C19 overlap'
      number: 45
      regex: ['Footprint C16', 'Footprint C19'] 

Does it make sense or I am missing something?

Also, I believe it would be nice to improve the names of the tags since yaml is there for that, to be descriptive.
filter -> filter_msg
number -> error_number ---> Even better error_type since DRC shows # ErrType(45):


Totally off-topic

The regex term gives 16.1M hits on Google, regexp 5.8M. So I'll keep regex.

Google Fight! haha, the first one is a substring of the second. haha

This was just a simple tip.
Democracy is not always the right choice. I am learning this with the president of my country.

Reg is one of the abbreviations of the word regular
Expr|Exp are abbreviations of the word expression
image

@leoheck
Copy link
Author

leoheck commented Jun 11, 2020

The current output is this:

- Running the DRC

Maybe it is good to show to the user the filter message to remember the filters being applied as in:

- Running the DRC
Ignore C16/C19 overlap
Ignore R4/C21 overlap

@set-soft
Copy link

Thank you, this feature is going to be really useful.

:-)

I am thinking here that the way the error is being described does not look complete yet.

For instance, the error I want to filter out is composed of 2 overlapping items

# 1st error (this one I want to filter out)
ErrType(45): Courtyards overlap
    @(144.361 mm, 101.752 mm): Footprint C16 on F.Cu
    @(144.825 mm, 101.244 mm): Footprint C19 on F.Cu

I can also have a second error like thie one:

# 2nd error (this one have to fail, since was not intentional)
ErrType(45): Courtyards overlap <=== same as in 1st error
    @(144.361 mm, 101.752 mm): Footprint C16 on F.Cu  <=== same as in 1st error
    @(144.825 mm, 101.244 mm): Footprint C201 on F.Cu <== Not the Same!

It is the same ErrType 45, Courtyards overlap
And it also references the C16 but they differ because of the second element.

Try the following regex:

(?s)C16(.*)C19

Regular expressions are really powerful, you can do logic operations too.
As an example:

(?s)C16(.*)(C19|C201)

Matches both, but not a violation involving C16 and a component that isn't C19 or C201.

Also, I believe it would be nice to improve the names of the tags since yaml is there for that, to be descriptive.
filter -> filter_msg
number -> error_number ---> Even better error_type since DRC shows # ErrType(45):

I really preffer less typing.

Totally off-topic

The regex term gives 16.1M hits on Google, regexp 5.8M. So I'll keep regex.

Google Fight! haha, the first one is a substring of the second. haha

This was just a simple tip.
Democracy is not always the right choice. I am learning this with the president of my country.

I know our former president is a friend of yours ;-)

Reg is one of the abbreviations of the word regular
Expr|Exp are abbreviations of the word expression

I understand, but we are talking about Python regular expressions in this context the term is regex. Here is the documentation:

https://docs.python.org/3/library/re.html

@set-soft
Copy link

The current output is this:

- Running the DRC

Maybe it is good to show to the user the filter message to remember the filters being applied as in:

- Running the DRC
Ignore C16/C19 overlap
Ignore R4/C21 overlap

This is more complex than it looks.
The messages you mention are suitable for "information" verbosity level of KiPlot. But the tool that knows it is pcbnew_do and for it this implies much more messages.

I'm thinking about adding a warning message

set-soft added a commit to INTI-CMNB/KiBot that referenced this issue Jun 11, 2020
set-soft added a commit to INTI-CMNB/KiBot that referenced this issue Jun 11, 2020
@leoheck
Copy link
Author

leoheck commented Jun 12, 2020

Cool, thanks. I know a little bit of regexp, so my question was more related to know how you are applying it. Are you applying it in the whole DRC log or you are applying each pattern for each DRC error? This changes the way you build the expression.

@set-soft
Copy link

Cool, thanks. I know a little bit of regexp, so my question was more related to know how you are applying it. Are you applying it in the whole DRC log or you are applying each pattern for each DRC error? This changes the way you build the expression.

Each error is processed individually. As one string including the \n character to separate the lines.

@set-soft
Copy link

Hi @leoheck!
Now the filters are parsed using the "Optionable" class, this class enables aliases. So now your propposed key names are aliases for the original ones.

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

No branches or pull requests

2 participants