From 2385013bddb1e8699e0215d151c8dd06aa4d7d82 Mon Sep 17 00:00:00 2001 From: "Salvador E. Tropea" Date: Thu, 18 Jan 2024 13:06:13 -0300 Subject: [PATCH] [DRC/ERC][Added] Individual directory for reports - Also joined ignore_unconnected and erc_warnings options - Deprecated separated options Closes #562 --- CHANGELOG.md | 2 + docs/samples/generic_plot.kibot.yaml | 10 ++-- docs/source/configuration/sup_preflights.rst | 25 ++++++-- kibot/pre_erc_warnings.py | 3 +- kibot/pre_ignore_unconnected.py | 3 +- kibot/pre_run_drc.py | 53 +++++++++++++---- kibot/pre_run_erc.py | 61 ++++++++++++++------ tests/test_plot/test_preflight.py | 27 ++++++++- tests/yaml_samples/drc_unco_2.kibot.yaml | 8 +++ tests/yaml_samples/erc_no_w_2.kibot.yaml | 18 ++++++ 10 files changed, 169 insertions(+), 41 deletions(-) create mode 100644 tests/yaml_samples/drc_unco_2.kibot.yaml create mode 100644 tests/yaml_samples/erc_no_w_2.kibot.yaml diff --git a/CHANGELOG.md b/CHANGELOG.md index 7fb0b01c..90a417fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `download_lcsc` option to disable LCSC 3D model download (See #415) - Problems when creating a colored resistor, but we didn't have a cache yet (i.e. no model downloaded) #553 +- Preflights: + - Individual directory for the ERC and DRC reports (#562) - BoM: - Support for ${field} expansion. (#471) - LCSC links (SchrodingersGat/KiBoM#190) diff --git a/docs/samples/generic_plot.kibot.yaml b/docs/samples/generic_plot.kibot.yaml index 766bf4b3..8b2d5c07 100644 --- a/docs/samples/generic_plot.kibot.yaml +++ b/docs/samples/generic_plot.kibot.yaml @@ -29,7 +29,8 @@ preflight: # The original PCB remains unchanged. If you need to abort when the zone fill # creates significant changes to a layer use the CheckZoneFill internal template. check_zone_fills: true - # [boolean=false] Option for `run_erc`. ERC warnings are considered errors. + # [boolean=false] **Deprecated**, use the `warnings_as_errors` option from `run_erc`. + # Option for `run_erc`. ERC warnings are considered errors. erc_warnings: false # [boolean=false] Fill all zones again and save the PCB. fill_zones: true @@ -40,7 +41,8 @@ preflight: - filter: 'Filter description' error: '10' regex: 'Regular expression to match' - # [boolean=false] Option for `run_drc`. Ignores the unconnected nets. Useful if you didn't finish the routing. + # [boolean=false] **Deprecated**, use the `ignore_unconnected` option from `run_drc`. + # Option for `run_drc`. Ignores the unconnected nets. Useful if you didn't finish the routing. # It will also ignore KiCad 6 warnings. ignore_unconnected: false # [dict] Replaces tags in the PCB. I.e. to insert the git hash or last revision date. @@ -53,7 +55,7 @@ preflight: command: 'git log -1 --format="%h" "$KIBOT_PCB_NAME"' before: 'Git hash: <' after: '>' - # [boolean=false] Runs the DRC (Distance Rules Check). To ensure we have a valid PCB. + # [boolean=false|dict] Runs the DRC (Distance Rules Check). To ensure we have a valid PCB. # The report file name is controlled by the global output pattern (%i=drc %x=txt). # Note that the KiCad 6+ *Test for parity between PCB and schematic* option is not supported. # If you need to check the parity use the `update_xml` preflight. @@ -61,7 +63,7 @@ preflight: # This will change in the future. # If you use DRC exclusions please consult the `drc_exclusions_workaround` global option. run_drc: true - # [boolean=false] Runs the ERC (Electrical Rules Check). To ensure the schematic is electrically correct. + # [boolean=false|dict] Runs the ERC (Electrical Rules Check). To ensure the schematic is electrically correct. # The report file name is controlled by the global output pattern (%i=erc %x=txt). run_erc: true # [dict] Replaces tags in the schematic. I.e. to insert the git hash or last revision date. diff --git a/docs/source/configuration/sup_preflights.rst b/docs/source/configuration/sup_preflights.rst index e02b35f3..2d0fb9a1 100644 --- a/docs/source/configuration/sup_preflights.rst +++ b/docs/source/configuration/sup_preflights.rst @@ -32,7 +32,8 @@ Supported preflights - **check_zone_fills**: :index:`: ` [boolean=false] Zones are filled before doing any operation involving PCB layers. The original PCB remains unchanged. If you need to abort when the zone fill creates significant changes to a layer use the CheckZoneFill internal template. -- **erc_warnings**: :index:`: ` [boolean=false] Option for `run_erc`. ERC warnings are considered errors. +- **erc_warnings**: :index:`: ` [boolean=false] **Deprecated**, use the `warnings_as_errors` option from `run_erc`. + Option for `run_erc`. ERC warnings are considered errors. - **fill_zones**: :index:`: ` [boolean=false] Fill all zones again and save the PCB. - **filters**: :index:`: ` [list(dict)] A list of entries to filter out ERC/DRC messages. Note that ignored errors will become KiBot warnings (i.e. `(W058) ...`). |br| @@ -50,7 +51,8 @@ Supported preflights - ``regex`` :index:`: ` [string=''] Regular expression to match the text for the error we want to exclude. - *regexp* :index:`: ` Alias for regex. -- **ignore_unconnected**: :index:`: ` [boolean=false] Option for `run_drc`. Ignores the unconnected nets. Useful if you didn't finish the routing. +- **ignore_unconnected**: :index:`: ` [boolean=false] **Deprecated**, use the `ignore_unconnected` option from `run_drc`. + Option for `run_drc`. Ignores the unconnected nets. Useful if you didn't finish the routing. |br| It will also ignore KiCad 6 warnings. - **pcb_replace**: :index:`: ` [dict] Replaces tags in the PCB. I.e. to insert the git hash or last revision date. This is useful for KiCad 5, use `set_text_variables` when using KiCad 6. |br| @@ -79,15 +81,30 @@ Supported preflights - ``text`` :index:`: ` [string=''] Text to insert instead of the tag. -- **run_drc**: :index:`: ` [boolean=false] Runs the DRC (Distance Rules Check). To ensure we have a valid PCB. +- **run_drc**: :index:`: ` [boolean=false|dict] Runs the DRC (Distance Rules Check). To ensure we have a valid PCB. The report file name is controlled by the global output pattern (%i=drc %x=txt). |br| Note that the KiCad 6+ *Test for parity between PCB and schematic* option is not supported. |br| If you need to check the parity use the `update_xml` preflight. |br| KiCad 6 introduced `warnings` they are currently counted be the `unconnected` counter of KiBot. |br| This will change in the future. |br| If you use DRC exclusions please consult the `drc_exclusions_workaround` global option. -- **run_erc**: :index:`: ` [boolean=false] Runs the ERC (Electrical Rules Check). To ensure the schematic is electrically correct. + + - Valid keys: + + - ``dir`` :index:`: ` [string=''] Sub-directory for the report. + - ``enabled`` :index:`: ` [boolean=true] Enable the DRC. This is the replacement for the boolean value. + - ``ignore_unconnected`` :index:`: ` [boolean=false] Ignores the unconnected nets. Useful if you didn't finish the routing. + It will also ignore KiCad 6 warnings. + +- **run_erc**: :index:`: ` [boolean=false|dict] Runs the ERC (Electrical Rules Check). To ensure the schematic is electrically correct. The report file name is controlled by the global output pattern (%i=erc %x=txt). + + - Valid keys: + + - ``dir`` :index:`: ` [string=''] Sub-directory for the report. + - ``enabled`` :index:`: ` [boolean=true] Enable the ERC. This is the replacement for the boolean value. + - ``warnings_as_errors`` :index:`: ` [boolean=false] ERC warnings are considered errors. + - **sch_replace**: :index:`: ` [dict] Replaces tags in the schematic. I.e. to insert the git hash or last revision date. This is useful for KiCad 5, use `set_text_variables` when using KiCad 6. |br| This preflight modifies the schematics. Even when a back-up is done use it carefully. diff --git a/kibot/pre_erc_warnings.py b/kibot/pre_erc_warnings.py index 4bb62be1..759baa77 100644 --- a/kibot/pre_erc_warnings.py +++ b/kibot/pre_erc_warnings.py @@ -9,7 +9,8 @@ from .error import (KiPlotConfigurationError) @pre_class class ERC_Warnings(BasePreFlight): # noqa: F821 - """ [boolean=false] Option for `run_erc`. ERC warnings are considered errors """ + """ [boolean=false] **Deprecated**, use the `warnings_as_errors` option from `run_erc`. + Option for `run_erc`. ERC warnings are considered errors """ def __init__(self, name, value): super().__init__(name, value) if not isinstance(value, bool): diff --git a/kibot/pre_ignore_unconnected.py b/kibot/pre_ignore_unconnected.py index f2e5df77..345b63cb 100644 --- a/kibot/pre_ignore_unconnected.py +++ b/kibot/pre_ignore_unconnected.py @@ -9,7 +9,8 @@ from .error import (KiPlotConfigurationError) @pre_class class Ignore_Unconnected(BasePreFlight): # noqa: F821 - """ [boolean=false] Option for `run_drc`. Ignores the unconnected nets. Useful if you didn't finish the routing. + """ [boolean=false] **Deprecated**, use the `ignore_unconnected` option from `run_drc`. + Option for `run_drc`. Ignores the unconnected nets. Useful if you didn't finish the routing. It will also ignore KiCad 6 warnings """ def __init__(self, name, value): super().__init__(name, value) diff --git a/kibot/pre_run_drc.py b/kibot/pre_run_drc.py index b5025fe6..adf679fb 100644 --- a/kibot/pre_run_drc.py +++ b/kibot/pre_run_drc.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- -# Copyright (c) 2020-2023 Salvador E. Tropea -# Copyright (c) 2020-2023 Instituto Nacional de Tecnología Industrial -# License: GPL-3.0 +# Copyright (c) 2020-2024 Salvador E. Tropea +# Copyright (c) 2020-2024 Instituto Nacional de Tecnología Industrial +# License: AGPL-3.0 # Project: KiBot (formerly KiPlot) """ Dependencies: @@ -10,7 +10,7 @@ Dependencies: version: 2.0.0 """ import os -from .macros import macros, pre_class # noqa: F401 +from .macros import macros, document, pre_class # noqa: F401 from .error import KiPlotConfigurationError from .gs import GS from .optionable import Optionable @@ -22,9 +22,24 @@ from .log import get_logger logger = get_logger(__name__) +class Run_DRCOptions(Optionable): + """ DRC options """ + def __init__(self): + super().__init__() + with document: + self.enabled = True + """ Enable the DRC. This is the replacement for the boolean value """ + self.dir = '' + """ Sub-directory for the report """ + self.ignore_unconnected = False + """ Ignores the unconnected nets. Useful if you didn't finish the routing. + It will also ignore KiCad 6 warnings """ + self._unknown_is_error = True + + @pre_class class Run_DRC(BasePreFlight): # noqa: F821 - """ [boolean=false] Runs the DRC (Distance Rules Check). To ensure we have a valid PCB. + """ [boolean=false|dict] Runs the DRC (Distance Rules Check). To ensure we have a valid PCB. The report file name is controlled by the global output pattern (%i=drc %x=txt). Note that the KiCad 6+ *Test for parity between PCB and schematic* option is not supported. If you need to check the parity use the `update_xml` preflight. @@ -33,9 +48,19 @@ class Run_DRC(BasePreFlight): # noqa: F821 If you use DRC exclusions please consult the `drc_exclusions_workaround` global option """ def __init__(self, name, value): super().__init__(name, value) - if not isinstance(value, bool): - raise KiPlotConfigurationError('must be boolean') - self._enabled = value + if isinstance(value, bool): + self._enabled = value + self._dir = '' + self._ignore_unconnected = False + elif isinstance(value, dict): + f = Run_DRCOptions() + f.set_tree(value) + f.config(self) + self._enabled = f.enabled + self._dir = f.dir + self._ignore_unconnected = f.ignore_unconnected + else: + raise KiPlotConfigurationError('must be boolean or dict') self._pcb_related = True self._expand_id = 'drc' self._expand_ext = 'txt' @@ -48,7 +73,11 @@ class Run_DRC(BasePreFlight): # noqa: F821 out_dir = self.expand_dirname(GS.out_dir) if GS.global_dir and GS.global_use_dir_for_preflights: out_dir = os.path.join(out_dir, self.expand_dirname(GS.global_dir)) - return [os.path.abspath(os.path.join(out_dir, name))] + return [os.path.abspath(os.path.join(out_dir, self._dir, name))] + + @classmethod + def get_doc(cls): + return cls.__doc__, Run_DRCOptions def run(self): command = self.ensure_tool('KiAuto') @@ -62,14 +91,14 @@ class Run_DRC(BasePreFlight): # noqa: F821 output = self.get_targets()[0] os.makedirs(os.path.dirname(output), exist_ok=True) logger.debug('DRC report: '+output) - cmd = [command, 'run_drc', '-o', output] + cmd = [command, 'run_drc', '-o', os.path.basename(output)] if GS.filter_file: cmd.extend(['-f', GS.filter_file]) if GS.global_drc_exclusions_workaround: cmd.append('-F') - if BasePreFlight.get_option('ignore_unconnected'): # noqa: F821 + if self._ignore_unconnected or BasePreFlight.get_option('ignore_unconnected'): # noqa: F821 cmd.append('-i') - cmd.extend([GS.pcb_file, self.expand_dirname(GS.out_dir)]) + cmd.extend([GS.pcb_file, os.path.dirname(output)]) # If we are in verbose mode enable debug in the child cmd = self.add_extra_options(cmd) logger.info('- Running the DRC') diff --git a/kibot/pre_run_erc.py b/kibot/pre_run_erc.py index 14a15efb..9216bbde 100644 --- a/kibot/pre_run_erc.py +++ b/kibot/pre_run_erc.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- -# Copyright (c) 2020-2023 Salvador E. Tropea -# Copyright (c) 2020-2023 Instituto Nacional de Tecnología Industrial -# License: GPL-3.0 +# Copyright (c) 2020-2024 Salvador E. Tropea +# Copyright (c) 2020-2024 Instituto Nacional de Tecnología Industrial +# License: AGPL-3.0 # Project: KiBot (formerly KiPlot) """ Dependencies: @@ -13,7 +13,7 @@ Dependencies: import os from shutil import move from tempfile import NamedTemporaryFile -from .macros import macros, pre_class # noqa: F401 +from .macros import macros, document, pre_class # noqa: F401 from .gs import GS from .optionable import Optionable from .kiplot import load_sch @@ -24,15 +24,39 @@ from .log import get_logger logger = get_logger(__name__) +class Run_ERCOptions(Optionable): + """ ERC options """ + def __init__(self): + super().__init__() + with document: + self.enabled = True + """ Enable the ERC. This is the replacement for the boolean value """ + self.dir = '' + """ Sub-directory for the report """ + self.warnings_as_errors = False + """ ERC warnings are considered errors """ + self._unknown_is_error = True + + @pre_class class Run_ERC(BasePreFlight): # noqa: F821 - """ [boolean=false] Runs the ERC (Electrical Rules Check). To ensure the schematic is electrically correct. + """ [boolean=false|dict] Runs the ERC (Electrical Rules Check). To ensure the schematic is electrically correct. The report file name is controlled by the global output pattern (%i=erc %x=txt) """ def __init__(self, name, value): super().__init__(name, value) - if not isinstance(value, bool): - raise KiPlotConfigurationError('must be boolean') - self._enabled = value + if isinstance(value, bool): + self._enabled = value + self._dir = '' + self._warnings_as_errors = False + elif isinstance(value, dict): + f = Run_ERCOptions() + f.set_tree(value) + f.config(self) + self._enabled = f.enabled + self._dir = f.dir + self._warnings_as_errors = f.warnings_as_errors + else: + raise KiPlotConfigurationError('must be boolean or dict') self._sch_related = True self._expand_id = 'erc' self._expand_ext = 'txt' @@ -45,33 +69,36 @@ class Run_ERC(BasePreFlight): # noqa: F821 out_dir = self.expand_dirname(GS.out_dir) if GS.global_dir and GS.global_use_dir_for_preflights: out_dir = os.path.join(out_dir, self.expand_dirname(GS.global_dir)) - return [os.path.abspath(os.path.join(out_dir, name))] + return [os.path.abspath(os.path.join(out_dir, self._dir, name))] + + @classmethod + def get_doc(cls): + return cls.__doc__, Run_ERCOptions def run(self): command = self.ensure_tool('KiAuto') - # The schematic is loaded only before executing an output related to it. - # But here we need data from it. - output = self.get_targets()[0] - os.makedirs(os.path.dirname(output), exist_ok=True) # Workaround for KiCad 7 odd behavior: it forces a file extension # Note: One thing is adding the extension before you enter a name, other is add something you removed on purpose with NamedTemporaryFile(mode='w', delete=False, suffix='.rpt', prefix='erc_report') as f: tmp_name = f.name logger.debug('ERC report: '+tmp_name) - cmd = [command, 'run_erc', '-o', tmp_name, '-g', str(GS.global_erc_grid)] - if BasePreFlight.get_option('erc_warnings'): # noqa: F821 + cmd = [command, 'run_erc', '-o', os.path.basename(tmp_name), '-g', str(GS.global_erc_grid)] + if self._warnings_as_errors or BasePreFlight.get_option('erc_warnings'): # noqa: F821 cmd.append('-w') if GS.filter_file: cmd.extend(['-f', GS.filter_file]) - cmd.extend([GS.sch_file, self.expand_dirname(GS.out_dir)]) + cmd.extend([GS.sch_file, os.path.dirname(tmp_name)]) # If we are in verbose mode enable debug in the child cmd = self.add_extra_options(cmd) logger.info('- Running the ERC') ret = self.exec_with_retry(cmd) + # Move the report to the desired name + output = self.get_targets()[0] + os.makedirs(os.path.dirname(output), exist_ok=True) try: move(tmp_name, output) except FileNotFoundError: - pass + logger.error(' Oops!') if ret: if ret > 127: ret = -(256-ret) diff --git a/tests/test_plot/test_preflight.py b/tests/test_plot/test_preflight.py index 284f3d79..408fd030 100644 --- a/tests/test_plot/test_preflight.py +++ b/tests/test_plot/test_preflight.py @@ -80,6 +80,19 @@ def test_erc_warning_2(test_dir): ctx.clean_up(keep_project=context.ki7()) +@pytest.mark.slow +@pytest.mark.eeschema +def test_erc_warning_3(test_dir): + """ Using an SCH with ERC warnings as errors """ + prj = 'warning-project' + ctx = context.TestContextSCH(test_dir, 'erc_warning/'+prj, 'erc_no_w_2', 'def_dir') + ctx.run(ERC_ERROR) + # Check all outputs are there + ctx.expect_out_file(prj+'-erc.txt', sub=True) + ctx.search_err(r"ERROR:1 ERC errors detected") + ctx.clean_up(keep_project=context.ki7()) + + @pytest.mark.slow @pytest.mark.eeschema @pytest.mark.skipif(not context.ki7(), reason="KiCad 7 off grid check") @@ -129,8 +142,8 @@ def test_drc_filter_2(test_dir): ctx.clean_up(keep_project=True) -def test_drc_unco(test_dir): - """ Check we can ignore unconnected nets """ +def test_drc_unco_1(test_dir): + """ Check we can ignore unconnected nets. Old style """ prj = 'warning-project' ctx = context.TestContext(test_dir, prj, 'drc_unco', '') ctx.run() @@ -139,6 +152,16 @@ def test_drc_unco(test_dir): ctx.clean_up() +def test_drc_unco_2(test_dir): + """ Check we can ignore unconnected nets. New style """ + prj = 'warning-project' + ctx = context.TestContext(test_dir, prj, 'drc_unco_2', 'def_dir') + ctx.run() + # Check all outputs are there + ctx.expect_out_file(prj+'-drc.txt', sub=True) + ctx.clean_up() + + def test_drc_error(test_dir): """ Check we catch DRC errors """ prj = 'warning-project' diff --git a/tests/yaml_samples/drc_unco_2.kibot.yaml b/tests/yaml_samples/drc_unco_2.kibot.yaml new file mode 100644 index 00000000..53404356 --- /dev/null +++ b/tests/yaml_samples/drc_unco_2.kibot.yaml @@ -0,0 +1,8 @@ +# Example KiBot config file +kibot: + version: 1 + +preflight: + run_drc: + dir: def_dir + ignore_unconnected: true diff --git a/tests/yaml_samples/erc_no_w_2.kibot.yaml b/tests/yaml_samples/erc_no_w_2.kibot.yaml new file mode 100644 index 00000000..fd9ab8b1 --- /dev/null +++ b/tests/yaml_samples/erc_no_w_2.kibot.yaml @@ -0,0 +1,18 @@ +# Example KiBot config file +kibot: + version: 1 + +preflight: + run_erc: + dir: 'def_dir' + warnings_as_errors: true + filters: + - filter: 'Ignore C3 pad 2 too close to anything' + number: 4 + regex: 'Pad 2 of C3' + - filter: 'Ignore unconnected pad 2 of C4' + number: 2 + regex: 'Pad 2 of C4' + - filter: 'Ignore KiCad 6 lib_symbol_issues' + error: lib_symbol_issues + regex: ''