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

[FEATURE] Support custom worksheets version 20220228 #532

Closed
oliv3r opened this issue Dec 17, 2023 · 11 comments
Closed

[FEATURE] Support custom worksheets version 20220228 #532

oliv3r opened this issue Dec 17, 2023 · 11 comments
Assignees
Labels
enhancement New feature or request

Comments

@oliv3r
Copy link

oliv3r commented Dec 17, 2023

With the kicad 7 full docker image, using my own worksheet, kibot complains about SHEETPATH being unknown and the version being incompatible. The version in the wks was made using kicad 7 and is dated feb 2022, so not that new.

WARNING:(W086) Unsupported worksheet version, loading could fail (kibot - worksheet.py:498)
WARNING:(W082) Unknown text variable `SHEETPATH` (kibot.gs - gs.py:407)

The outputs section I got from an kibot example, which looks like:

  - name: 'pdf_pcb_print'
    comment: 'Exports the PCB to the most common exchange format. Suitable for printing.'
    type: 'pcb_print'
    dir: 'output/pdf'
    options:
      force_edge_cuts: true
      keep_temporal_files: false
      sheet_reference_layout: '../git.kicad_wks'
      scaling: 2.0
      pages:
        - layers: [ 'F.Paste', 'F.Adhes', 'Dwgs.User', 'F.Fab' ]
          sheet: 'Fabrication layers'
        - layers:
            - layer: 'F.Cu'
            - layer: 'F.Mask'
              color: '#14332440'
            - layer: 'F.SilkS'
            - layer: 'Dwgs.User'
          sheet: 'Top layer'
        - layers: [ 'In1.Cu', 'Dwgs.User' ]
          sheet: 'Inner1 plane'
        - layers: [ 'In2.Cu', 'Dwgs.User' ]
          sheet: 'Inner2 plane'
        - layers:
            - layer: 'B.Cu'
            - layer: 'B.Mask'
              color: '#14332440'
            - layer: 'B.SilkS'
            - layer: 'Dwgs.User'
          sheet: 'Bottom layer'
          mirror: true

I haven't really tweaked or modified it yet, but stumbled upon the error as I was going ...

I suppose the sheetpath error is not that relevant, as it's a follow up from the earlier error, about the unsupported wks version.

It does seem to somewhat work, the first page is empty, and the wks looks like the default, the other 4 sheets for do have my 'git' wks. All sheets however list the 'SHEETPATH' variable for the 'SHEET' field. Which I think kicad sets to '/'.

git.kicad_wks.zip

@set-soft set-soft added the enhancement New feature or request label Dec 18, 2023
@set-soft set-soft changed the title [BUG] Custom worksheet causes warnings [FEATURE] Support custom worksheets version 20220228 Dec 18, 2023
@set-soft
Copy link
Member

Hi @oliv3r !
You still confuse warning and error. This is not an error, and this is why somewhat work. KiBot is just explaining you that it wasn't tested using v20220228 and hence you could find things like a newly defined variable, SHEETPATH. Which, yes, is irrelevant for a PCB.

Which I think kicad sets to '/'.

In fact KiCad sets it to empty because this variable has meaning only for schematics.

Note that this format is poorly documented, official docs are here, which says Last Modified 2023-04-13, but if you take a look at KiBot code you'll find comments like Not documented 2022/04/15, and it still not documented.

Now about the first empty page: This seems to be a bug in KiCad, take a look at the SVGs (keep_temporal_files: false), you'll see the scaled outputs are drawn out of the page. This is somehow fixed when the first SVG in the merged output is correctly inside the page. I have to figure out how to workaround it. For your case: just manually include the Edge.Cuts layer (not forced).

set-soft added a commit that referenced this issue Dec 18, 2023
- Not used for PCBs
- KiCad defines it as empty

See #532
set-soft added a commit that referenced this issue Dec 18, 2023
- When forced
- This is needed and helps to avoid hitting a bug in KiCad

Fixes #532
@set-soft
Copy link
Member

The above patch solves the empty first page. Is a fix for KiBot, but doesn't solve the errors in computed coordinates seen in KiCad side, in a test using paste in almost all the PCB the coordinates made the PCB "invisible", your problem, without a real reason.

set-soft added a commit that referenced this issue Dec 18, 2023
- Seems to be fully supported
- We added plenty of undocumented stuff on 2022/04/15
- They still undocumented today

See #532
@oliv3r
Copy link
Author

oliv3r commented Dec 19, 2023

@set-soft I'm not confusing errors and warnings, but I do set '-Werror' if you will, e.g. a pipeline run for a tagged release, is expected to be warning free. So warnings are 'just as bad'.

I suppose you could argue, that you confuse the severity of a warning, which could be 'INFO' :)

With my custom wks, I wanted to use the same wks for my PCB and Schematics. I noticed that the default wks uses the SHEET variable for PCB, and SHEETPATH for schematics. Why doesn't kibot use the kicad_pro (page setup) defined wks though? Different bug? What wks is the default for PCB's? I tried to use ${SHEETPATH:-${SHEETNAME}} but that still triggers the warning (obviously). I'm now just using ${SHEETPATH}${SHEETNAME} and rely on the fact that they are mutually exclusive, but that of course triggers the warning. Interestingly, the warning is only triggered on the PCB, not the SCH. I know I can filter the warning, but rather have a clean way to deal with this. How can I set the variable to avoid this warning as an alternative solution (a bit out of scope). Creating a pre-flight that just creates an empty variable works, but is also slightly less 'clean', and creating two wks's just to have a different name here seems a bit overkill.

