From b9cb24c325ab37e8c01a44bf3100af5a0b33af65 Mon Sep 17 00:00:00 2001 From: "Salvador E. Tropea" Date: Thu, 2 Dec 2021 13:21:57 -0300 Subject: [PATCH] Now you get an error when defining two outputs with the same name. --- CHANGELOG.md | 1 + kibot/config_reader.py | 23 +++++++++-------- kibot/registrable.py | 25 +++++++++++++++++-- tests/test_plot/test_yaml_errors.py | 24 ++++++++++++++++++ .../yaml_samples/error_same_name_1.kibot.yaml | 24 ++++++++++++++++++ .../yaml_samples/error_same_name_2.kibot.yaml | 17 +++++++++++++ .../yaml_samples/error_same_name_3.kibot.yaml | 18 +++++++++++++ tests/yaml_samples/pcbdraw.kibot.yaml | 2 +- tests/yaml_samples/pcbdraw_simple.kibot.yaml | 2 +- 9 files changed, 122 insertions(+), 14 deletions(-) create mode 100644 tests/yaml_samples/error_same_name_1.kibot.yaml create mode 100644 tests/yaml_samples/error_same_name_2.kibot.yaml create mode 100644 tests/yaml_samples/error_same_name_3.kibot.yaml diff --git a/CHANGELOG.md b/CHANGELOG.md index 13e7e9c4..89bf68f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - The `%v/%V` expansion patterns now expand to the global variant when used in a context not related to variants. I.e. when a `compress` target expands `%v`. +- Now you get an error when defining two outputs with the same name. ### Fixed - Position files now defaults to use the auxiliar origin as KiCad. diff --git a/kibot/config_reader.py b/kibot/config_reader.py index e18630f1..0538c814 100644 --- a/kibot/config_reader.py +++ b/kibot/config_reader.py @@ -200,7 +200,7 @@ class CfgYamlReader(object): return values CfgYamlReader._config_error_import(fname, '`{}` must be a string or a list ({})'.format(kind, str(v))) - def _parse_import_outputs(self, outs, explicit_outs, fn_rel, data, outputs): + def _parse_import_outputs(self, outs, explicit_outs, fn_rel, data): if (outs is None or len(outs) > 0) and 'outputs' in data: i_outs = self._parse_outputs(data['outputs']) if outs is not None: @@ -216,8 +216,11 @@ class CfgYamlReader(object): if len(sel_outs) == 0: logger.warning(W_NOOUTPUTS+"No outputs found in `{}`".format(fn_rel)) else: - outputs.extend(sel_outs) - logger.debug('Outputs loaded from `{}`: {}'.format(fn_rel, list(map(lambda c: c.name, sel_outs)))) + try: + RegOutput.add_outputs(sel_outs, fn_rel) + except KiPlotConfigurationError as e: + config_error(str(e)) + logger.debug('Outputs loaded from `{}`: {}'.format(fn_rel, list(map(lambda c: c.name, sel_outs)))) if outs is None and explicit_outs and 'outputs' not in data: logger.warning(W_NOOUTPUTS+"No outputs found in `{}`".format(fn_rel)) @@ -290,7 +293,6 @@ class CfgYamlReader(object): config_error("Incorrect `import` section (must be a list)") # Import the files dir = os.path.dirname(os.path.abspath(name)) - outputs = [] for entry in imp: if isinstance(entry, str): fn = entry @@ -335,14 +337,13 @@ class CfgYamlReader(object): fn_rel = os.path.relpath(fn) data = self.load_yaml(open(fn)) # Outputs - self._parse_import_outputs(outs, explicit_outs, fn_rel, data, outputs) + self._parse_import_outputs(outs, explicit_outs, fn_rel, data) # Filters self._parse_import_filters(fils, explicit_fils, fn_rel, data) # Variants self._parse_import_variants(vars, explicit_vars, fn_rel, data) # Globals self._parse_import_globals(globals, explicit_globals, fn_rel, data) - return outputs def load_yaml(self, fstream): try: @@ -368,7 +369,6 @@ class CfgYamlReader(object): GS.global_kiauto_wait_start = GS.global_from_cli.get('kiauto_wait_start', None) GS.global_kiauto_time_out_scale = GS.global_from_cli.get('kiauto_time_out_scale', None) # List of outputs - outputs = [] version = None globals_found = False # Analize each section @@ -382,13 +382,16 @@ class CfgYamlReader(object): self._parse_global(v) globals_found = True elif k == 'import': - outputs.extend(self._parse_import(v, fstream.name)) + self._parse_import(v, fstream.name) elif k == 'variants': RegOutput.add_variants(self._parse_variants(v)) elif k == 'filters': RegOutput.add_filters(self._parse_filters(v)) elif k == 'outputs': - outputs.extend(self._parse_outputs(v)) + try: + RegOutput.add_outputs(self._parse_outputs(v)) + except KiPlotConfigurationError as e: + config_error(str(e)) else: config_error('Unknown section `{}` in config.'.format(k)) if version is None: @@ -402,7 +405,7 @@ class CfgYamlReader(object): GS.solved_global_variant = RegOutput.check_variant(GS.global_variant) except KiPlotConfigurationError as e: config_error("In global section: "+str(e)) - return outputs + return RegOutput.get_outputs() def trim(docstring): diff --git a/kibot/registrable.py b/kibot/registrable.py index 7776ac64..07b6d72c 100644 --- a/kibot/registrable.py +++ b/kibot/registrable.py @@ -1,8 +1,9 @@ # -*- coding: utf-8 -*- -# Copyright (c) 2020 Salvador E. Tropea -# Copyright (c) 2020 Instituto Nacional de Tecnología Industrial +# Copyright (c) 2020-2021 Salvador E. Tropea +# Copyright (c) 2020-2021 Instituto Nacional de Tecnología Industrial # License: GPL-3.0 # Project: KiBot (formerly KiPlot) +from collections import OrderedDict from .optionable import Optionable from .error import KiPlotConfigurationError @@ -41,6 +42,8 @@ class RegOutput(Optionable, Registrable): _def_filters = {} # List of defined variants _def_variants = {} + # List of defined outputs + _def_outputs = OrderedDict() def __init__(self): super().__init__() @@ -73,6 +76,24 @@ class RegOutput(Optionable, Registrable): def add_filter(obj): RegOutput._def_filters[obj.name] = obj + @staticmethod + def add_output(obj, file=None): + if obj.name in RegOutput._def_outputs: + msg = "Output name `{}` already defined".format(obj.name) + if file: + msg += ", while importing from `{}`".format(file) + raise KiPlotConfigurationError(msg) + RegOutput._def_outputs[obj.name] = obj + + @staticmethod + def add_outputs(objs, file=None): + for o in objs: + RegOutput.add_output(o, file) + + @staticmethod + def get_outputs(): + return RegOutput._def_outputs.values() + @staticmethod def check_variant(variant): if variant: diff --git a/tests/test_plot/test_yaml_errors.py b/tests/test_plot/test_yaml_errors.py index 9dc860fd..8528556e 100644 --- a/tests/test_plot/test_yaml_errors.py +++ b/tests/test_plot/test_yaml_errors.py @@ -696,3 +696,27 @@ def test_error_import_no_outputs(test_dir): ctx.run() assert ctx.search_err(r"No outputs found in `(.*)drc.kibot.yaml`") ctx.clean_up() + + +def test_same_name_1(test_dir): + """ 2 outputs with the same name in the same file """ + ctx = context.TestContext(test_dir, 'test_same_name_1', PRJ, 'error_same_name_1', '') + ctx.run(EXIT_BAD_CONFIG) + assert ctx.search_err(r"Output name `position` already defined") + ctx.clean_up() + + +def test_same_name_2(test_dir): + """ Using import, but the 2nd is in the main file """ + ctx = context.TestContext(test_dir, 'test_same_name_2', PRJ, 'error_same_name_2', '') + ctx.run(EXIT_BAD_CONFIG) + assert ctx.search_err(r"Output name `position` already defined") + ctx.clean_up() + + +def test_same_name_3(test_dir): + """ Using import and the 2nd is from the import """ + ctx = context.TestContext(test_dir, 'test_same_name_3', PRJ, 'error_same_name_3', '') + ctx.run(EXIT_BAD_CONFIG) + assert ctx.search_err(r"Output name `position` already defined, while importing from") + ctx.clean_up() diff --git a/tests/yaml_samples/error_same_name_1.kibot.yaml b/tests/yaml_samples/error_same_name_1.kibot.yaml new file mode 100644 index 00000000..d5f93ce0 --- /dev/null +++ b/tests/yaml_samples/error_same_name_1.kibot.yaml @@ -0,0 +1,24 @@ +# Example KiBot config file for a basic 2-layer board +kibot: + version: 1 + +outputs: + - name: 'position' + comment: "Pick and place file" + type: position + dir: positiondir + options: + format: ASCII # CSV or ASCII format + units: millimeters # millimeters or inches + separate_files_for_front_and_back: true + only_smd: true + + - name: 'position' + comment: "Pick and place file" + type: position + dir: positiondir + options: + format: ASCII # CSV or ASCII format + units: millimeters # millimeters or inches + separate_files_for_front_and_back: true + only_smd: true diff --git a/tests/yaml_samples/error_same_name_2.kibot.yaml b/tests/yaml_samples/error_same_name_2.kibot.yaml new file mode 100644 index 00000000..52b4dc5e --- /dev/null +++ b/tests/yaml_samples/error_same_name_2.kibot.yaml @@ -0,0 +1,17 @@ +# Example KiBot config file for a basic 2-layer board +kibot: + version: 1 + +import: + - simple_position.kibot.yaml + +outputs: + - name: 'position' + comment: "Pick and place file" + type: position + dir: positiondir + options: + format: ASCII # CSV or ASCII format + units: millimeters # millimeters or inches + separate_files_for_front_and_back: true + only_smd: true diff --git a/tests/yaml_samples/error_same_name_3.kibot.yaml b/tests/yaml_samples/error_same_name_3.kibot.yaml new file mode 100644 index 00000000..1b9a26f6 --- /dev/null +++ b/tests/yaml_samples/error_same_name_3.kibot.yaml @@ -0,0 +1,18 @@ +# Example KiBot config file for a basic 2-layer board +kibot: + version: 1 + +outputs: + - name: 'position' + comment: "Pick and place file" + type: position + dir: positiondir + options: + format: ASCII # CSV or ASCII format + units: millimeters # millimeters or inches + separate_files_for_front_and_back: true + only_smd: true + +import: + - simple_position.kibot.yaml + diff --git a/tests/yaml_samples/pcbdraw.kibot.yaml b/tests/yaml_samples/pcbdraw.kibot.yaml index 011e2ec0..3105ded8 100644 --- a/tests/yaml_samples/pcbdraw.kibot.yaml +++ b/tests/yaml_samples/pcbdraw.kibot.yaml @@ -47,7 +47,7 @@ outputs: warnings: visible dpi: 600 - - name: PcbDraw + - name: PcbDraw2 comment: "PcbDraw test bottom" type: pcbdraw dir: PcbDraw diff --git a/tests/yaml_samples/pcbdraw_simple.kibot.yaml b/tests/yaml_samples/pcbdraw_simple.kibot.yaml index f2544518..686be017 100644 --- a/tests/yaml_samples/pcbdraw_simple.kibot.yaml +++ b/tests/yaml_samples/pcbdraw_simple.kibot.yaml @@ -15,7 +15,7 @@ outputs: warnings: all show_components: all - - name: PcbDraw + - name: PcbDraw2 comment: "PcbDraw test bottom" type: pcbdraw dir: PcbDraw