From 86ead74c270be4ab235dbef61ff20effb45a7162 Mon Sep 17 00:00:00 2001 From: "Salvador E. Tropea" Date: Mon, 12 Oct 2020 16:31:47 -0300 Subject: [PATCH] Added KiBot warning filters. - Closes #15 - This patch also changes the logging initialization, that was broken at some recent point. - Also adds tests to ensure the mechanism used to avoid repeated warnings is working. --- CHANGELOG.md | 1 + README.md | 33 +++++++ docs/README.in | 33 +++++++ kibot/__main__.py | 8 +- kibot/config_reader.py | 24 ++--- kibot/globals.py | 43 +++++++++ kibot/gs.py | 1 + kibot/kiplot.py | 6 +- kibot/log.py | 42 ++++++--- kibot/misc.py | 88 +++++++++---------- kibot/optionable.py | 8 +- kibot/pre_filters.py | 38 ++++---- tests/test_plot/force_xlsx_error.py | 2 +- tests/test_plot/test_print_sch.py | 16 ++++ tests/test_plot/test_yaml_errors.py | 8 +- .../sch_no_inductors_1_filtered.kibot.yaml | 30 +++++++ 16 files changed, 282 insertions(+), 99 deletions(-) create mode 100644 kibot/globals.py create mode 100644 tests/yaml_samples/sch_no_inductors_1_filtered.kibot.yaml diff --git a/CHANGELOG.md b/CHANGELOG.md index a8490b56..aa101c24 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 separator used for the list of references. - Help for filters and variants. - Support for new `pcbnew_do export` options. +- Filters for KiBot warnings. ### Fixed - KiBom variants when using multiple variants and a components used more diff --git a/README.md b/README.md index 4fb40bd9..1577db92 100644 --- a/README.md +++ b/README.md @@ -20,6 +20,8 @@ To learn more about KiBot variants visit the [example repo](https://inti-cmnb.gi * [Filtering DRC and ERC errors](#filtering-drc-and-erc-errors) * [Default global options](#default-global-options) * [Default *output* option](#default-output-option) + * [Default *variant* option](#default-variant-option) + * [Filtering KiBot warnings](#filtering-kibot-warnings) * [Filters and variants](#filters-and-variants) * [Supported filters](#supported-filters) * [Examples for filters](#examples-for-filters) @@ -204,6 +206,37 @@ global: output: '%f_rev_%r-%i.%x' ``` +#### Default *variant* option + +This option controls the default variant applied to all the outputs. Example: + +```yaml +global: + variant: 'production' +``` + +#### Filtering KiBot warnings + +KiBot warnings are marked with `(Wn)` where *n* is the warning id. + +Some warnings are just recommendations and you could want to avoid them to focus on details that are more relevant to your project. +In this case you can define filters in a similar way used to [filter DRC/ERC errors](#filtering-drc-and-erc-errors). + +As an example, if you have the following warning: + +``` +WARNING:(W43) Missing component `l1:FooBar` +``` + +You can create the following filter to remove it: + +```yaml +global: + filters: + - number: 43 + regex: 'FooBar' +``` + ### Filters and variants The filters and variants are mechanism used to modify the circuit components. diff --git a/docs/README.in b/docs/README.in index 9b24e6fc..8e454844 100644 --- a/docs/README.in +++ b/docs/README.in @@ -20,6 +20,8 @@ To learn more about KiBot variants visit the [example repo](https://inti-cmnb.gi * [Filtering DRC and ERC errors](#filtering-drc-and-erc-errors) * [Default global options](#default-global-options) * [Default *output* option](#default-output-option) + * [Default *variant* option](#default-variant-option) + * [Filtering KiBot warnings](#filtering-kibot-warnings) * [Filters and variants](#filters-and-variants) * [Supported filters](#supported-filters) * [Examples for filters](#examples-for-filters) @@ -187,6 +189,37 @@ global: output: '%f_rev_%r-%i.%x' ``` +#### Default *variant* option + +This option controls the default variant applied to all the outputs. Example: + +```yaml +global: + variant: 'production' +``` + +#### Filtering KiBot warnings + +KiBot warnings are marked with `(Wn)` where *n* is the warning id. + +Some warnings are just recommendations and you could want to avoid them to focus on details that are more relevant to your project. +In this case you can define filters in a similar way used to [filter DRC/ERC errors](#filtering-drc-and-erc-errors). + +As an example, if you have the following warning: + +``` +WARNING:(W43) Missing component `l1:FooBar` +``` + +You can create the following filter to remove it: + +```yaml +global: + filters: + - number: 43 + regex: 'FooBar' +``` + ### Filters and variants The filters and variants are mechanism used to modify the circuit components. diff --git a/kibot/__main__.py b/kibot/__main__.py index 6b54eafa..906bdecd 100644 --- a/kibot/__main__.py +++ b/kibot/__main__.py @@ -67,6 +67,7 @@ from logging import DEBUG # Import log first to set the domain from . import log log.set_domain('kibot') +logger = log.init() from .gs import (GS) from .kiplot import (generate_outputs, load_actions, config_output) from .pre_base import (BasePreFlight) @@ -75,12 +76,12 @@ from .config_reader import (CfgYamlReader, print_outputs_help, print_output_help from .misc import (NO_PCB_FILE, NO_SCH_FILE, EXIT_BAD_ARGS, W_VARSCH, W_VARCFG, W_VARPCB, W_PYCACHE) from .docopt import docopt -logger = None has_macro = [ 'layer', 'drill_marks', 'fil_base', 'fil_generic', + 'globals', 'out_any_drill', 'out_any_layer', 'out_base', @@ -255,9 +256,8 @@ def main(): ver = 'KiBot '+__version__+' - '+__copyright__+' - License: '+__license__ args = docopt(__doc__, version=ver, options_first=True) - # Create a logger with the specified verbosity - global logger - logger = log.init(args.verbose, args.quiet) + # Set the specified verbosity + log.set_verbosity(logger, args.verbose, args.quiet) GS.debug_enabled = logger.getEffectiveLevel() <= DEBUG GS.debug_level = args.verbose diff --git a/kibot/config_reader.py b/kibot/config_reader.py index 45a29c51..d59bdd92 100644 --- a/kibot/config_reader.py +++ b/kibot/config_reader.py @@ -15,7 +15,7 @@ from collections import OrderedDict from .error import (KiPlotConfigurationError, config_error) from .kiplot import (load_board) -from .misc import (NO_YAML_MODULE, EXIT_BAD_ARGS, EXAMPLE_CFG, WONT_OVERWRITE, W_UNKGLOBAL) +from .misc import (NO_YAML_MODULE, EXIT_BAD_ARGS, EXAMPLE_CFG, WONT_OVERWRITE) from .gs import GS from .registrable import RegOutput, RegVariant, RegFilter from .pre_base import BasePreFlight @@ -150,28 +150,18 @@ class CfgYamlReader(object): config_error("In preflight '"+k+"': "+str(e)) BasePreFlight.add_preflight(o_pre) - @staticmethod - def _parse_global_str(k, v, current): - if not isinstance(v, str): - config_error("Global `{}` must be a string".format(k)) - if current: - logger.info('Using command line value `{}` for global option `{}`'.format(current, k)) - return current - return v - def _parse_global(self, gb): """ Get global options """ logger.debug("Parsing global options: {}".format(gb)) if not isinstance(gb, dict): config_error("Incorrect `global` section") # Parse all keys inside it - for k, v in gb.items(): - if k == 'output': - GS.global_output = self._parse_global_str(k, v, GS.global_output) - elif k == 'variant': - GS.global_variant = self._parse_global_str(k, v, GS.global_variant) - else: - logger.warning(W_UNKGLOBAL + "Unknown global option `{}`".format(k)) + glb = GS.global_opts_class() + glb.set_tree(gb) + try: + glb.config() + except KiPlotConfigurationError as e: + config_error("In `global` section: "+str(e)) def read(self, fstream): """ diff --git a/kibot/globals.py b/kibot/globals.py new file mode 100644 index 00000000..d932af32 --- /dev/null +++ b/kibot/globals.py @@ -0,0 +1,43 @@ +# -*- coding: utf-8 -*- +# Copyright (c) 2020 Salvador E. Tropea +# Copyright (c) 2020 Instituto Nacional de Tecnología Industrial +# License: GPL-3.0 +# Project: KiBot (formerly KiPlot) +from .gs import GS +from .macros import macros, document # noqa: F401 +from .pre_filters import FiltersOptions +from .log import get_logger, set_filters + + +class Globals(FiltersOptions): + """ Global options """ + def __init__(self): + super().__init__() + with document: + self.output = '' + """ Default pattern for output file names """ + self.variant = '' + """ Default variant to apply to all outputs """ + self.set_doc('filters', " [list(dict)] KiBot warnings to be ignored ") + self._filter_what = 'KiBot warnings' + self._unkown_is_error = True + self._error_context = 'global ' + + @staticmethod + def set_global(current, new_val, opt): + if current: + logger.info('Using command line value `{}` for global option `{}`'.format(current, opt)) + return current + if new_val: + return new_val + return current + + def config(self): + super().config() + GS.global_output = self.set_global(GS.global_output, self.output, 'output') + GS.global_variant = self.set_global(GS.global_variant, self.variant, 'variant') + set_filters(self.unparsed) + + +logger = get_logger(__name__) +GS.global_opts_class = Globals diff --git a/kibot/gs.py b/kibot/gs.py index b3dfa0f5..f6fef1df 100644 --- a/kibot/gs.py +++ b/kibot/gs.py @@ -64,6 +64,7 @@ class GS(object): global_from_cli = {} global_output = None global_variant = None + global_opts_class = None @staticmethod def set_sch(name): diff --git a/kibot/kiplot.py b/kibot/kiplot.py index e12ec6d2..319d1aee 100644 --- a/kibot/kiplot.py +++ b/kibot/kiplot.py @@ -65,10 +65,12 @@ def _import(name, path): exit(WRONG_INSTALL) -def _load_actions(path): +def _load_actions(path, load_internals=False): logger.debug("Importing from "+path) lst = glob(os.path.join(path, 'out_*.py')) + glob(os.path.join(path, 'pre_*.py')) lst += glob(os.path.join(path, 'var_*.py')) + glob(os.path.join(path, 'fil_*.py')) + if load_internals: + lst += [os.path.join(path, 'globals.py')] for p in lst: name = os.path.splitext(os.path.basename(p))[0] logger.debug("- Importing "+name) @@ -79,7 +81,7 @@ def load_actions(): """ Load all the available ouputs and preflights """ from kibot.mcpy import activate # activate.activate() - _load_actions(os.path.abspath(os.path.dirname(__file__))) + _load_actions(os.path.abspath(os.path.dirname(__file__)), True) home = os.environ.get('HOME') if home: dir = os.path.join(home, '.config', 'kiplot', 'plugins') diff --git a/kibot/log.py b/kibot/log.py index aadb76d0..17e5ef30 100644 --- a/kibot/log.py +++ b/kibot/log.py @@ -15,11 +15,13 @@ from io import StringIO # Default domain, base name for the tool domain = 'kilog' +filters = None def get_logger(name=None): """Get a module for a submodule or the root logger if no name is provided""" + # print('get_logger '+str(name)) if name: return logging.getLogger(domain+'.'+name) return logging.getLogger(domain) @@ -31,21 +33,38 @@ def set_domain(name): domain = name +def set_filters(f): + """Set the list of warning filters""" + global filters + filters = f + + class MyLogger(logging.Logger): warn_hash = {} - warn_tcnt = warn_cnt = 0 + warn_tcnt = warn_cnt = n_filtered = 0 def warning(self, msg, *args, **kwargs): MyLogger.warn_tcnt += 1 + # Get the message applying optional C style expansions if isinstance(msg, str) and len(args): buf = StringIO() buf.write(msg % args) buf = buf.getvalue() else: buf = str(msg) + # Avoid repeated warnings if buf in MyLogger.warn_hash: MyLogger.warn_hash[buf] += 1 return + # Apply the filters + if filters and buf.startswith('(W'): + pos_end = buf.find(')') + if pos_end > 0: + number = int(buf[2:pos_end]) + for f in filters: + if f.number == number and f.regex.search(buf): + MyLogger.n_filtered += 1 + return MyLogger.warn_cnt += 1 MyLogger.warn_hash[buf] = 1 if sys.version_info.major > 3 or (sys.version_info.major == 3 and sys.version_info.minor >= 8): @@ -55,27 +74,30 @@ class MyLogger(logging.Logger): def log_totals(self): if MyLogger.warn_cnt: - self.info('Found {} unique warning/s ({} total)'.format(MyLogger.warn_cnt, MyLogger.warn_tcnt)) + filt_msg = '' + if MyLogger.n_filtered: + filt_msg = ', {} filtered'.format(MyLogger.n_filtered) + self.info('Found {} unique warning/s ({} total{})'.format(MyLogger.warn_cnt, MyLogger.warn_tcnt, filt_msg)) -def init(verbose, quiet): - """Initialize the logging feature using a custom format and the specified - verbosity level""" - # Use a class to count and filter warnings - logging.setLoggerClass(MyLogger) +def set_verbosity(logger, verbose, quiet): # Choose the log level log_level = logging.INFO if verbose: log_level = logging.DEBUG if quiet: log_level = logging.WARNING - - logger = get_logger() logger.setLevel(log_level) + + +def init(): + """Initialize the logging feature using a custom format""" + # Use a class to count and filter warnings + logging.setLoggerClass(MyLogger) + logger = get_logger() ch = logging.StreamHandler() ch.setFormatter(CustomFormatter()) logger.addHandler(ch) - return logger diff --git a/kibot/misc.py b/kibot/misc.py index 18bf1daf..9afdccf5 100644 --- a/kibot/misc.py +++ b/kibot/misc.py @@ -87,50 +87,50 @@ DNC = { "fixed", } -W_VARSCH = '(0) ' -W_VARCFG = '(1) ' -W_VARPCB = '(2) ' -W_PYCACHE = '(3) ' -W_FIELDCONF = '(4) ' -W_NOHOME = '(5) ' -W_NOUSER = '(6) ' -W_BADSYS = '(7) ' -W_NOCONFIG = '(8) ' -W_NOKIENV = '(9) ' -W_NOLIBS = '(10) ' -W_NODEFSYMLIB = '(11) ' -W_UNKGLOBAL = '(12) ' -W_PCBNOSCH = '(13) ' -W_NONEEDSKIP = '(14) ' -W_UNKOPS = '(15) ' -W_AMBLIST = '(16) ' -W_UNRETOOL = '(17) ' -W_USESVG2 = '(18) ' -W_USEIMAGICK = '(19) ' -W_BADVAL1 = '(20) ' -W_BADVAL2 = '(21) ' -W_BADVAL3 = '(22) ' -W_BADPOLI = '(23) ' -W_POLICOORDS = '(24) ' -W_BADSQUARE = '(25) ' -W_BADCIRCLE = '(26) ' -W_BADARC = '(27) ' -W_BADTEXT = '(28) ' -W_BADPIN = '(29) ' -W_BADCOMP = '(30) ' -W_BADDRAW = '(31) ' -W_UNKDCM = '(32) ' -W_UNKAR = '(33) ' -W_ARNOPATH = '(34) ' -W_ARNOREF = '(35) ' -W_MISCFLD = '(36) ' -W_EXTRASPC = '(37) ' -W_NOLIB = '(38) ' -W_INCPOS = '(39) ' -W_NOANNO = '(40) ' -W_MISSLIB = '(41) ' -W_MISSDCM = '(42) ' -W_MISSCMP = '(43) ' +W_VARCFG = '(W1) ' +W_VARPCB = '(W2) ' +W_PYCACHE = '(W3) ' +W_FIELDCONF = '(W4) ' +W_NOHOME = '(W5) ' +W_NOUSER = '(W6) ' +W_BADSYS = '(W7) ' +W_NOCONFIG = '(W8) ' +W_NOKIENV = '(W9) ' +W_NOLIBS = '(W10) ' +W_NODEFSYMLIB = '(W11) ' +W_UNKGLOBAL = '(W12) ' +W_PCBNOSCH = '(W13) ' +W_NONEEDSKIP = '(W14) ' +W_UNKOPS = '(W15) ' +W_AMBLIST = '(W16) ' +W_UNRETOOL = '(W17) ' +W_USESVG2 = '(W18) ' +W_USEIMAGICK = '(W19) ' +W_BADVAL1 = '(W20) ' +W_BADVAL2 = '(W21) ' +W_BADVAL3 = '(W22) ' +W_BADPOLI = '(W23) ' +W_POLICOORDS = '(W24) ' +W_BADSQUARE = '(W25) ' +W_BADCIRCLE = '(W26) ' +W_BADARC = '(W27) ' +W_BADTEXT = '(W28) ' +W_BADPIN = '(W29) ' +W_BADCOMP = '(W30) ' +W_BADDRAW = '(W31) ' +W_UNKDCM = '(W32) ' +W_UNKAR = '(W33) ' +W_ARNOPATH = '(W34) ' +W_ARNOREF = '(W35) ' +W_MISCFLD = '(W36) ' +W_EXTRASPC = '(W37) ' +W_NOLIB = '(W38) ' +W_INCPOS = '(W39) ' +W_NOANNO = '(W40) ' +W_MISSLIB = '(W41) ' +W_MISSDCM = '(W42) ' +W_MISSCMP = '(W43) ' +W_VARSCH = '(W44) ' class Rect(object): diff --git a/kibot/optionable.py b/kibot/optionable.py index 1273320c..22728dd0 100644 --- a/kibot/optionable.py +++ b/kibot/optionable.py @@ -28,6 +28,7 @@ class Optionable(object): def __init__(self): self._unkown_is_error = False + self._error_context = '' self._tree = {} super().__init__() if GS.global_output is not None and getattr(self, 'output', None): @@ -74,6 +75,9 @@ class Optionable(object): doc = getattr(self, '_help_'+name).strip() setattr(self, '_help_'+name, doc+'.\n'+text) + def set_doc(self, name, text): + setattr(self, '_help_'+name, text) + @staticmethod def _typeof(v): if isinstance(v, bool): @@ -97,8 +101,8 @@ class Optionable(object): # Map known attributes and avoid mapping private ones if (k[0] == '_') or (k not in attrs): if self._unkown_is_error: - raise KiPlotConfigurationError("Unknown option `{}`".format(k)) - logger.warning(W_UNKOPS + "Unknown option `{}`".format(k)) + raise KiPlotConfigurationError("Unknown {}option `{}`".format(self._error_context, k)) + logger.warning(W_UNKOPS + "Unknown {}option `{}`".format(self._error_context, k)) continue # Check the data type cur_doc, alias, is_alias = self.get_doc(k) diff --git a/kibot/pre_filters.py b/kibot/pre_filters.py index f6dcd308..ec1be274 100644 --- a/kibot/pre_filters.py +++ b/kibot/pre_filters.py @@ -5,6 +5,7 @@ # Project: KiBot (formerly KiPlot) # Contributors: Leandro Heck (@leoheck) import os +import re from .gs import GS from .error import KiPlotConfigurationError from .optionable import Optionable @@ -41,25 +42,32 @@ class FiltersOptions(Optionable): with document: self.filters = FilterOptions """ [list(dict)] DRC/ERC errors to be ignored """ + self._filter_what = 'DRC/ERC errors' def config(self): super().config() parsed = None - for f in self.filters: - where = ' (in `{}` filter)'.format(f.filter) if f.filter else '' - number = f.number - if not number: - raise KiPlotConfigurationError('Missing `number`'+where) - regex = f.regex - if regex == 'None': - raise KiPlotConfigurationError('Missing `regex`'+where) - comment = f.filter - logger.debug("Adding DRC/ERC filter '{}','{}','{}'".format(comment, number, regex)) - if parsed is None: - parsed = '' - if comment: - parsed += '# '+comment+'\n' - parsed += '{},{}\n'.format(number, regex) + self.unparsed = None + if not isinstance(self.filters, type): + for f in self.filters: + where = ' (in `{}` filter)'.format(f.filter) if f.filter else '' + number = f.number + if not number: + raise KiPlotConfigurationError('Missing `number`'+where) + regex = f.regex + if regex == 'None': + raise KiPlotConfigurationError('Missing `regex`'+where) + comment = f.filter + logger.debug("Adding {} filter '{}','{}','{}'".format(self._filter_what, comment, number, regex)) + if parsed is None: + parsed = '' + if comment: + parsed += '# '+comment+'\n' + parsed += '{},{}\n'.format(number, regex) + f.regex = re.compile(regex) + # If the list is valid make a copy for the warnings filter + if parsed: + self.unparsed = self.filters self.filters = parsed diff --git a/tests/test_plot/force_xlsx_error.py b/tests/test_plot/force_xlsx_error.py index dd4335cb..3ac95dc2 100755 --- a/tests/test_plot/force_xlsx_error.py +++ b/tests/test_plot/force_xlsx_error.py @@ -10,7 +10,7 @@ sys.modules['xlsxwriter'] = None # Initialize the logger from kibot import log log.set_domain('kibot') -logger = log.init(True, False) +logger = log.init() logger.debug("Testing bom_writer without xlsxwriter") # Import the module to test diff --git a/tests/test_plot/test_print_sch.py b/tests/test_plot/test_print_sch.py index 13604b7c..f58a688c 100644 --- a/tests/test_plot/test_print_sch.py +++ b/tests/test_plot/test_print_sch.py @@ -142,6 +142,22 @@ def test_sch_missing(): ctx.search_err("Component .?Resistor.? doesn't specify its library") ctx.search_err("Missing component .?l1:FooBar.?") ctx.search_err("Missing component(.*)Resistor", invert=True) + ctx.search_err(r"Found 2 unique warning/s \(3 total\)") + ctx.clean_up() + + +def test_sch_missing_filtered(): + """ R1 exists in l1.lib, but the lib isn't specified. + R2 is bogus, completely missing """ + prj = 'missing' + ctx = context.TestContextSCH('test_sch_missing', prj, 'sch_no_inductors_1_filtered', PDF_DIR) + ctx.run() + o_name = os.path.join(NI_DIR, prj+'.sch') + ctx.expect_out_file(o_name) + ctx.search_err("Component .?Resistor.? doesn't specify its library") + ctx.search_err("Missing component .?l1:FooBar.?", invert=True) + ctx.search_err("Missing component(.*)Resistor", invert=True) + ctx.search_err(r"Found 1 unique warning/s \(3 total, 2 filtered\)") ctx.clean_up() diff --git a/tests/test_plot/test_yaml_errors.py b/tests/test_plot/test_yaml_errors.py index 40a705aa..ed75192b 100644 --- a/tests/test_plot/test_yaml_errors.py +++ b/tests/test_plot/test_yaml_errors.py @@ -475,15 +475,15 @@ def test_wrong_global(): def test_goutput_not_string(): - ctx = context.TestContext('GOutNotString', 'bom', 'error_goutput_not_string', '') + ctx = context.TestContext('test_goutput_not_string', 'bom', 'error_goutput_not_string', '') ctx.run(EXIT_BAD_CONFIG) - assert ctx.search_err("Global .?output.? must be a string") + assert ctx.search_err("Option .?output.? must be a string") ctx.clean_up() def test_unk_global(): - ctx = context.TestContext('UnkGlobal', 'bom', 'error_unk_global', '') - ctx.run() + ctx = context.TestContext('test_unk_global', 'bom', 'error_unk_global', '') + ctx.run(EXIT_BAD_CONFIG) assert ctx.search_err("Unknown global option") ctx.clean_up() diff --git a/tests/yaml_samples/sch_no_inductors_1_filtered.kibot.yaml b/tests/yaml_samples/sch_no_inductors_1_filtered.kibot.yaml new file mode 100644 index 00000000..d8364a80 --- /dev/null +++ b/tests/yaml_samples/sch_no_inductors_1_filtered.kibot.yaml @@ -0,0 +1,30 @@ +# Example KiBot config file +kibot: + version: 1 + +global: + filters: + - number: 43 + regex: FooBar + +filters: + - name: 'no_inductor' + comment: 'Inductors removed' + type: generic + exclude_refs: + - L* + +variants: + - name: 'no_inductor' + comment: 'Inductors removed' + type: kibom + file_id: '_(no_L)' + dnf_filter: 'no_inductor' + +outputs: + - name: 'no_inductor' + comment: "Inductors removed" + type: sch_variant + dir: no_inductor + options: + variant: 'no_inductor'