INTI-CMNB / KiBot

KiCad automation utility
GNU Affero General Public License v3.0
571 stars 67 forks source link

[BUG] ERC/DRC errors/warnings do not cause exit 0 without '--stop-on-warning' (or --stop-on-warning should print the full report) #637

Closed oliv3r closed 4 months ago

oliv3r commented 4 months ago

Describe the bug When running kibot from a CI/CD pipeline, it is reasonable to use --stop-on-warning (aka --Werror), treat warnings as errors. However in that case, we do not get a full error report. KiCad does save it in the .rpt file, so we can get the actually cause of the error. Kibot will just print 'there where 14 DRC errors'.

Expected behavior When running the DRC (or ERC), always print the error report (in kibot colorized fashion). Alternatively (but we discussed this in the past) KiBot should 'exit 0' when warnings/errors occur.

Screenshots

# kibot  -c .kibot/kici_drc.kibot.yaml 
Using SCH file: ochime.kicad_sch
- Running the DRC
WARNING:(W145) 1 DRC warnings detected (kibot.pre_drc - pre_drc.py:149)
WARNING:(W145) (silk_over_copper) Silkscreen clipped by solder mask
    @(105.0 mm, 46.0 mm): PCB Text '${VCS_TAG}, ${VCS_PCB_DATE}' on B.Silkscreen
    Check: Violations (kibot.pre_drc - pre_drc.py:149)
ERROR:DRC warnings: 1, promoted as errors (kibot.gs - gs.py:836)
Found 2 unique warning/s (3 total, 1 filtered)
# echo $?
0

Environment (please complete the following information): Where are you running KiBot:

set-soft commented 4 months ago
  1. The --stop-on-warnings option will stop as soon any kind of warning is issued. So you can't pretend that KiBot won't stop just because more warnings and/or errors were detected.
  2. ERC/DRC errors must stop and generate an error level. This is the intended behavior.

Both are documented and intended behavior.

If you want a full report and stop execution on ERC/DRC warnings use the erc/drc preflights (not run_erc/run_drc) and enable the warnings_as_errors options. It will show the full report.

oliv3r commented 4 months ago

So, this is what I do. I use the following prefilight:

preflight:
  erc:
    dir: 'reports'
    dont_stop: true
    enabled: true
    format: 'CSV,HTML,JSON,RPT'
    warnings_as_errors: true

but when running kibot, I do not get an exit value of !0.

root@30a6156963df:/workdir/breakout_hdmi# kibot -c .kibot/kici_erc.kibot.yaml 
Using SCH file: breakout_hdmi.kicad_sch
- Running the ERC
WARNING:(W142) 8 ERC warnings detected (kibot.pre_erc - pre_erc.py:144)
WARNING:(W142) (lib_symbol_issues) The current configuration does not include the library 'LCSC'
    @(1.6256 mm, 1.2192 mm): Symbol LED4 [TC5050RGBF08-3CJH-AF53A]
    Sheet: / (kibot.pre_erc - pre_erc.py:144)
WARNING:(W142) (lib_symbol_issues) Symbol 'HDMI_A' has been modified in library 'Connector'
    @(1.6256 mm, 0.5334 mm): Symbol J4 [HDMI_A]
    Sheet: / (kibot.pre_erc - pre_erc.py:144)
WARNING:(W142) (lib_symbol_issues) Symbol 'HDMI_A' has been modified in library 'Connector'
    @(1.1684 mm, 0.5334 mm): Symbol J3 [HDMI_A]
    Sheet: / (kibot.pre_erc - pre_erc.py:144)
WARNING:(W142) (lib_symbol_issues) Symbol 'HDMI_A' has been modified in library 'Connector'
    @(0.7366 mm, 0.5334 mm): Symbol J2 [HDMI_A]
    Sheet: / (kibot.pre_erc - pre_erc.py:144)