The first page thing, it was only empty if I had a single layer there. Having two layers 'removes' the problem. But using the manual edge cuts, if it works, is also fine by me. I suppose I triggered that bug by accident anyway, as I've decided to want something different on my first page now anyway :)

@oliv3r
Copy link
Author

oliv3r commented Dec 19, 2023

@set-soft I've also created https://gitlab.com/kicad/services/kicad-dev-docs/-/issues/26 to track this upstream.

@set-soft
Copy link
Member

@set-soft I'm not confusing errors and warnings, but I do set '-Werror' if you will, e.g. a pipeline run for a tagged release, is expected to be warning free. So warnings are 'just as bad'.

But you say things like I suppose the sheetpath error is not that relevant ... when this is a warning, not an error.

I suppose you could argue, that you confuse the severity of a warning, which could be 'INFO' :)

I apply the following criteria: if I have to inform something that is quite possible to generate a problem then I name it warning. If you try to expand a variable that doesn't exist then this is a warning, not INFO. And I'm not talking about the severities, I'm saying you call errors to messages reported as warnings.

With my custom wks, I wanted to use the same wks for my PCB and Schematics. I noticed that the default wks uses the SHEET variable for PCB, and SHEETPATH for schematics. Why doesn't kibot use the kicad_pro (page setup) defined wks though?

KiBot uses it if you define it, and the file is available of course.

Different bug? What wks is the default for PCB's?

This one, copied from KiCad.

I tried to use ${SHEETPATH:-${SHEETNAME}} but that still triggers the warning (obviously). I'm now just using ${SHEETPATH}${SHEETNAME} and rely on the fact that they are mutually exclusive, but that of course triggers the warning.

Switch to dev images, otherwise you won't have the above mentioned patches.

Interestingly, the warning is only triggered on the PCB, not the SCH.

Because KiCad handles the schematic. KiCad lacks a Python API for schematics, was in the KiCad 6 roadmap, but not even KiCad 8 implements it.

I know I can filter the warning, but rather have a clean way to deal with this. How can I set the variable to avoid this warning as an alternative solution (a bit out of scope). Creating a pre-flight that just creates an empty variable works, but is also slightly less 'clean', and creating two wks's just to have a different name here seems a bit overkill.

Look in the history of this issue, you'll find the dev code now defines SHEETPATH to nothing.

The first page thing, it was only empty if I had a single layer there. Having two layers 'removes' the problem. But using the manual edge cuts, if it works, is also fine by me. I suppose I triggered that bug by accident anyway, as I've decided to want something different on my first page now anyway :)

KiCad logic about how scales are computed is buggy or at least questionable. When the Edge.Cuts isn't visible it miserably fail to compute the bounding box and prints things outside the page.

@oliv3r
Copy link
Author

oliv3r commented Dec 21, 2023

@set-soft I'm not confusing errors and warnings, but I do set '-Werror' if you will, e.g. a pipeline run for a tagged release, is
I apply the following criteria: if I have to inform something that is quite possible to generate a problem then I name it warning. If you try to expand a variable that doesn't exist then this is a warning, not INFO. And I'm not talking about the severities, I'm saying you call errors to messages reported as warnings.

I apply the following criteria; when something is run from the pipeline, there shall be no warnings or errors allowed. In very certain scenario's, a warning could be a allow_failure: true in gitlab pipeline terminology, resulting in an orange pipeline. But this should not be structural, we want green pipelines, not orange or reds.

In terms of a developer, I follow the criteria 'for development, warnings can be allowed, for production builds, treat all warnings as errors (-Werror).

You say though, warnings could be a problem, but can be ignored. So that's where we differ, and that's fine; but now I need ways to deal with this :) For example, Warnings do not cause non-zero exit codes etc.

Though I suppose that discussion warrants its own bug report to talk about maybe? :)

@set-soft
Copy link
Member

For example, Warnings do not cause non-zero exit codes etc.

And this is the case, warnings doesn't change the exit code.

Though I suppose that discussion warrants its own bug report to talk about maybe? :)

If you want to propose a -Werror equivalent feature, then yes, open a separated issue.

@oliv3r
Copy link
Author

oliv3r commented Jan 4, 2024

@set-soft Btw, considering the title 'support worksheets version 20220228, in combination with #532 (comment), should we keep this issue open until this is fixed upstream?

@set-soft
Copy link
Member

set-soft commented Jan 4, 2024

I don't think they added anything important that isn't currently supported by KiBot. I think is safe to assume the "new" format is fully supported. If not people can open issues, this is quite common due to the lack of documentation.

@oliv3r
Copy link
Author

oliv3r commented Jan 4, 2024

@set-soft I meant more in the lines to track the warning. Right now, we get a warning that the version is unsupported. Once all features are properly documented, the warnings can be removed, and this issue closed? Just to track things I meant really.

@set-soft
Copy link
Member

set-soft commented Jan 4, 2024

v1.6.4 doesn't print a warning, assumes 20220228 is fully supported

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants