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

[Drill][Added] CSV Drill Table output #760

Merged
merged 10 commits into from
Jan 6, 2025

Conversation

nguyen-v
Copy link

@nguyen-v nguyen-v commented Jan 2, 2025

This PR adds a CSV drill table output to the excellon output. I've added a simple test case in test_drill.py.
It's basically a reimplementation of
https://gitlab.com/kicad/code/kicad/-/blob/master/pcbnew/exporters/gendrill_file_writer_base.cpp
It should work for KiCad 5+, but for KiCad 5 the generated table is slightly different (because of missing attributes).
The column order/names can be changed similar to how it's done in bom.
Example from drill.kibot.yaml

# Drills and Gerber drills
kibot:
  version: 1

outputs:

  - name: excellon_drill
    comment: "Excellon drill files"
    type: excellon
    dir: Drill
    options:
      metric_units: true
      pth_and_npth_single_file: false
      use_aux_axis_as_origin: false
      minimal_header: false
      mirror_y_axis: false
      report: '%f-%i.%x'
      map: 'pdf'
      table:
        pth_and_npth_single_file: false

  - name: gerber_drills
    comment: "Gerber drill files"
    type: gerb_drill
    dir: Drill
    options:
      use_aux_axis_as_origin: false
      map: 'pdf'

I've also modified the 3Rs_bv.kicad_pcb PCB for KiCad 5, as it was not similar to the other versions (there was a missing through hole via, and the tests I wrote are expecting a similar PCB for all versions)

@nguyen-v
Copy link
Author

nguyen-v commented Jan 3, 2025

I'm not very satisfied with having to specify pth_and_npth_single_file twice. Should I move the DrillTable class to out_excellon.py instead of out_any_drill.py?

@set-soft
Copy link
Member

set-soft commented Jan 3, 2025

I'm not very satisfied with having to specify pth_and_npth_single_file twice. Should I move the DrillTable class to out_excellon.py instead of out_any_drill.py?

If the user wants gerber drills and drill table then DrillTable is for both and goes to out_any_drill.py
If the user wants a table for PTH and another for NPTH, but just one drill file for the drill machine then we need 2 options.
We could change the DrillTable version to be named differently, lets say "unify_pth_and_npth" and have values 'yes', 'no' and 'auto'. With 'auto' as default. Then 'auto' can copy the excellon pth_and_npth_single_file and be 'no' for gerbers.

@set-soft
Copy link
Member

set-soft commented Jan 3, 2025

Oh! BTW, the layer numbering is changing on KiCad 9 so we can't rely on B_Cu in this way.
Now F_Cu is 0, B_Cu is 2 and the inner layers are 4, 6, ... 62

@nguyen-v
Copy link
Author

nguyen-v commented Jan 3, 2025

We could change the DrillTable version to be named differently, lets say "unify_pth_and_npth" and have values 'yes', 'no' and 'auto'. With 'auto' as default. Then 'auto' can copy the excellon pth_and_npth_single_file and be 'no' for gerbers.

Sounds good

Oh! BTW, the layer numbering is changing on KiCad 9 so we can't rely on B_Cu in this way. Now F_Cu is 0, B_Cu is 2 and the inner layers are 4, 6, ... 62

https://gitlab.com/kicad/code/kicad/-/commit/a4292ea51631001c60cb01ac759606a13747f861
I will have a look at it

@nguyen-v
Copy link
Author

nguyen-v commented Jan 3, 2025

I've changed pth_and_npth_single_file to unify_pth_and_npth for the DrillOptions. By default it will be auto. If it detects that there is a pth_and_npth_single_file in the class (i.e. ExcellonOptions or any derivated class), it will use the pth_and_npth_single_file if unify_pth_and_npth = 'auto' else it will just use unify_pth_and_npth.
For classes without pth_and_npth_single_file (i.e. derivatives of AnyDrill), unify_pth_and_npth will default to 'no' if nothing is specified.
I've added some support for the new layer numbering (I think it only affects the function that gets the layer names in physical order, front being 1 and back being the # of layers). Could you have a quick look at it?

@set-soft
Copy link
Member

set-soft commented Jan 3, 2025

Could you have a quick look at it?

Looks ok.

Also take a look to the review about GS.get_modules()

@nguyen-v
Copy link
Author

nguyen-v commented Jan 3, 2025

Also take a look to the review about GS.get_modules()

What do you mean? Did you write a review somewhere? Is it about using GS.get_modules() here https://github.com/nguyen-v/KiBot/blob/c6241a02748a94ff6ee2aedc554267303e54c14d/kibot/kicad/drill_info.py#L193

@set-soft
Copy link
Member

set-soft commented Jan 3, 2025

Also take a look to the review about GS.get_modules()

What do you mean? Did you write a review somewhere? Is it about using GS.get_modules() here https://github.com/nguyen-v/KiBot/blob/c6241a02748a94ff6ee2aedc554267303e54c14d/kibot/kicad/drill_info.py#L193

Yes, this should be a call to GS.get_modules(), which already does what you wrote there.

@@ -186,7 +186,8 @@ def build_holes_list(layer_pair, merge_PTH_NPTH, generate_NPTH_list=True,

# Add footprint/pad related PTH to hole_list_layer_pair
if layer_pair == (pcbnew.F_Cu, pcbnew.B_Cu):
for footprint in GS.board.GetFootprints():
footprints = GS.board.GetModules() if GS.ki5 else GS.board.GetFootprints()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already abstracted in GS.get_modules()

@set-soft
Copy link
Member

set-soft commented Jan 3, 2025

Now should be visible, I forgot to mark it as ready

@set-soft set-soft merged commit 4bfef85 into INTI-CMNB:dev Jan 6, 2025
16 of 17 checks passed
@set-soft
Copy link
Member

set-soft commented Jan 6, 2025

Hi @nguyen-v !
I did some changes:

  1. I removed spaces in the layer pair names, spaces in file names are a bad thing
  2. I changed the way columns are validated using more internal checks instead of manual checks

@nguyen-v
Copy link
Author

nguyen-v commented Jan 6, 2025

Hi @nguyen-v ! I did some changes:

  1. I removed spaces in the layer pair names, spaces in file names are a bad thing
  2. I changed the way columns are validated using more internal checks instead of manual checks

Thanks!

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

Successfully merging this pull request may close these issues.

2 participants