WARNING:(W142) (lib_symbol_issues) Symbol 'HDMI_A' has been modified in library 'Connector'
    @(0.3048 mm, 0.5334 mm): Symbol J1 [HDMI_A]
    Sheet: / (kibot.pre_erc - pre_erc.py:144)
WARNING:(W142) (lib_symbol_issues) The current configuration does not include the library 'LCSC'
    @(0.3048 mm, 1.2192 mm): Symbol LED1 [TC5050RGBF08-3CJH-AF53A]
    Sheet: / (kibot.pre_erc - pre_erc.py:144)
WARNING:(W142) (lib_symbol_issues) The current configuration does not include the library 'LCSC'
    @(0.7366 mm, 1.2192 mm): Symbol LED2 [TC5050RGBF08-3CJH-AF53A]
    Sheet: / (kibot.pre_erc - pre_erc.py:144)
WARNING:(W142) (lib_symbol_issues) The current configuration does not include the library 'LCSC'
    @(1.1684 mm, 1.2192 mm): Symbol LED3 [TC5050RGBF08-3CJH-AF53A]
    Sheet: / (kibot.pre_erc - pre_erc.py:144)
ERROR:ERC warnings: 8, promoted as errors (kibot.gs - gs.py:836)
Found 9 unique warning/s (11 total, 2 filtered)
root@30a6156963df:/workdir/breakout_hdmi# echo $?
0

Running the same preflight, we do stop, but loose the log.

root@30a6156963df:/workdir/breakout_hdmi# kibot --stop-on-warnings -c .kibot/kici_erc.kibot.yaml 
Using SCH file: breakout_hdmi.kicad_sch
- Running the ERC
WARNING:(W142) 8 ERC warnings detected (kibot.pre_erc - pre_erc.py:144)
ERROR:Warnings treated as errors (kibot.pre_erc - log.py:115)
root@30a6156963df:/workdir/breakout_hdmi# echo $?
36

So both options are working, just not the combination as expected (due to warnings_as_errors: true). So is this a bug or working as intended, because it's not very obvious to me :)

set-soft commented 4 months ago

The problem is dont_stop: true, remove it!

oliv3r commented 4 months ago

The problem is dont_stop: true, remove it!

Ah, curious; unexpected for sure!

set-soft commented 4 months ago

You are saying "Don't stop, even if the ERC has errors" and also "Warnings are reported as warnings, but taken as errors". So in this case the return value is 0 and the execution doesn't stop, even when the warnings becomes as severe as errors.

Then you try to fix the "problem" using "--stop-on-warning", so at the first warning (reported as a warning, which is documented" KiBot stops. The command line normally has more precedence than the configuration file. So this is normal for such a contradiction

oliv3r commented 4 months ago

You are saying "Don't stop, even if the ERC has errors" Indeed, I want the ERC to run all tests, so we get a full report

"Warnings are reported as warnings, but taken as errors" Indeed, as I want to get an exit !0, so that the CI pipeline will properly fail on warnings and errors!

"problem" using "--stop-on-warning", so at the first warning Which is treated as error.

So maybe I had this flag from before the new kicad erc 8, and it was working well before. I'm not sure. Likewise I just hope this won't cause problems for other jobs, which rely on --stop-on-warning.

Anyways, I made use of your suggestions, so lets see if this does as I expect :)

Thanks Salvador!

set-soft commented 3 months ago

Hi @oliv3r !

Patch 3ff1d18050763729f45ad5f87ac00cdd46e6625e will allow to use --stop-on-warnings and get all the warnings. Now the code temporally disables the flag, prints all warnings and then exits

oliv3r commented 3 months ago

Thank You! I'll try it out asap!

oliv3r commented 1 month ago

@set-soft This seems to work I think, any idea when you'll tag a new release with these changes in-cooperated?

set-soft commented 1 month ago

I'm arranging a new release, but I want to also include KiCad 8.0.4 support, which is currently breaking at leas the 3D printed stencil from KiKit.

You can use the dev images, they are the current "release candidate"